[PATCH v2] powerpc: Blacklist GCC 5.4 6.1 and 6.2

[PATCH v2] powerpc: Blacklist GCC 5.4 6.1 and 6.2

[Cyril Bur]

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(+)

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)
-- 
2.11.1

Re: [PATCH v2] powerpc: Blacklist GCC 5.4 6.1 and 6.2

[Segher Boessenkool]

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

Re: [PATCH v2] powerpc: Blacklist GCC 5.4 6.1 and 6.2

[Cyril Bur]

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

Re: [PATCH v2] powerpc: Blacklist GCC 5.4 6.1 and 6.2

[Segher Boessenkool]

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

Re: [PATCH v2] powerpc: Blacklist GCC 5.4 6.1 and 6.2

[Michal Suchánek]

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

Re: [PATCH v2] powerpc: Blacklist GCC 5.4 6.1 and 6.2

[Michael Ellerman]

Michal Such=C3=A1nek <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.=20=20
>> > >=20
>> > > 6.3 is (of course) the newer release; 5.4 is a maintenance
>> > > release of a compiler that is a year older.=20=20
>> >=20
>> > 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.=20=20
>>=20
>> 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).
>>=20
>> Don't be so unsure about these things, we aren't *that*
>> incompetent ;-)
>
> Nonetheless the message is factually correct.
>
>>=20
>> > > Please mention the GCC PR # somewhere in the code, too?=20=20
>> >=20
>> > Sure.=20=20
>
> 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