compiler/gcc4: Add quirk for 'asm goto' miscompilation bug

Linus,

Please pull the latest core-urgent-for-linus git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-urgent-for-linus

HEAD: 3f0116c3238a96bc18ad4b4acefe4e7be32fa861 compiler/gcc4: Add quirk for 'asm goto' miscompilation bug

This is the fix for the GCC miscompilation discussed in the following lkml thread:

[x86] BUG: unable to handle kernel paging request at 00740060

The bug in GCC has been fixed by Jakub and the fix will be part of the GCC 4.8.2 release expected to be released next week - so the quirk's version test checks for <= 4.8.1.

The quirk is only added to compiler-gcc4.h and not to the higher level compiler.h because all asm goto uses are behind a feature check.

Thanks,

Ingo

------------------>
Ingo Molnar (1):
compiler/gcc4: Add quirk for 'asm goto' miscompilation bug

 arch/arm/include/asm/jump_label.h     |  2 +-
 arch/mips/include/asm/jump_label.h    |  2 +-
 arch/powerpc/include/asm/jump_label.h |  2 +-
 arch/s390/include/asm/jump_label.h    |  2 +-
 arch/sparc/include/asm/jump_label.h   |  2 +-
 arch/x86/include/asm/cpufeature.h     |  6 +++---
 arch/x86/include/asm/jump_label.h     |  2 +-
 arch/x86/include/asm/mutex_64.h       |  4 ++--
 include/linux/compiler-gcc4.h         | 15 +++++++++++++++
 9 files changed, 26 insertions(+), 11 deletions(-)

arch/arm/include/asm/jump_label.h +1 -1

index bfc198c..863c892 100644
@@ -16,7 +16,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-       asm goto("1:\n\t"
+       asm_volatile_goto("1:\n\t"
                 JUMP_LABEL_NOP "\n\t"
                 ".pushsection __jump_table,  \"aw\"\n\t"
                 ".word 1b, %l[l_yes], %c0\n\t"

arch/mips/include/asm/jump_label.h +1 -1

index 4d6d77e..e194f95 100644
@@ -22,7 +22,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-       asm goto("1:\tnop\n\t"
+       asm_volatile_goto("1:\tnop\n\t"
                "nop\n\t"
                ".pushsection __jump_table,  \"aw\"\n\t"
                WORD_INSN " 1b, %l[l_yes], %0\n\t"

arch/powerpc/include/asm/jump_label.h +1 -1

index ae098c4..f016bb6 100644
@@ -19,7 +19,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-       asm goto("1:\n\t"
+       asm_volatile_goto("1:\n\t"
                 "nop\n\t"
                 ".pushsection __jump_table,  \"aw\"\n\t"
                 JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"

arch/s390/include/asm/jump_label.h +1 -1

index 6c32190..346b1c8 100644
@@ -15,7 +15,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-       asm goto("0:    brcl 0,0\n"
+       asm_volatile_goto("0:   brcl 0,0\n"
                ".pushsection __jump_table, \"aw\"\n"
                ASM_ALIGN "\n"
                ASM_PTR " 0b, %l[label], %0\n"

arch/sparc/include/asm/jump_label.h +1 -1

index 5080d16..ec2e2e2 100644
@@ -9,7 +9,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-               asm goto("1:\n\t"
+               asm_volatile_goto("1:\n\t"
                         "nop\n\t"
                         "nop\n\t"
                         ".pushsection __jump_table,  \"aw\"\n\t"

arch/x86/include/asm/cpufeature.h +3 -3

index d3f5c63..89270b4 100644
@@ -374,7 +374,7 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
                 * Catch too early usage of this before alternatives
                 * have run.
                 */
-               asm goto("1: jmp %l[t_warn]\n"
+               asm_volatile_goto("1: jmp %l[t_warn]\n"
                         "2:\n"
                         ".section .altinstructions,\"a\"\n"
                         " .long 1b - .\n"

@@ -388,7 +388,7 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
 
 #endif
 
-               asm goto("1: jmp %l[t_no]\n"
+               asm_volatile_goto("1: jmp %l[t_no]\n"
                         "2:\n"
                         ".section .altinstructions,\"a\"\n"
                         " .long 1b - .\n"

@@ -453,7 +453,7 @@ static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
  * have. Thus, we force the jump to the widest, 4-byte, signed relative
  * offset even though the last would often fit in less bytes.
  */
-               asm goto("1: .byte 0xe9\n .long %l[t_dynamic] - 2f\n"
+               asm_volatile_goto("1: .byte 0xe9\n .long %l[t_dynamic] - 2f\n"
                         "2:\n"
                         ".section .altinstructions,\"a\"\n"
                         " .long 1b - .\n"              /* src offset */

arch/x86/include/asm/jump_label.h +1 -1

index 64507f3..6a2cefb 100644
@@ -18,7 +18,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-       asm goto("1:"
+       asm_volatile_goto("1:"
                ".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t"
                ".pushsection __jump_table,  \"aw\" \n\t"
                _ASM_ALIGN "\n\t"

arch/x86/include/asm/mutex_64.h +2 -2

index e7e6751..07537a4 100644
@@ -20,7 +20,7 @@
 static inline void __mutex_fastpath_lock(atomic_t *v,
                                         void (*fail_fn)(atomic_t *))
 {
-       asm volatile goto(LOCK_PREFIX "   decl %0\n"
+       asm_volatile_goto(LOCK_PREFIX "   decl %0\n"
                          "   jns %l[exit]\n"
                          : : "m" (v->counter)
                          : "memory", "cc"

@@ -75,7 +75,7 @@ static inline int __mutex_fastpath_lock_retval(atomic_t *count)
 static inline void __mutex_fastpath_unlock(atomic_t *v,
                                           void (*fail_fn)(atomic_t *))
 {
-       asm volatile goto(LOCK_PREFIX "   incl %0\n"
+       asm_volatile_goto(LOCK_PREFIX "   incl %0\n"
                          "   jg %l[exit]\n"
                          : : "m" (v->counter)
                          : "memory", "cc"

include/linux/compiler-gcc4.h +15 -1

index 842de22..ded4299 100644
@@ -65,6 +65,21 @@
 #define __visible __attribute__((externally_visible))
 #endif
 
+/*
+ * GCC 'asm goto' miscompiles certain code sequences:
+ *
+ *   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
+ *
+ * Work it around via a compiler barrier quirk suggested by Jakub Jelinek.
+ * Fixed in GCC 4.8.2 and later versions.
+ *
+ * (asm goto is automatically volatile - the naming reflects this.)
+ */
+#if GCC_VERSION <= 40801
+# define asm_volatile_goto(x...)       do { asm goto(x); asm (""); } while (0)
+#else
+# define asm_volatile_goto(x...)       do { asm goto(x); } while (0)
+#endif
 
 #ifdef CONFIG_ARCH_USE_BUILTIN_BSWAP
 #if GCC_VERSION >= 40400
 
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

If I change the GCC_VERSION check for the asm_volatile_goto quirk to include 4.8.2, then KVM guests are properly cleaned up.

So is there some other 'asm goto' bug we haven't yet fully uncovered and reported to GCC upstream?

I started noticing problems with KVM guest destruction on Linux 3.12+, where guest memory wasn't being cleaned up. I bisected it down to the commit introducing the new 'asm goto'-based atomics, and found this quirk was later applied to those.

Unfortunately, even with GCC 4.8.2 (which ostensibly fixed the known 'asm goto' bug) I am still getting some kind of miscompilation. If I enable the asm_volatile_goto quirk for my compiler, KVM guests are destroyed correctly and the memory is cleaned up.

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
Cc: Ingo Molnar <[email protected]>
Cc: Linus Torvalds <[email protected]>

Signed-off-by Steven Noonan <[email protected]>

---
 include/linux/compiler-gcc4.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

include/linux/compiler-gcc4.h +1 -5

index ded4299..2507fd2 100644
@@ -75,11 +75,7 @@
  *
  * (asm goto is automatically volatile - the naming reflects this.)
  */
-#if GCC_VERSION <= 40801
-# define asm_volatile_goto(x...)	do { asm goto(x); asm (""); } while (0)
-#else
-# define asm_volatile_goto(x...)	do { asm goto(x); } while (0)
-#endif
+#define asm_volatile_goto(x...)	do { asm goto(x); asm (""); } while (0)
 
 #ifdef CONFIG_ARCH_USE_BUILTIN_BSWAP
 #if GCC_VERSION >= 40400

Commit-ID:  a9f180345f5378ac87d80ed0bea55ba421d83859
Gitweb:     http://git.kernel.org/tip/a9f180345f5378ac87d80ed0bea55ba421d83859
Author:     Steven Noonan <[email protected]>
AuthorDate: Wed, 12 Feb 2014 23:01:07 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 13 Feb 2014 12:34:05 +0100

compiler/gcc4: Make quirk for asm_volatile_goto() unconditional

I started noticing problems with KVM guest destruction on Linux 3.12+, where guest memory wasn't being cleaned up. I bisected it down to the commit introducing the new 'asm goto'-based atomics, and found this quirk was later applied to those.

Unfortunately, even with GCC 4.8.2 (which ostensibly fixed the known 'asm goto' bug) I am still getting some kind of miscompilation. If I enable the asm_volatile_goto quirk for my compiler, KVM guests are destroyed correctly and the memory is cleaned up.

Quoted 14 lines · tip-bot for Steven Noonan

Commit-ID:  a9f180345f5378ac87d80ed0bea55ba421d83859
Gitweb:     http://git.kernel.org/tip/a9f180345f5378ac87d80ed0bea55ba421d83859
Author:     Steven Noonan <[email protected]>
AuthorDate: Wed, 12 Feb 2014 23:01:07 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 13 Feb 2014 12:34:05 +0100

compiler/gcc4: Make quirk for asm_volatile_goto() unconditional

I started noticing problems with KVM guest destruction on Linux 3.12+, where guest memory wasn't being cleaned up. I bisected it down to the commit introducing the new 'asm goto'-based atomics, and found this quirk was later applied to those.

Quoted 5 lines

Unfortunately, even with GCC 4.8.2 (which ostensibly fixed the known 'asm goto' bug) I am still getting some kind of miscompilation. If I enable the asm_volatile_goto quirk for my compiler, KVM guests are destroyed correctly and the memory is cleaned up.

BTW, which exact 4.8.2 were you using?
The last known asm goto bug has been fixed on October, 10th, 2013:
http://gcc.gnu.org/PR58670
so before the October, 16th, 2013 4.8.2 release. But already since May 31th, 2013 the tip of the 4.8 GCC branch has been announcing itself as 4.8.2 prerelease. While some distribution versions of GCC announce themselves as the new version only starting from the release date, i.e. snapshots in between 4.8.1 release and 4.8.2 release announce themselves as 4.8.1, in other distributions or upstream it announces itself as 4.8.2. So, if you are using the latter and a snapshot in between May 31th, 2013 and October, 10th, 2013, then you could see gcc patchlevel 2, yet have a gcc with that bug unfixed.
So, if the kernel doesn't use a runtime test/configure test to check for this issue, but instead just relies on the patchlevel version, the only safe way would be to look for GCC >= 4.9 or GCC 4.8 with patchlevel > 2 rather than > 1.

Jakub

Quoted 14 lines · tip-bot for Steven Noonan

Commit-ID:  a9f180345f5378ac87d80ed0bea55ba421d83859
Gitweb:     http://git.kernel.org/tip/a9f180345f5378ac87d80ed0bea55ba421d83859
Author:     Steven Noonan <[email protected]>
AuthorDate: Wed, 12 Feb 2014 23:01:07 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 13 Feb 2014 12:34:05 +0100

compiler/gcc4: Make quirk for asm_volatile_goto() unconditional

I started noticing problems with KVM guest destruction on Linux 3.12+, where guest memory wasn't being cleaned up. I bisected it down to the commit introducing the new 'asm goto'-based atomics, and found this quirk was later applied to those.

Quoted 5 lines

Unfortunately, even with GCC 4.8.2 (which ostensibly fixed the known 'asm goto' bug) I am still getting some kind of miscompilation. If I enable the asm_volatile_goto quirk for my compiler, KVM guests are destroyed correctly and the memory is cleaned up.

BTW, which exact 4.8.2 were you using?
The last known asm goto bug has been fixed on October, 10th, 2013:
http://gcc.gnu.org/PR58670

so before the October, 16th, 2013 4.8.2 release. But already since May 31th, 2013 the tip of the 4.8 GCC branch has been announcing itself as 4.8.2 prerelease. While some distribution versions of GCC announce themselves as the new version only starting from the release date, i.e. snapshots in between 4.8.1 release and 4.8.2 release announce themselves as 4.8.1, in other distributions or upstream it announces itself as 4.8.2. So, if you are using the latter and a snapshot in between May 31th, 2013 and October, 10th, 2013, then you could see gcc patchlevel 2, yet have a gcc with that bug unfixed.
So, if the kernel doesn't use a runtime test/configure test to check for this issue, but instead just relies on the patchlevel version, the only safe way would be to look for GCC >= 4.9 or GCC 4.8 with patchlevel > 2 rather than > 1.

Jakub
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Jakub, any suggestions to how Steven might be able to pinpoint where the code generation problem lies?

Quoted 3 lines · Steven Noonan

If I change the GCC_VERSION check for the asm_volatile_goto quirk to include 4.8.2, then KVM guests are properly cleaned up.

Ok, I guess that means we should just make the quirk unconditional.

Ingo, do you want to do that or should I?