+ Andrew Andrew, here's the full thread, I cut most of it out: https://lore.kernel.org/lkml/20240208220604.140859-1-seanjc@xxxxxxxxxx/ . On Thu, Feb 8, 2024 at 2:06 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > Explicitly require gcc-12+ to enable asm goto with outputs on gcc to avoid > what is effectively a data corruption bug on gcc-11. As per > https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html, "asm goto" is > *supposed* be implicitly volatile, but gcc-11 fails to treat it as such. > When compiling with -O2, failure to treat the asm block as volatile can > result in the entire block being discarded during optimization. > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103979 Shoot, I was cc'ed on that bug (I think I noticed it in testing as well, and pointed it out to Andrew on IRC who then cc'ed me to it). I probably should have asked if that would cause issues at some point for the kernel. I took a look at the test case added in that bug; it doesn't compile until gcc-13 (specifically gcc 13.2, not gcc 13.1). I'm curious since the bug says the fix was backported to gcc-12 and gcc-13. Are there specific versions of those that contain the fix? If so, should Sean amend his version checks below? For instance, was the fix backported to gcc 12.3, so users of gcc 12.2 would still have issues? I can't tell in godbolt since the added test case doesn't compile until gcc 13.2. https://godbolt.org/z/eqaa7dfo3 My implementation in clang still has some issues, too. It's hard to get new control flow right, and there are minimal users outside the kernel to help us validate. So as much of a fan of feature detection as I am, I admit some of these edge cases aren't perfect, and we may need to result to version detection when such bugs become observable to users. I'm happy to ack this patch, but I would like to wait for feedback from Andrew as to whether we can be even more precise with avoiding more specific versions of gcc 12 and 13 (if necessary). > Fixes: 587f17018a2c ("Kconfig: add config option for asm goto w/ outputs") > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Cc: kvm@xxxxxxxxxxxxxxx > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> > --- > > Linus, I'm sending to you directly as this seems urgent enough to apply > straightaway, and this obviously affects much more than the build system. > > init/Kconfig | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/init/Kconfig b/init/Kconfig > index deda3d14135b..f4e46d64c1e7 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -82,6 +82,11 @@ config CC_CAN_LINK_STATIC > default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m32-flag) -static) > > config CC_HAS_ASM_GOTO_OUTPUT > + # gcc-11 has a nasty bug where it doesn't treat asm goto as volatile, > + # which can result in asm blocks being dropped when compiling with -02. > + # Note, explicitly forcing volatile doesn't entirely fix the bug! > + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103979 > + depends on !CC_IS_GCC || GCC_VERSION >= 120000 LGTM; but we might need to be more specific about avoiding certain min versions of gcc 13 and 12. > def_bool $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null) > > config CC_HAS_ASM_GOTO_TIED_OUTPUT > > base-commit: 047371968ffc470769f541d6933e262dc7085456 > -- > 2.43.0.687.g38aa6559b0-goog > -- Thanks, ~Nick Desaulniers
