compiler.h: Raise minimum version of GCC to 5.1 for arm64
PATCH 1/1 compiler.h: Raise minimum version of GCC to 5.1 for arm64
GCC versions >= 4.9 and < 5.1 have been shown to emit memory references beyond the stack pointer, resulting in memory corruption if an interrupt is taken after the stack pointer has been adjusted but before the reference has been executed. This leads to subtle, infrequent data corruption such as the EXT4 problems reported by Russell King at the link below.
Life is too short for buggy compilers, so raise the minimum GCC version required by arm64 to 5.1.
--- include/linux/compiler-gcc.h | 6 ++++++ 1 file changed, 6 insertions(+)
include/linux/compiler-gcc.h +6 -0
@@ -13,6 +13,12 @@/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */ #if GCC_VERSION <40900 # error Sorry, your version of GCC is too old - please use 4.9 or newer. +#elif defined(CONFIG_ARM64 ) &&GCC_VERSION <50100 +/* +* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 +* https://lore.kernel.org/r/[email protected] +*/ +# error Sorry, your version of GCC is too old - please use 5.1 or newer.#endif /*
Signature · 1 line
2.30.0.284.gd98b1dd5eaa7-goog
Quoted 11 lines · Will Deacon
GCC versions >= 4.9 and < 5.1 have been shown to emit memory references beyond the stack pointer, resulting in memory corruption if an interrupt is taken after the stack pointer has been adjusted but before the reference has been executed. This leads to subtle, infrequent data corruption such as the EXT4 problems reported by Russell King at the link below.
Life is too short for buggy compilers, so raise the minimum GCC version required by arm64 to 5.1.
Quoted 10 lines
Cc Theodore Ts'o <[email protected]>Cc Catalin Marinas <[email protected]>Cc Florian Weimer <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Linus Torvalds <[email protected]>Cc Nick Desaulniers <[email protected]>Reported-by Russell King <[email protected]>Suggested-by Arnd Bergmann <[email protected]>Signed-off-by Will Deacon <[email protected]>
Quoted 24 lines
--- include/linux/compiler-gcc.h | 6 ++++++ 1 file changed, 6 insertions(+)include/linux/compiler-gcc.h +6
-1@@ -13,6 +13,12 @@/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */ #if GCC_VERSION <40900 # error Sorry, your version of GCC is too old - please use 4.9 or newer. +#elif defined(CONFIG_ARM64 ) &&GCC_VERSION <50100 +/* +* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 +* https://lore.kernel.org/r/[email protected] +*/ +# error Sorry, your version of GCC is too old - please use 5.1 or newer.#endif /* -- 2.30.0.284.gd98b1dd5eaa7-goog
Signature · 2 lines
Thanks, ~Nick Desaulniers
Quoted 10 lines · Will Deacon
GCC versions >= 4.9 and < 5.1 have been shown to emit memory references beyond the stack pointer, resulting in memory corruption if an interrupt is taken after the stack pointer has been adjusted but before the reference has been executed. This leads to subtle, infrequent data corruption such as the EXT4 problems reported by Russell King at the link below.
Life is too short for buggy compilers, so raise the minimum GCC version required by arm64 to 5.1.
Quoted 10 lines
Cc Theodore Ts'o <[email protected]>Cc Catalin Marinas <[email protected]>Cc Florian Weimer <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Linus Torvalds <[email protected]>Cc Nick Desaulniers <[email protected]>Reported-by Russell King <[email protected]>Suggested-by Arnd Bergmann <[email protected]>Signed-off-by Will Deacon <[email protected]>
Quoted 24 lines
--- include/linux/compiler-gcc.h | 6 ++++++ 1 file changed, 6 insertions(+)include/linux/compiler-gcc.h +6
-0@@ -13,6 +13,12 @@/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */ #if GCC_VERSION <40900 # error Sorry, your version of GCC is too old - please use 4.9 or newer. +#elif defined(CONFIG_ARM64 ) &&GCC_VERSION <50100 +/* +* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 +* https://lore.kernel.org/r/[email protected] +*/ +# error Sorry, your version of GCC is too old - please use 5.1 or newer.#endif /* Signature · 1 line
2.30.0.284.gd98b1dd5eaa7-goog
Quoted 3 lines · Will Deacon
Life is too short for buggy compilers, so raise the minimum GCC version required by arm64 to 5.1.
Ack. I'll assume I get this the usual ways from the arm64 tree..
Linus
Quoted 2 lines · Linus Torvalds
Ack. I'll assume I get this the usual ways from the arm64 tree..
Oh.. Actually, while you can use my ack if you decide to go this way, I do wonder if it might not be better to introduce a notion of an error at Kconfig time, and then we could make this whole GCC_VERSION check be something that gets covered much earlier - when configuring the kernel, rather than randomly (ok, very early) when building it.
We already have the CONFIG_GCC_VERSION config variable, after all.
And Kconfig already has an error functionality, which it uses for things like compilers not found etc.
So something like
$(error-if,CC_IS_GCC && GCC_VERSION < 90100,"Gcc version too old")
in the arm64 Kconfig file should do it.
Adding Masahiro, because I couldn't actually get it to work. I'm probably doing something wrong, but it might also be that it cannot depend on config variables currently (our only use is for running shell script tests unconditionally).
Linus
Quoted 13 lines · Linus Torvalds
Quoted 2 lines · Linus Torvalds
Ack. I'll assume I get this the usual ways from the arm64 tree..
Oh.. Actually, while you can use my ack if you decide to go this way, I do wonder if it might not be better to introduce a notion of an error at
Kconfigtime, and then we could make this wholeGCC_VERSIONcheck be something that gets covered much earlier - when configuring the kernel, rather than randomly (ok, very early) when building it.We already have the
CONFIG_GCC_VERSIONconfig variable, after all.
Quoted 8 lines
And
Kconfigalready has an error functionality, which it uses for things like compilers not found etc.So something like
$(error-if,CC_IS_GCC && GCC_VERSION < 90100,"Gcc version too old")in the arm64
Kconfigfile should do it.
$(error-if) seems to expect a y/n as a condition. We do have $(failure) and $(success) but they translate a (shell) command's return code to y/n. Even with something like:
config GCC_IS_OLD
def_bool CC_IS_GCC && GCC_VERSION < ...
I can't get $(error-if,GCC_IS_OLD) to expand the config value, no matter what other. GCC_VERSION is also a config option in your example.
I'll queue Will's patch in the meantime.
Signature · 1 line
Catalin
Catalin Marinas
AppliedQuoted 8 lines · Will Deacon
GCC versions >= 4.9 and < 5.1 have been shown to emit memory references beyond the stack pointer, resulting in memory corruption if an interrupt is taken after the stack pointer has been adjusted but before the reference has been executed. This leads to subtle, infrequent data corruption such as the EXT4 problems reported by Russell King at the link below.
[...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] compiler.h: Raise minimum version of GCC to 5.1 for arm64
https://git.kernel.org/arm64/c/1f1244a5ddb7
Signature · 1 line
Catalin
Quoted 11 lines · Catalin Marinas
Quoted 8 lines · Will Deacon
GCC versions >= 4.9 and < 5.1 have been shown to emit memory references beyond the stack pointer, resulting in memory corruption if an interrupt is taken after the stack pointer has been adjusted but before the reference has been executed. This leads to subtle, infrequent data corruption such as the EXT4 problems reported by Russell King at the link below.
[...]
Quoted 8 lines
Applied to arm64 (for-next/fixes), thanks!
[1/1]
compiler.h: Raise minimum version of GCC to 5.1 for arm64https://git.kernel.org/arm64/c/1f1244a5ddb7--
Catalin
Maybe, we can raise the minimal version to gcc 5.1 for all architectures.
Signature · 2 lines
Best Regards Masahiro Yamada
Quoted 10 lines · Masahiro Yamada
Quoted 9 lines · Catalin Marinas
Quoted 7 lines · Will Deacon
GCC versions >= 4.9 and < 5.1 have been shown to emit memory references beyond the stack pointer, resulting in memory corruption if an interrupt is taken after the stack pointer has been adjusted but before the reference has been executed. This leads to subtle, infrequent data corruption such as the EXT4 problems reported by Russell King at the link below.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
[...]
Applied to arm64 (for-next/fixes), thanks!
[1/1]
compiler.h: Raise minimum version of GCC to 5.1 for arm64
https://git.kernel.org/arm64/c/1f1244a5ddb7
[]
Quoted 2 lines
Maybe, we can raise the minimal version to gcc 5.1 for all architectures.
If really raising the required minimum, maybe use 7.1 so kasan v5 and fallthrough are always enabled too.
https://gcc.gnu.org/releases.html
GCC 7.1 May 2, 2017
GCC 5.1 April 22, 2015
GCC 4.9.0 April 22, 2014
Quoted 3 lines · Masahiro Yamada
Maybe, we can raise the minimal version to gcc 5.1 for all architectures.
It was discussed, but the immediate reason for this thing really does seem to be specific to just arm64 (ie this is not some generic gcc stack access bug that just happens to rear its head on arm64 - the patch to fix this in the gcc bugzilla is very much arm64-only).
Linus
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · Masahiro Yamada
Maybe, we can raise the minimal version to gcc 5.1 for all architectures.
It was discussed, but the immediate reason for this thing really does seem to be specific to just arm64 (ie this is not some generic gcc stack access bug that just happens to rear its head on arm64 - the patch to fix this in the gcc bugzilla is very much arm64-only).
I seem to remember during the discussion of the arm64 problem, that there was a similar bug on e.g. sparc, but they patched the kernel. *digs through irc logs...*
https://patchwork.kernel.org/project/linux-crypto/patch/[email protected]/
https://marc.info/?l=linux-sparc&m=149636946609980&w=2
(and they even reference the arm64 bug). If you move on two messages, then the disassembly clearly shows that it is the same bug on Sparc.
DaveM came up with the following to fix it:
commit d41519a69b35b10af7fda867fb9100df24fdf403
Author: David Miller <[email protected]> Date: Fri Jun 2 11:28:54 2017 -0400
crypto: Work around deallocated stack frame reference gcc bug on sparc.
which added a bunch of barriers across the kernel to cater for this, but for them, ext4 was not impacted at that time.
Apparently, davem's justification for not changing ext4 was:
"Actually, ext4 doesn't trigger the problem because the on-stack object used in ext4 is a fixed size at compile time"
So, maybe the Sparc issue was just a similar but different bug in gcc 4.9.x.
Signature · 7 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 3 lines · Russell King - ARM Linux admin
So, maybe the Sparc issue was just a similar but different bug in gcc 4.9.x.
Good catch. And I know this bug has happened independently on different architectures several times (I remember this on x86-64 as well), so I started looking around.
And in fact, 4.9 was buggy on x86-64 too:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904
And yeah, _that_ gcc bug wasn't actually x86-64 specific, but apparently a generic instruction scheduling bug.
So it's an independent bug, but I do have to admit that the arguments against 4.9 are piling up (even if that particular fix apparently got fixed in the gcc branches and apparently backported to distro compilers too).
Linus
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 15 lines · Linus Torvalds
Quoted 3 lines · Russell King - ARM Linux admin
So, maybe the Sparc issue was just a similar but different bug in gcc 4.9.x.
Good catch. And I know this bug has happened independently on different architectures several times (I remember this on x86-64 as well), so I started looking around.
And in fact, 4.9 was buggy on x86-64 too:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904
Quoted 8 lines
And yeah, _that_ gcc bug wasn't actually x86-64 specific, but apparently a generic instruction scheduling bug.
So it's an independent bug, but I do have to admit that the arguments against 4.9 are piling up (even if that particular fix apparently got fixed in the gcc branches and apparently backported to distro compilers too).
So if the arguments are piling up, what is holding us back, other than inertia? RHEL 7 used to be a factor, but it ships with 4.8 not 4.9, so its users already need to upgrade. Is anyone aware of a good reason to keep 4.9 supported? Are any other long term supported distros using 4.9.x?
I know that distros probably backported fixes for all of these issues, but without a way to interrogate the compiler about this, that doesn't really make a difference IMHO.
Note that banning 4.9 for arm64 and banning it in general should be two different changes in any case, as the former will need to be backported to -stable kernels as well.
--
Ard.
Quoted 3 lines · Ard Biesheuvel
So if the arguments are piling up, what is holding us back, other than inertia?
I think we can most certainly just try increasing the minimum version to 5.1 in the next merge window and see.
Quoted 3 lines
Note that banning 4.9 for arm64 and banning it in general should be two different changes in any case, as the former will need to be backported to -stable kernels as well.
Yes. The arm64 issue is a clear and known bug, plus I suspect gcc-4.9 is ridiculously old in the arm64 ecosystem anyway.
So the arm64 issue is a bug-fix, the follow-up of just upgrading gcc requirements in general would be a "keep up with the times, and allow those variable declarations in loops".
Linus
Quoted 12 lines · Linus Torvalds
Quoted 3 lines · Ard Biesheuvel
So if the arguments are piling up, what is holding us back, other than inertia?
I think we can most certainly just try increasing the minimum version to 5.1 in the next merge window and see.
Quoted 3 lines
Note that banning 4.9 for arm64 and banning it in general should be two different changes in any case, as the former will need to be backported to -stable kernels as well.
Quoted 6 lines
Yes. The arm64 issue is a clear and known bug, plus I suspect gcc-4.9 is ridiculously old in the arm64 ecosystem anyway.
So the arm64 issue is a bug-fix, the follow-up of just upgrading gcc requirements in general would be a "keep up with the times, and allow those variable declarations in loops".
Given the upgrade requirement, and how clang version requirements constantly change, how much more difficult would it be for others to use gcc 7.1 or higher now instead of later?
Quoted 4 lines · Joe Perches
Given the upgrade requirement, and how clang version requirements constantly change, how much more difficult would it be for others to use gcc 7.1 or higher now instead of later?
What was the argument for jumping all the way to gcc-7.1?
I do think we want to have real reasons we can point to, rather than a "just because".
Linus
Quoted 10 lines · Linus Torvalds
Quoted 4 lines · Joe Perches
Given the upgrade requirement, and how clang version requirements constantly change, how much more difficult would it be for others to use gcc 7.1 or higher now instead of later?
What was the argument for jumping all the way to gcc-7.1?
I do think we want to have real reasons we can point to, rather than a "just because".
KASAN v5 instead of all the old versions
gcc 7.1 supports fallthrough.
Probably more, but those might be sufficient.
Quoted 3 lines · Joe Perches
KASAN v5 instead of all the old versions
gcc 7.1 supports fallthrough.
Considering that most people won't even enable KASAN, I think that's not a huge reason to then force people to potentially upgrade their compilers.
That said, I do think that it might be good to just try to standardize on the KASAN v5 code - but that could easily be done by simply making KASAN depend as a feature on having a newer compiler version.
So the KASAN option itself could just have something like
depends on !CC_IS_GCC || GCC_VERSION >= 70000
in it, which would allow us to just say "only v5 need apply".
As to "fallthrough", I think it's more than enough that lots of people build with compilers that support it, and then any warnings they see will be fixed. No need to force upgrades - it doesn't buy kernel developers anything.
Linus
Hi Linus,
Quoted 8 lines · Linus Torvalds
Quoted 3 lines · Ard Biesheuvel
So if the arguments are piling up, what is holding us back, other than inertia?
I think we can most certainly just try increasing the minimum version to 5.1 in the next merge window and see.
Just a friendly reminder.
It will be nice to increase the minimum GCC version to GCC 5.x globally.
Now, the version check has been moved to scripts/cc-version.sh, which is invoked from Kconfig.
If you decide to do this, please update 'gcc_min_version' in that file.
I'd like to suggest GCC 5.2 instead of 5.1 so that we can drop the following line
from arch/powerpc/Kconfig.
if GCC_VERSION >= 50200 # plugin support on gcc <= 5.1 is buggy on PPC
Quoted 12 lines
Quoted 3 lines
Note that banning 4.9 for arm64 and banning it in general should be two different changes in any case, as the former will need to be backported to -stable kernels as well.
Yes. The arm64 issue is a clear and known bug, plus I suspect gcc-4.9 is ridiculously old in the arm64 ecosystem anyway.
So the arm64 issue is a bug-fix, the follow-up of just upgrading gcc requirements in general would be a "keep up with the times, and allow those variable declarations in loops".
Linus
Signature · 2 lines
Best Regards Masahiro Yamada