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

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 [off-list ref]
Signed-off-by: Cyril Bur <redacted>
---
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

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:

quoted

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."

quoted

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.

quoted

+# - 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:

quoted

quoted

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

quoted

Please mention the GCC PR # somewhere in the code, too?

Sure.

Thanks!


Segher

On Mon, 13 Feb 2017 19:49:16 -0600
Segher Boessenkool [off-list ref] wrote:

On Tue, Feb 14, 2017 at 11:25:43AM +1100, Cyril Bur wrote:

quoted

quoted

quoted

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.

quoted

quoted

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=C3=A1nek [off-list ref] writes:

On Mon, 13 Feb 2017 19:49:16 -0600
Segher Boessenkool [off-list ref] wrote:

quoted

On Tue, Feb 14, 2017 at 11:25:43AM +1100, Cyril Bur wrote:

quoted

quoted

quoted

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.

quoted

=20

quoted

quoted

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