[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732 Antony Polukhin changed: What|Removed |Added Resolution|INVALID |FIXED --- Comment #9 from Antony Polukhin --- (In reply to Jeffrey A. Law from comment #8) > I wouldn't object to that. In fact I thought we kicked that around along > with an option to remove path leading to the undefined behavior completely. > But it's not something I'm likely to work on. IMO you will need this flag anyway with the "C++20 Contracts" feature to turn contract violations into UB/trap/HandlerCall/terminate/...
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732 --- Comment #8 from Jeffrey A. Law --- I wouldn't object to that. In fact I thought we kicked that around along with an option to remove path leading to the undefined behavior completely. But it's not something I'm likely to work on.
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732 --- Comment #7 from Marc Glisse --- (In reply to Jeffrey A. Law from comment #5) > And FWIW, I think we should be using __builtin_trap rather than > __builtin_unreachable in many more places because of the security concerns. It would be better to control it with flags instead of doing it inconsistently depending on who wrote each pass. We already have -fsanitize=unreachable -fsanitize-undefined-trap-on-error to automatically replace __builtin_unreachable with __builtin_trap. However, we do not have an opposite 'performance' option for cases where security is irrelevant.
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732 Jeffrey A. Law changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID --- Comment #6 from Jeffrey A. Law --- Code is working as designed AFAICT.
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732 Jeffrey A. Law changed: What|Removed |Added CC||law at redhat dot com --- Comment #5 from Jeffrey A. Law --- The code is working as designed. As noted, we want the dereference to occur so that programs can catch the signal that occurs as a result of the dereference. Using __builtin_trap is definitely better from a security standpoint because the program halts after the undefined behavior -- consider an mmu-less system that doesn't trap on *0. If you use __builtin_unreachable you'll just start executing random code which would be a wonderful attack vector. And FWIW, I think we should be using __builtin_trap rather than __builtin_unreachable in many more places because of the security concerns.
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732 --- Comment #4 from Marc Glisse --- (In reply to Richard Biener from comment #3) > note how it doesn't eliminate the actual load which probably causes the > odd code-generation. The code says: /* We want the NULL pointer dereference to actually occur so that code that wishes to catch the signal can do so. /* If we had a NULL pointer dereference, then we want to insert the __builtin_trap after the statement, for the other cases we want to insert before the statement. */ Maybe that should be protected by more flags, but it is done on purpose. > security implications. Those didn't convince me much at the time though, it is far from the only transformation that can have such effect, and we are not consistent in using trap vs unreachable.
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732
Richard Biener changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Last reconfirmed||2018-07-30
CC||law at gcc dot gnu.org,
||rguenth at gcc dot gnu.org
Ever confirmed|0 |1
--- Comment #3 from Richard Biener ---
We have the isolate-errorneous-paths pass which does:
example (const int & a)
{
int _3;
int _4;
const int * _5;
const int * _6;
int _7;
[local count: 1073741825]:
_3 = *a_2(D);
if (_3 == 0)
goto ; [100.00%]
else
goto ; [0.00%]
[local count: 598933193]:
# _5 = PHI
_4 = *_5;
return _4;
[count: 0]:
# _6 = PHI <0B(2)>
_7 ={v} *_6;
__builtin_trap ();
}
note how it doesn't eliminate the actual load which probably causes the
odd code-generation. Nothing removes it afterwards because it may
(and will!) trap.
Then isolate-errorneus-paths runs a bit late so the next CSE pass to
CSE *a_2(D) and *_5 is PRE. But we still end up with the following
in the end, keeping the condition live. As Marc said the condition
would be only dead if we'd use __builtin_unreachable () rather than
__builtin_trap () here which has security implications. But the
load doesn't need to be there.
[local count: 1073741825]:
_3 = *a_2(D);
if (_3 == 0)
goto ; [100.00%]
else
goto ; [0.00%]
[local count: 598933193]:
return _3;
[count: 0]:
_7 ={v} MEM[(const int *)0B];
__builtin_trap ();
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732 --- Comment #2 from Marc Glisse --- While I would also like to see this optimized better, ISTR that this was done on purpose, you may want to look at the old discussions. Some languages may have things set up to catch null dereferences, but that should be controlled by -fdelete-null-pointer-checks, -fnon-call-exceptions or some similar flag. Maybe that was just because Jeff doesn't like __builtin_unreachable (for security reasons).
[Bug tree-optimization/86732] Potential nullptr dereference does not propagate knowledge about the pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86732
--- Comment #1 from Antony Polukhin ---
Probably a more fair example< without taking an address of a reference:
static const int* get_if(const int* v) {
if (v && *v == 0) return v;
return nullptr;
}
int example(const int* a) {
return *get_if(a);
}
GCC produces the same suboptimal assembly:
_Z7examplePKi:
test rdi, rdi
je .L2
mov eax, DWORD PTR [rdi]
test eax, eax
jne .L2
xor eax, eax
ret
_Z7examplePKi.cold.0:
.L2:
mov eax, DWORD PTR ds:0
ud2
While Clang just generates:
_Z7examplePKi: # @_Z7examplePKi
mov eax, dword ptr [rdi]
ret
