Message ID | 20170213033536.8908-1-cyrilbur@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Hi Cyril, On Mon, Feb 13, 2017 at 02:35:36PM +1100, Cyril Bur wrote: > A bug in the -02 optimisation of GCC 5.4 6.1 and 6.2 causes > setup_command_line() to not pass the correct first argument to strcpy > and therefore not actually copy the command_line. There is no such thing as an "-O2 optimisation". > At the time of writing GCC 5.4 is the most recent and is affected. GCC > 6.3 contains the backported fix, has been tested and appears safe to > use. 6.3 is (of course) the newer release; 5.4 is a maintenance release of a compiler that is a year older. > +# - gcc-5.4, 6.1, 6.2 don't copy the command_line around correctly > + echo -n '*** GCC-5.4 6.1 6.2 have a bad -O2 optimisation ' ; \ > + echo 'which will cause lost command_line options (at least).' ; \ Maybe something more like "GCC 5.4, 6.1, and 6.2 have a bug that results in a kernel that does not boot. Please use GCC 6.3 or later.". Please mention the GCC PR # somewhere in the code, too? Segher
On Mon, 2017-02-13 at 09:44 -0600, Segher Boessenkool wrote: > Hi Cyril, > > On Mon, Feb 13, 2017 at 02:35:36PM +1100, Cyril Bur wrote: > > A bug in the -02 optimisation of GCC 5.4 6.1 and 6.2 causes > > setup_command_line() to not pass the correct first argument to strcpy > > and therefore not actually copy the command_line. > > There is no such thing as an "-O2 optimisation". Right, perhaps I should have phrased it as "One of the -O2 level optimisations of GCC 5.4, 6.1 and 6.2 causes setup_command_line() to not pass the correct first argument to strcpy and therefore not actually copy the command_line, -O1 does not have this problem." > > > At the time of writing GCC 5.4 is the most recent and is affected. GCC > > 6.3 contains the backported fix, has been tested and appears safe to > > use. > > 6.3 is (of course) the newer release; 5.4 is a maintenance release of > a compiler that is a year older. Yes. I think the point I was trying to make is that since they backported the fix to 5.x and 6.x then I expect that 5.5 will have the fix but since it doesn't exist yet, I can't be sure. I'll add something to that effect. > > > +# - gcc-5.4, 6.1, 6.2 don't copy the command_line around correctly > > + echo -n '*** GCC-5.4 6.1 6.2 have a bad -O2 optimisation ' ; \ > > + echo 'which will cause lost command_line options (at least).' ; \ > > Maybe something more like > > "GCC 5.4, 6.1, and 6.2 have a bug that results in a kernel that does > not boot. Please use GCC 6.3 or later.". "that may not boot" is more accurate, if it can boot without a command_line param it might just do so. > > Please mention the GCC PR # somewhere in the code, too? > Sure. Thanks, Cyril > > Segher
On Tue, Feb 14, 2017 at 11:25:43AM +1100, Cyril Bur wrote: > > > At the time of writing GCC 5.4 is the most recent and is affected. GCC > > > 6.3 contains the backported fix, has been tested and appears safe to > > > use. > > > > 6.3 is (of course) the newer release; 5.4 is a maintenance release of > > a compiler that is a year older. > > Yes. I think the point I was trying to make is that since they > backported the fix to 5.x and 6.x then I expect that 5.5 will have the > fix but since it doesn't exist yet, I can't be sure. I'll add something > to that effect. The patch has been backported to the GCC 5 branch; it will be part of any future GCC 5 release (5.5 and later, if any later will happen; 5.5 will). Don't be so unsure about these things, we aren't *that* incompetent ;-) > > Please mention the GCC PR # somewhere in the code, too? > > Sure. Thanks! Segher
On Mon, 13 Feb 2017 19:49:16 -0600 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Tue, Feb 14, 2017 at 11:25:43AM +1100, Cyril Bur wrote: > > > > At the time of writing GCC 5.4 is the most recent and is > > > > affected. GCC 6.3 contains the backported fix, has been tested > > > > and appears safe to use. > > > > > > 6.3 is (of course) the newer release; 5.4 is a maintenance > > > release of a compiler that is a year older. > > > > Yes. I think the point I was trying to make is that since they > > backported the fix to 5.x and 6.x then I expect that 5.5 will have > > the fix but since it doesn't exist yet, I can't be sure. I'll add > > something to that effect. > > The patch has been backported to the GCC 5 branch; it will be part of > any future GCC 5 release (5.5 and later, if any later will happen; 5.5 > will). > > Don't be so unsure about these things, we aren't *that* > incompetent ;-) Nonetheless the message is factually correct. > > > > Please mention the GCC PR # somewhere in the code, too? > > > > Sure. It seems this has never happened? Anyway, having the bug number in the text reported to user seems excessive. So far the bug number has been mentioned only in the commit messages and it looks fine to me. You can always look it up if you really want. Thanks Michal
Michal Suchánek <msuchanek@suse.de> writes: > On Mon, 13 Feb 2017 19:49:16 -0600 > Segher Boessenkool <segher@kernel.crashing.org> wrote: > >> On Tue, Feb 14, 2017 at 11:25:43AM +1100, Cyril Bur wrote: >> > > > At the time of writing GCC 5.4 is the most recent and is >> > > > affected. GCC 6.3 contains the backported fix, has been tested >> > > > and appears safe to use. >> > > >> > > 6.3 is (of course) the newer release; 5.4 is a maintenance >> > > release of a compiler that is a year older. >> > >> > Yes. I think the point I was trying to make is that since they >> > backported the fix to 5.x and 6.x then I expect that 5.5 will have >> > the fix but since it doesn't exist yet, I can't be sure. I'll add >> > something to that effect. >> >> The patch has been backported to the GCC 5 branch; it will be part of >> any future GCC 5 release (5.5 and later, if any later will happen; 5.5 >> will). >> >> Don't be so unsure about these things, we aren't *that* >> incompetent ;-) > > Nonetheless the message is factually correct. > >> >> > > Please mention the GCC PR # somewhere in the code, too? >> > >> > Sure. > > It seems this has never happened? From memory we discovered that there were patched versions of the compilers in distros, which still reported those version numbers. ie. the patch would break working compilers. So Cyril was going to write a test that actually built some code and checked the generated asm. But I suspect he never got around to it. cheers
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 31286fa7873c..db5d8dabf1ca 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -381,6 +381,7 @@ TOUT := .tmp_gas_check # - gcc-3.4 and binutils-2.14 are a fatal combination # - Require gcc 4.0 or above on 64-bit # - gcc-4.2.0 has issues compiling modules on 64-bit +# - gcc-5.4, 6.1, 6.2 don't copy the command_line around correctly checkbin: @if test "$(cc-name)" != "clang" \ && test "$(cc-version)" = "0304" ; then \ @@ -414,6 +415,16 @@ checkbin: echo -n '*** Please use a different binutils version.' ; \ false ; \ fi + @if test "x${CONFIG_CPU_LITTLE_ENDIAN}" = "xy" \ + && { test "$(cc-version)" = "0504" \ + || test "$(cc-version)" = "0601" \ + || test "$(cc-version)" = "0602" ; } ; then \ + echo -n '*** GCC-5.4 6.1 6.2 have a bad -O2 optimisation ' ; \ + echo 'which will cause lost command_line options (at least).' ; \ + echo '*** Please use a different GCC version.' ; \ + false ; \ + fi + CLEAN_FILES += $(TOUT)
A bug in the -02 optimisation of GCC 5.4 6.1 and 6.2 causes setup_command_line() to not pass the correct first argument to strcpy and therefore not actually copy the command_line. A workaround patch was proposed: http://patchwork.ozlabs.org/patch/673130/ some discussion ensued. A GCC bug was raised: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71709 The bug has been fixed in 7.0 and backported to GCC 5 and GCC 6. At the time of writing GCC 5.4 is the most recent and is affected. GCC 6.3 contains the backported fix, has been tested and appears safe to use. Heavy-lifting-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> Signed-off-by: Cyril Bur <cyrilbur@gmail.com> --- v2: Added check to only blacklist compilers on little-endian arch/powerpc/Makefile | 11 +++++++++++ 1 file changed, 11 insertions(+)