From: Anton Blanchard <hidden> Date: 2017-03-04 23:54:37
From: Anton Blanchard <redacted>
I see a panic in early boot when building with a recent gcc toolchain.
The issue is a divide by zero, which is undefined. Older toolchains
let us get away with it:
int foo(int a) { return a / 0; }
foo:
li 9,0
divw 3,3,9
extsw 3,3
blr
But newer ones catch it:
foo:
trap
Add a check to avoid the divide by zero.
Fixes: bd067f83b084 ("powerpc/64: Fix naming of cache block vs. cache line")
Signed-off-by: Anton Blanchard <redacted>
---
arch/powerpc/kernel/setup_64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2017-03-05 00:25:29
On Sun, 2017-03-05 at 10:54 +1100, Anton Blanchard wrote:
From: Anton Blanchard <redacted>
I see a panic in early boot when building with a recent gcc
toolchain.
The issue is a divide by zero, which is undefined. Older toolchains
let us get away with it:
Maybe we should panic though ... not having a valid cache block size is
going to be fatal in other areas...
quoted hunk
int foo(int a) { return a / 0; }
foo:
li 9,0
divw 3,3,9
extsw 3,3
blr
But newer ones catch it:
foo:
trap
Add a check to avoid the divide by zero.
Fixes: bd067f83b084 ("powerpc/64: Fix naming of cache block vs. cache
line")
Signed-off-by: Anton Blanchard <redacted>
---
arch/powerpc/kernel/setup_64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/setup_64.c
b/arch/powerpc/kernel/setup_64.c
index adf2084..afd1c26 100644
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2017-03-05 00:27:25
On Sun, 2017-03-05 at 11:25 +1100, Benjamin Herrenschmidt wrote:
On Sun, 2017-03-05 at 10:54 +1100, Anton Blanchard wrote:
quoted
From: Anton Blanchard <redacted>
I see a panic in early boot when building with a recent gcc
toolchain.
The issue is a divide by zero, which is undefined. Older toolchains
let us get away with it:
Maybe we should panic though ... not having a valid cache block size
is going to be fatal in other areas...
... Unless it's for L2/L3 caches. Of course...
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
quoted
int foo(int a) { return a / 0; }
foo:
li 9,0
divw 3,3,9
extsw 3,3
blr
But newer ones catch it:
foo:
trap
Add a check to avoid the divide by zero.
Fixes: bd067f83b084 ("powerpc/64: Fix naming of cache block vs.
cache
line")
Signed-off-by: Anton Blanchard <redacted>
---
arch/powerpc/kernel/setup_64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/setup_64.c
b/arch/powerpc/kernel/setup_64.c
index adf2084..afd1c26 100644
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-03-05 10:26:52
Anton Blanchard [off-list ref] writes:
From: Anton Blanchard <redacted>
I see a panic in early boot when building with a recent gcc toolchain.
The issue is a divide by zero, which is undefined. Older toolchains
let us get away with it:
int foo(int a) { return a / 0; }
foo:
li 9,0
divw 3,3,9
extsw 3,3
blr
But newer ones catch it:
foo:
trap
Add a check to avoid the divide by zero.
Erk sorry. One of the static checkers spotted it, but I hadn't got
around to fixing it because it seemed to not actually blow up, guess
not.
cheers
On Sun, Mar 05, 2017 at 09:26:47PM +1100, Michael Ellerman wrote:
quoted
I see a panic in early boot when building with a recent gcc toolchain.
The issue is a divide by zero, which is undefined. Older toolchains
let us get away with it:
int foo(int a) { return a / 0; }
foo:
li 9,0
divw 3,3,9
extsw 3,3
blr
But newer ones catch it:
foo:
trap
Add a check to avoid the divide by zero.
Erk sorry. One of the static checkers spotted it, but I hadn't got
around to fixing it because it seemed to not actually blow up, guess
not.
The PowerPC divw etc. instructions do not trap by themselves, but recent
GCC inserts trap instructions on code paths that are always undefined
behaviour (like, dividing by zero).
Segher
From: Gabriel Paubert <hidden> Date: 2017-03-05 17:10:20
On Sun, Mar 05, 2017 at 06:37:37AM -0600, Segher Boessenkool wrote:
On Sun, Mar 05, 2017 at 09:26:47PM +1100, Michael Ellerman wrote:
quoted
quoted
I see a panic in early boot when building with a recent gcc toolchain.
The issue is a divide by zero, which is undefined. Older toolchains
let us get away with it:
int foo(int a) { return a / 0; }
foo:
li 9,0
divw 3,3,9
extsw 3,3
blr
But newer ones catch it:
foo:
trap
Add a check to avoid the divide by zero.
Erk sorry. One of the static checkers spotted it, but I hadn't got
around to fixing it because it seemed to not actually blow up, guess
not.
The PowerPC divw etc. instructions do not trap by themselves, but recent
GCC inserts trap instructions on code paths that are always undefined
behaviour (like, dividing by zero).
Is it systematic or does it depend from, e.g., optimization levels?
Is there anything in the standards about this feature?
Gabriel
On Sun, Mar 05, 2017 at 05:58:37PM +0100, Gabriel Paubert wrote:
quoted
quoted
Erk sorry. One of the static checkers spotted it, but I hadn't got
around to fixing it because it seemed to not actually blow up, guess
not.
The PowerPC divw etc. instructions do not trap by themselves, but recent
GCC inserts trap instructions on code paths that are always undefined
behaviour (like, dividing by zero).
Is it systematic or does it depend from, e.g., optimization levels?
In this case it needs -fisolate-erroneous-paths-dereference which is
default at -O2 and higher.
Is there anything in the standards about this feature?
The compiler can do whatever it likes with code that has undefined
behaviour. With this optimisation it a) can compile the conforming
code to something better; and b) undefined behaviour will trap instead
of doing something random (which often is exploitable).
Segher
From: Benjamin Herrenschmidt <hidden> Date: 2017-03-05 23:10:10
On Sun, 2017-03-05 at 11:24 -0600, Segher Boessenkool wrote:
On Sun, Mar 05, 2017 at 05:58:37PM +0100, Gabriel Paubert wrote:
quoted
quoted
quoted
Erk sorry. One of the static checkers spotted it, but I hadn't got
around to fixing it because it seemed to not actually blow up, guess
not.
The PowerPC divw etc. instructions do not trap by themselves, but recent
GCC inserts trap instructions on code paths that are always undefined
behaviour (like, dividing by zero).
Is it systematic or does it depend from, e.g., optimization levels?
In this case it needs -fisolate-erroneous-paths-dereference which is
default at -O2 and higher.
quoted
Is there anything in the standards about this feature?
The compiler can do whatever it likes with code that has undefined
behaviour. With this optimisation it a) can compile the conforming
code to something better; and b) undefined behaviour will trap instead
of doing something random (which often is exploitable).
I actually like that feature, except it did bite me once or twice in the past
adding traps to intentional NULL dereferences ;-) Ah the joys of writing
a firmware where you poke at stuff at fixed addresses in low memory :-)
Cheers,
Ben.
On Mon, Mar 06, 2017 at 10:09:01AM +1100, Benjamin Herrenschmidt wrote:
quoted
The compiler can do whatever it likes with code that has undefined
behaviour. With this optimisation it a) can compile the conforming
code to something better; and b) undefined behaviour will trap instead
of doing something random (which often is exploitable).
I actually like that feature,
Yeah, me too -- it also (currently) makes *smaller* code than it would
without it. Win-win-win.
except it did bite me once or twice in the past
adding traps to intentional NULL dereferences ;-) Ah the joys of writing
a firmware where you poke at stuff at fixed addresses in low memory :-)
You cannot really have something at address 0, the way NULL pointers
are represented in GCC. 0 in firmware, so *fun*, especially before the
CFAR was invented. "Something jumped to 0, CTR is 0 so it's probably
a BCTR, but which one of the 6000?"
What do you have at 0? Not anything you need often I hope?
Segher
From: Benjamin Herrenschmidt <hidden> Date: 2017-03-06 00:21:43
On Sun, 2017-03-05 at 18:10 -0600, Segher Boessenkool wrote:
You cannot really have something at address 0, the way NULL pointers
are represented in GCC. 0 in firmware, so *fun*, especially before
the
CFAR was invented. "Something jumped to 0, CTR is 0 so it's probably
a BCTR, but which one of the 6000?"
What do you have at 0? Not anything you need often I hope?
I think it was some kind of boot flag. I've had cases also of
copying the 0..0x100 region over with the kexec gunk etc...
Ben.
From: Gabriel Paubert <hidden> Date: 2017-03-06 12:04:06
On Sun, Mar 05, 2017 at 11:24:56AM -0600, Segher Boessenkool wrote:
On Sun, Mar 05, 2017 at 05:58:37PM +0100, Gabriel Paubert wrote:
quoted
quoted
quoted
Erk sorry. One of the static checkers spotted it, but I hadn't got
around to fixing it because it seemed to not actually blow up, guess
not.
The PowerPC divw etc. instructions do not trap by themselves, but recent
GCC inserts trap instructions on code paths that are always undefined
behaviour (like, dividing by zero).
Is it systematic or does it depend from, e.g., optimization levels?
In this case it needs -fisolate-erroneous-paths-dereference which is
default at -O2 and higher.
Great, another optimization-dependent behaviour. :-(
But this is not the most serious issue: on PPC, when you #include
<limits>, the numeric_limits<any_integer_type>::traps is false on PPC,
and on no other architecture that I know of (in practice this trap
reflects the hardware behaviour on division by zero).
By generating a trap in this case, I believe that the compiler violates
a contract given by <limits>, and the standard.
I'd certainly prefer a compile time warning, easily convertible to an
error.
quoted
Is there anything in the standards about this feature?
The compiler can do whatever it likes with code that has undefined
behaviour. With this optimisation it a) can compile the conforming
code to something better; and b) undefined behaviour will trap instead
of doing something random (which often is exploitable).
It may be undefined, but I believe that the numeric_limits<>::traps
value clearly prohibits generating a trap in this case.
Gabriel
On Mon, Mar 06, 2017 at 01:03:19PM +0100, Gabriel Paubert wrote:
quoted
quoted
quoted
The PowerPC divw etc. instructions do not trap by themselves, but recent
GCC inserts trap instructions on code paths that are always undefined
behaviour (like, dividing by zero).
Is it systematic or does it depend from, e.g., optimization levels?
In this case it needs -fisolate-erroneous-paths-dereference which is
default at -O2 and higher.
Great, another optimization-dependent behaviour. :-(
It makes the "behaviour" for undefined behaviour *less* surprising.
It does not change anything else: malformed programs stay malformed,
correct programs do exactly what they did before, too.
But this is not the most serious issue: on PPC, when you #include
<limits>, the numeric_limits<any_integer_type>::traps is false on PPC,
and on no other architecture that I know of (in practice this trap
reflects the hardware behaviour on division by zero).
By generating a trap in this case, I believe that the compiler violates
a contract given by <limits>, and the standard.
[ snip ]
I have no idea why you are bringing C++ into this. Please open a PR
if you think there is a bug in the C++ library.
I'll note that this cannot violate the standard, see the "terms and
definitions":
[defns.undefined]
undefined behavior
behavior for which this International Standard imposes no requirements
Segher
From: David Laight <hidden> Date: 2017-03-06 15:18:48
From: Segher Boessenkool
Sent: 06 March 2017 14:18
On Mon, Mar 06, 2017 at 01:03:19PM +0100, Gabriel Paubert wrote:
quoted
quoted
quoted
quoted
The PowerPC divw etc. instructions do not trap by themselves, but=
recent
quoted
quoted
quoted
quoted
GCC inserts trap instructions on code paths that are always undef=
ined
quoted
quoted
quoted
quoted
behaviour (like, dividing by zero).
Is it systematic or does it depend from, e.g., optimization levels?
In this case it needs -fisolate-erroneous-paths-dereference which is
default at -O2 and higher.
Great, another optimization-dependent behaviour. :-(
=20
It makes the "behaviour" for undefined behaviour *less* surprising.
It does not change anything else: malformed programs stay malformed,
correct programs do exactly what they did before, too.
Yep, 'undefined behaviour' is exactly that.
It doesn't mean 'undefined result', or 'maybe a signal'.
Wiping the disk and targeting the user with an ICBM are both valid.
David
From: Michael Ellerman <hidden> Date: 2017-03-08 07:25:58
On Sat, 2017-03-04 at 23:54:34 UTC, Anton Blanchard wrote:
From: Anton Blanchard <redacted>
I see a panic in early boot when building with a recent gcc toolchain.
The issue is a divide by zero, which is undefined. Older toolchains
let us get away with it:
int foo(int a) { return a / 0; }
foo:
li 9,0
divw 3,3,9
extsw 3,3
blr
But newer ones catch it:
foo:
trap
Add a check to avoid the divide by zero.
Fixes: bd067f83b084 ("powerpc/64: Fix naming of cache block vs. cache line")
Signed-off-by: Anton Blanchard <redacted>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>