public inbox for gcc-help@gcc.gnu.org
 help / color / mirror / Atom feed
* m68k/cpu32 TRAP instruction?
@ 2018-10-04 12:20 Josef Wolf
  2018-10-04 12:34 ` Jeff Law
  0 siblings, 1 reply; 17+ messages in thread
From: Josef Wolf @ 2018-10-04 12:20 UTC (permalink / raw)
  To: gcc-help

Hello,

I have a project with a mc68332 controller (cpu32 based). This project worked
great with older releases of GCC. gcc-2.95 up to gcc-4.8.1 worked great.

For various reasons, I upgraded to GCC-8.2.0 and get various crashes
now. Closer investigations reveal, that GCC now uses the "trap #7" and
"trap #15" instructions.

Why does gcc use those instructions? There is nothing special in the code
which is now translated into those trap instructions. I don't use floating
point at all, so I don't see any reason to use those instructions (and crash
on them).

Any hints?

Thanks!

-- 
Josef Wolf
jw@raven.inka.de

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-04 12:20 m68k/cpu32 TRAP instruction? Josef Wolf
@ 2018-10-04 12:34 ` Jeff Law
  2018-10-04 17:10   ` Josef Wolf
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Law @ 2018-10-04 12:34 UTC (permalink / raw)
  To: Josef Wolf, gcc-help

On 10/4/18 6:19 AM, Josef Wolf wrote:
> Hello,
> 
> I have a project with a mc68332 controller (cpu32 based). This project worked
> great with older releases of GCC. gcc-2.95 up to gcc-4.8.1 worked great.
> 
> For various reasons, I upgraded to GCC-8.2.0 and get various crashes
> now. Closer investigations reveal, that GCC now uses the "trap #7" and
> "trap #15" instructions.
> 
> Why does gcc use those instructions? There is nothing special in the code
> which is now translated into those trap instructions. I don't use floating
> point at all, so I don't see any reason to use those instructions (and crash
> on them).
> 
> Any hints?
GCC will replace a NULL pointer dereference or division by zero with a
trap instruction.

jeff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-04 12:34 ` Jeff Law
@ 2018-10-04 17:10   ` Josef Wolf
  2018-10-04 17:52     ` Jeff Law
  0 siblings, 1 reply; 17+ messages in thread
From: Josef Wolf @ 2018-10-04 17:10 UTC (permalink / raw)
  To: gcc-help

On Thu, Oct 04, 2018 at 06:33:57AM -0600, Jeff Law wrote:
> On 10/4/18 6:19 AM, Josef Wolf wrote:
> > Why does gcc use those instructions? There is nothing special in the code
> > which is now translated into those trap instructions. I don't use floating
> > point at all, so I don't see any reason to use those instructions (and crash
> > on them).
> >
> GCC will replace a NULL pointer dereference or division by zero with a
> trap instruction.

Is this described somewhere? I am pretty much sure that there is no NULL
pointer dereference and no division in this code. There must be something else.

In addition, the crash disappears, when I insert some no-op function call at
the beginning of one of those functions. This causes GCC to use other
registers for local variables.

-- 
Josef Wolf
jw@raven.inka.de

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-04 17:10   ` Josef Wolf
@ 2018-10-04 17:52     ` Jeff Law
  2018-10-04 19:40       ` Josef Wolf
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Law @ 2018-10-04 17:52 UTC (permalink / raw)
  To: Josef Wolf, gcc-help

On 10/4/18 11:01 AM, Josef Wolf wrote:
> On Thu, Oct 04, 2018 at 06:33:57AM -0600, Jeff Law wrote:
>> On 10/4/18 6:19 AM, Josef Wolf wrote:
>>> Why does gcc use those instructions? There is nothing special in the code
>>> which is now translated into those trap instructions. I don't use floating
>>> point at all, so I don't see any reason to use those instructions (and crash
>>> on them).
>>>
>> GCC will replace a NULL pointer dereference or division by zero with a
>> trap instruction.
> 
> Is this described somewhere? I am pretty much sure that there is no NULL
> pointer dereference and no division in this code. There must be something else.
You can use -fdump-tree-all-blocks-details and look at the
isolate-erroneous-paths dump file.  Those are the cases I'm aware of
where GCC will insert traps.




> 
> In addition, the crash disappears, when I insert some no-op function call at
> the beginning of one of those functions. This causes GCC to use other
> registers for local variables.
register allocation shouldn't have anything to do with whether or not
something gets translated into a trap.

jeff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-04 17:52     ` Jeff Law
@ 2018-10-04 19:40       ` Josef Wolf
  2018-10-04 21:20         ` Jeff Law
  0 siblings, 1 reply; 17+ messages in thread
From: Josef Wolf @ 2018-10-04 19:40 UTC (permalink / raw)
  To: gcc-help

On Thu, Oct 04, 2018 at 11:52:29AM -0600, Jeff Law wrote:
> On 10/4/18 11:01 AM, Josef Wolf wrote:
> > On Thu, Oct 04, 2018 at 06:33:57AM -0600, Jeff Law wrote:
> >> On 10/4/18 6:19 AM, Josef Wolf wrote:
> >>> Why does gcc use those instructions? There is nothing special in the code
> >>> which is now translated into those trap instructions. I don't use floating
> >>> point at all, so I don't see any reason to use those instructions (and crash
> >>> on them).
> >>>
> >> GCC will replace a NULL pointer dereference or division by zero with a
> >> trap instruction.
> > 
> > Is this described somewhere? I am pretty much sure that there is no NULL
> > pointer dereference and no division in this code. There must be something else.
> You can use -fdump-tree-all-blocks-details and look at the
> isolate-erroneous-paths dump file.  Those are the cases I'm aware of
> where GCC will insert traps.

Thanks, I'll check that out.

When gcc inserts trap instructions, it assumes that there is some sort of
hanlder installed for this? How should such a handler look like? I'd rather
have the handler output some useful diagnostics instead of just crashing.

> > In addition, the crash disappears, when I insert some no-op function call at
> > the beginning of one of those functions. This causes GCC to use other
> > registers for local variables.
> register allocation shouldn't have anything to do with whether or not
> something gets translated into a trap.

The traps are there in both cases. But with this no-op function call,
everything works as expected and the trap path is not taken. Without this
no-op function call, the trap path will be taken. The only difference in the
generated code is register allocation, caused by the additional function call.
The input data is identical in both cases.

I discovered this because I was inserting printf statements to track down the
problem.

-- 
Josef Wolf
jw@raven.inka.de

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-04 19:40       ` Josef Wolf
@ 2018-10-04 21:20         ` Jeff Law
  2018-10-04 22:07           ` Toby Douglass
  2018-10-05 21:20           ` Josef Wolf
  0 siblings, 2 replies; 17+ messages in thread
From: Jeff Law @ 2018-10-04 21:20 UTC (permalink / raw)
  To: Josef Wolf, gcc-help

On 10/4/18 1:39 PM, Josef Wolf wrote:
> On Thu, Oct 04, 2018 at 11:52:29AM -0600, Jeff Law wrote:
>> On 10/4/18 11:01 AM, Josef Wolf wrote:
>>> On Thu, Oct 04, 2018 at 06:33:57AM -0600, Jeff Law wrote:
>>>> On 10/4/18 6:19 AM, Josef Wolf wrote:
>>>>> Why does gcc use those instructions? There is nothing special in the code
>>>>> which is now translated into those trap instructions. I don't use floating
>>>>> point at all, so I don't see any reason to use those instructions (and crash
>>>>> on them).
>>>>>
>>>> GCC will replace a NULL pointer dereference or division by zero with a
>>>> trap instruction.
>>>
>>> Is this described somewhere? I am pretty much sure that there is no NULL
>>> pointer dereference and no division in this code. There must be something else.
>> You can use -fdump-tree-all-blocks-details and look at the
>> isolate-erroneous-paths dump file.  Those are the cases I'm aware of
>> where GCC will insert traps.
> 
> Thanks, I'll check that out.
> 
> When gcc inserts trap instructions, it assumes that there is some sort of
> hanlder installed for this? How should such a handler look like? I'd rather
> have the handler output some useful diagnostics instead of just crashing.
It's up to the application to figure out how to handle this situation.

The compiler only inserts the traps when it can prove conclusively that
the program will trigger undefined behavior at a particular execution point.

Stopping the program immediately is far better than the compiler trying
to issue diagnostics or anything like that.  You're in the world of
undefined behavior, the safe thing to do is halt immediately.

> 
>>> In addition, the crash disappears, when I insert some no-op function call at
>>> the beginning of one of those functions. This causes GCC to use other
>>> registers for local variables.
>> register allocation shouldn't have anything to do with whether or not
>> something gets translated into a trap.
> 
> The traps are there in both cases. But with this no-op function call,
> everything works as expected and the trap path is not taken. Without this
> no-op function call, the trap path will be taken. The only difference in the
> generated code is register allocation, caused by the additional function call.
> The input data is identical in both cases.
That indicates a deeper problem (the existence of the call changing the
paths through the program.

ANyway, I've done about as much as I can here.

I'll repeat, if the compiler is inserting traps, the program is horribly
buggy  at those points.  You should dig into why.

jeff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-04 21:20         ` Jeff Law
@ 2018-10-04 22:07           ` Toby Douglass
  2018-10-05 21:20           ` Josef Wolf
  1 sibling, 0 replies; 17+ messages in thread
From: Toby Douglass @ 2018-10-04 22:07 UTC (permalink / raw)
  To: gcc-help

On 04/10/2018 23:19, Jeff Law wrote:
> I'll repeat, if the compiler is inserting traps, the program is horribly
> buggy  at those points.  You should dig into why.

Seconded.  I have the same view.  The code worked before by chance; the 
compiler change has revealed this.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-04 21:20         ` Jeff Law
  2018-10-04 22:07           ` Toby Douglass
@ 2018-10-05 21:20           ` Josef Wolf
  2018-10-05 21:31             ` Jeff Law
  2018-10-06  3:51             ` Xi Ruoyao
  1 sibling, 2 replies; 17+ messages in thread
From: Josef Wolf @ 2018-10-05 21:20 UTC (permalink / raw)
  To: gcc-help

On Thu, Oct 04, 2018 at 03:19:56PM -0600, Jeff Law wrote:
> On 10/4/18 1:39 PM, Josef Wolf wrote:
> > When gcc inserts trap instructions, it assumes that there is some sort of
> > hanlder installed for this? How should such a handler look like? I'd rather
> > have the handler output some useful diagnostics instead of just crashing.
> It's up to the application to figure out how to handle this situation.
> 
> The compiler only inserts the traps when it can prove conclusively that
> the program will trigger undefined behavior at a particular execution point.

I think it's the other way around: the trap is inserted when gcc sees a chance
for NULL-dereference.

> Stopping the program immediately is far better than the compiler trying
> to issue diagnostics or anything like that.

I'd very much prefer a warning at compile time! With a warning, I'd have a
chance to fix the problem before the trap is taken in production.

I fail to see any good in this trap:

- If the compiler can detect such a serious problem, then it should emit a
  warning AT COMPILE TIME.

- If the compiler inserts a trap, then this trap should improve the situation
  instead of making it worse.

  Simply crashing leaves you without any clue. You have no idea where to look
  for the bug. The crash could even be by a power outage. That's not exactly
  improving the situation.

  Issuing a diagnostic "Oops, NULL-dereference at adsf.c:123" would improve
  the situation: you know where to look for the fix.

> You're in the world of
> undefined behavior, the safe thing to do is halt immediately.

Whether stopping immediately is good or not depends very much on the context.

Instead of stopping immediately, I'd also prefer to get something like
"hey NULL-dereference at foobar.c:123. Stopping now."

> That indicates a deeper problem (the existence of the call changing the
> paths through the program.

The existence of a call to a function with potential side effets (e.g. printf)
can AND WILL very much change paths due to the fact that the optimizer needs
to drop some assumptions.

> I'll repeat, if the compiler is inserting traps, the program is horribly
> buggy  at those points.  You should dig into why.

If this would be true, then traps in a program that is guaranteed to never
dereferencing NULL would be an indication of bugs in the compiler?

-- 
Josef Wolf
jw@raven.inka.de

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-05 21:20           ` Josef Wolf
@ 2018-10-05 21:31             ` Jeff Law
  2018-10-05 21:53               ` Segher Boessenkool
  2018-10-06  3:51             ` Xi Ruoyao
  1 sibling, 1 reply; 17+ messages in thread
From: Jeff Law @ 2018-10-05 21:31 UTC (permalink / raw)
  To: Josef Wolf, gcc-help

On 10/5/18 3:14 PM, Josef Wolf wrote:
> On Thu, Oct 04, 2018 at 03:19:56PM -0600, Jeff Law wrote:
>> On 10/4/18 1:39 PM, Josef Wolf wrote:
>>> When gcc inserts trap instructions, it assumes that there is some sort of
>>> hanlder installed for this? How should such a handler look like? I'd rather
>>> have the handler output some useful diagnostics instead of just crashing.
>> It's up to the application to figure out how to handle this situation.
>>
>> The compiler only inserts the traps when it can prove conclusively that
>> the program will trigger undefined behavior at a particular execution point.
> 
> I think it's the other way around: the trap is inserted when gcc sees a chance
> for NULL-dereference.If you hit the trap at runtime, then you were going to dereference a
null pointer.  It's that simple.




> 
>> Stopping the program immediately is far better than the compiler trying
>> to issue diagnostics or anything like that.
> 
> I'd very much prefer a warning at compile time! With a warning, I'd have a
> chance to fix the problem before the trap is taken in production.
> 
> I fail to see any good in this trap:
Well, your opinion and mine differ considerably.  GCC has worked like
this for years and it's exposed a bug in your code.  You have a few
options.  Fix your code, use an old compiler or turn off the behavior
with a switch.


> 
> - If the compiler can detect such a serious problem, then it should emit a
>   warning AT COMPILE TIME.
We don't for various reasons.  Frankly, I think we should as well, but I
lost that battle.

> 
> - If the compiler inserts a trap, then this trap should improve the situation
>   instead of making it worse.
It does improve things significantly.

> 
>   Simply crashing leaves you without any clue. You have no idea where to look
>   for the bug. The crash could even be by a power outage. That's not exactly
>   improving the situation.
I would disagree strongly.

> 
> The existence of a call to a function with potential side effets (e.g. printf)
> can AND WILL very much change paths due to the fact that the optimizer needs
> to drop some assumptions.
Again, if you encounter one of these traps, then your program was going
to dereference the null pointer.  I know.  I wrote the code.


> 
>> I'll repeat, if the compiler is inserting traps, the program is horribly
>> buggy  at those points.  You should dig into why.
> 
> If this would be true, then traps in a program that is guaranteed to never
> dereferencing NULL would be an indication of bugs in the compiler?
If the runtime branching behavior does not lead to the path where the
trap was inserted, then you won't get a runtime trap.  We can't solve
the halting problem, obviously.

But when the compiler sees a *0 and it'll turn that into a trap.  It
also will isolate paths to a dereference where one or more paths would
result in a *0.  The isolated paths will get a trap, the normal paths
will be left alone.

The same thing is done for division by zero.

But again, the compiler has exposed a bug in your program.  Your best
course of action is to fix your code.

jeff


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-05 21:31             ` Jeff Law
@ 2018-10-05 21:53               ` Segher Boessenkool
  2018-10-06 19:52                 ` Jeff Law
  0 siblings, 1 reply; 17+ messages in thread
From: Segher Boessenkool @ 2018-10-05 21:53 UTC (permalink / raw)
  To: Jeff Law; +Cc: Josef Wolf, gcc-help

On Fri, Oct 05, 2018 at 03:30:47PM -0600, Jeff Law wrote:
> On 10/5/18 3:14 PM, Josef Wolf wrote:
> > 
> > - If the compiler can detect such a serious problem, then it should emit a
> >   warning AT COMPILE TIME.
> We don't for various reasons.  Frankly, I think we should as well, but I
> lost that battle.

There is -Wnull-dereference.

If you want the compiler to warn every time there *could* be a null
dereference, well, how many false positives will that be?  ;-)

> But when the compiler sees a *0 and it'll turn that into a trap.  It
> also will isolate paths to a dereference where one or more paths would
> result in a *0.  The isolated paths will get a trap, the normal paths
> will be left alone.
> 
> The same thing is done for division by zero.
> 
> But again, the compiler has exposed a bug in your program.  Your best
> course of action is to fix your code.

If you have a hard time figuring out where the trap is coming from, run
your program under a debugger.


Segher

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-05 21:20           ` Josef Wolf
  2018-10-05 21:31             ` Jeff Law
@ 2018-10-06  3:51             ` Xi Ruoyao
  2018-10-08 20:00               ` Josef Wolf
  1 sibling, 1 reply; 17+ messages in thread
From: Xi Ruoyao @ 2018-10-06  3:51 UTC (permalink / raw)
  To: Josef Wolf; +Cc: gcc-help

On 2018-10-05 23:14 +0200, Josef Wolf wrote:
> On Thu, Oct 04, 2018 at 03:19:56PM -0600, Jeff Law wrote:
> > On 10/4/18 1:39 PM, Josef Wolf wrote:
> > > When gcc inserts trap instructions, it assumes that there is some sort of
> > > hanlder installed for this? How should such a handler look like? I'd rather
> > > have the handler output some useful diagnostics instead of just crashing.
> > 
> > It's up to the application to figure out how to handle this situation.
> > 
> > The compiler only inserts the traps when it can prove conclusively that
> > the program will trigger undefined behavior at a particular execution point.
> 
> I think it's the other way around: the trap is inserted when gcc sees a chance
> for NULL-dereference.
> 
> > Stopping the program immediately is far better than the compiler trying
> > to issue diagnostics or anything like that.
> 
> I'd very much prefer a warning at compile time! With a warning, I'd have a
> chance to fix the problem before the trap is taken in production.
> 
> I fail to see any good in this trap:

In my POV an undefined behavior is just like __builtin_unreachable() because
the compiler can assuems all undefined behaviors are unreachable.  It's a
common precedure to use __builtin_trap() instead of __builtin_unreachable()
while debugging:

#ifdef NDEBUG
#define unreachable() __builtin_unreachable()
#else
#define unreachable() __builtin_trap()
#endif

> - If the compiler can detect such a serious problem, then it should emit a
>   warning AT COMPILE TIME.

Then we should emit warnings for all "*p" unless we can prove "p" is not NULL.
It's easy to see the proving requires to solve halting problem :).

Don't tell me you will like such warnings to spam you terminal:

    warning: undefined behavior may be invoked depending on the content of 's':
            r = sscanf(s, "%d", &x);
                ~~~~~~~^~~~~~~~~~~~
    note: according to C1X-N1570 7.21.6.2p10 if 's' is a pointer to the first
    byte of '666666666666666666' this would invoke undefined behavior, but we
    can't prove 's' is not such a pointer.  You may wish to use HOL theorem-
    proving system <https://sourceforge.net/projects/hol/> to prove or disprove
    this proposition.

> - If the compiler inserts a trap, then this trap should improve the situation
>   instead of making it worse.
> 
>   Simply crashing leaves you without any clue. You have no idea where to look
>   for the bug. The crash could even be by a power outage. That's not exactly
>   improving the situation.

You have GDB.

>   Issuing a diagnostic "Oops, NULL-dereference at adsf.c:123" would improve
>   the situation: you know where to look for the fix.

You have -fsanitize=undefined.

> > You're in the world of
> > undefined behavior, the safe thing to do is halt immediately.
> 
> Whether stopping immediately is good or not depends very much on the context.

Then what?  Just leave it undefined and format your disk, or run a game from
/usr/game?  GCC used to do this and then was blamed by a CVE.

> Instead of stopping immediately, I'd also prefer to get something like
> "hey NULL-dereference at foobar.c:123. Stopping now."

You have -fsanitize=undefined.  Or you can just wrap this into a macro like
many projects (including GCC itself).

> > That indicates a deeper problem (the existence of the call changing the
> > paths through the program.
> 
> The existence of a call to a function with potential side effets (e.g. printf)
> can AND WILL very much change paths due to the fact that the optimizer needs
> to drop some assumptions.

The optimizer just follows the standard.  It's not an AI and can't guess what's
in your mind.

> > I'll repeat, if the compiler is inserting traps, the program is horribly
> > buggy  at those points.  You should dig into why.
> 
> If this would be true, then traps in a program that is guaranteed to never
> dereferencing NULL would be an indication of bugs in the compiler?

You can prove many lemmas and theorems to prove a program is guaranteed not to
dereferencing NULL.  But the compiler can not.  It is designed to compile C
programs according to ISO/IEC 9899, not to prove theorems.  The standard does
not require a C compiler to solve halting problem, after all.

This series of article from LLVM blog has answered all of your questions:
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
GCC wiki is your friend:
http://gcc.gnu.org/wiki/FAQ#undefinedbut
-- 
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-05 21:53               ` Segher Boessenkool
@ 2018-10-06 19:52                 ` Jeff Law
  2018-10-06 21:13                   ` Segher Boessenkool
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Law @ 2018-10-06 19:52 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Josef Wolf, gcc-help

On 10/5/18 3:53 PM, Segher Boessenkool wrote:
> On Fri, Oct 05, 2018 at 03:30:47PM -0600, Jeff Law wrote:
>> On 10/5/18 3:14 PM, Josef Wolf wrote:
>>>
>>> - If the compiler can detect such a serious problem, then it should emit a
>>>   warning AT COMPILE TIME.
>> We don't for various reasons.  Frankly, I think we should as well, but I
>> lost that battle.
> 
> There is -Wnull-dereference.
Yea, but our implementation is so lame that it's effectively useless.
It only warns out of the front-end and only does so for an explicit *0
that can be seen without any constant propagation or path isolation.

> 
> If you want the compiler to warn every time there *could* be a null
> dereference, well, how many false positives will that be?  ;-)
It's really not bad if you do it late and focus just on whether or not
you can see a NULL pointer flowing into a dereference.

I'd certainly agree that it'd be too noisy to warn if you had a random
pointer of unknown value that gets dereferenced.  It could be zero and
warning for all those would be useless in practice.



jeff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-06 19:52                 ` Jeff Law
@ 2018-10-06 21:13                   ` Segher Boessenkool
  2018-10-07 19:58                     ` Jeff Law
  0 siblings, 1 reply; 17+ messages in thread
From: Segher Boessenkool @ 2018-10-06 21:13 UTC (permalink / raw)
  To: Jeff Law; +Cc: Josef Wolf, gcc-help

On Sat, Oct 06, 2018 at 01:52:03PM -0600, Jeff Law wrote:
> On 10/5/18 3:53 PM, Segher Boessenkool wrote:
> > On Fri, Oct 05, 2018 at 03:30:47PM -0600, Jeff Law wrote:
> >> On 10/5/18 3:14 PM, Josef Wolf wrote:
> >>>
> >>> - If the compiler can detect such a serious problem, then it should emit a
> >>>   warning AT COMPILE TIME.
> >> We don't for various reasons.  Frankly, I think we should as well, but I
> >> lost that battle.
> > 
> > There is -Wnull-dereference.
> Yea, but our implementation is so lame that it's effectively useless.
> It only warns out of the front-end and only does so for an explicit *0
> that can be seen without any constant propagation or path isolation.

Ah, that is not so great :-(

> > If you want the compiler to warn every time there *could* be a null
> > dereference, well, how many false positives will that be?  ;-)
> It's really not bad if you do it late and focus just on whether or not
> you can see a NULL pointer flowing into a dereference.

If you can see under some given condition there will definitely be a null
deref, then yeah that will be useful.  Could put a trap there too.  Or
don't do the test for it if not otherwise needed, just warn at compile
time.

Such a warning whould be fine for -Wall, too, which makes it a lot more
useful.  If the current implementation is so restricted, why is it not
in -Wall?  It shouldn't give many fals positives?

> I'd certainly agree that it'd be too noisy to warn if you had a random
> pointer of unknown value that gets dereferenced.  It could be zero and
> warning for all those would be useless in practice.

Yeah exactly :-)


Segher

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-06 21:13                   ` Segher Boessenkool
@ 2018-10-07 19:58                     ` Jeff Law
  2018-10-08  7:45                       ` Segher Boessenkool
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Law @ 2018-10-07 19:58 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Josef Wolf, gcc-help

On 10/6/18 3:12 PM, Segher Boessenkool wrote:
> On Sat, Oct 06, 2018 at 01:52:03PM -0600, Jeff Law wrote:
>> On 10/5/18 3:53 PM, Segher Boessenkool wrote:
>>> On Fri, Oct 05, 2018 at 03:30:47PM -0600, Jeff Law wrote:
>>>> On 10/5/18 3:14 PM, Josef Wolf wrote:
>>>>>
>>>>> - If the compiler can detect such a serious problem, then it should emit a
>>>>>   warning AT COMPILE TIME.
>>>> We don't for various reasons.  Frankly, I think we should as well, but I
>>>> lost that battle.
>>>
>>> There is -Wnull-dereference.
>> Yea, but our implementation is so lame that it's effectively useless.
>> It only warns out of the front-end and only does so for an explicit *0
>> that can be seen without any constant propagation or path isolation.
> 
> Ah, that is not so great :-(
Actually, I'll have to correct myself.  It looks like we do warn for *0
in the path isolation code, so I guess I was able to push that through
back when I wrote the path isolation code.  I didn't look to see if it
was on by default.

> 
>>> If you want the compiler to warn every time there *could* be a null
>>> dereference, well, how many false positives will that be?  ;-)
>> It's really not bad if you do it late and focus just on whether or not
>> you can see a NULL pointer flowing into a dereference.
> 
> If you can see under some given condition there will definitely be a null
> deref, then yeah that will be useful.  Could put a trap there too.  Or
> don't do the test for it if not otherwise needed, just warn at compile
> time.
That's precisely what the path isolation bits do.  When they see a NULL
as a PHI arg and the result of the PHI is unconditionally dereferenced,
we isolate the path with the NULL arg replace the dereference with a
trap and let DCE clean things up.



jeff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-07 19:58                     ` Jeff Law
@ 2018-10-08  7:45                       ` Segher Boessenkool
  0 siblings, 0 replies; 17+ messages in thread
From: Segher Boessenkool @ 2018-10-08  7:45 UTC (permalink / raw)
  To: Jeff Law; +Cc: Josef Wolf, gcc-help

Hi Jeff,

On Sun, Oct 07, 2018 at 01:58:04PM -0600, Jeff Law wrote:
> On 10/6/18 3:12 PM, Segher Boessenkool wrote:
> > On Sat, Oct 06, 2018 at 01:52:03PM -0600, Jeff Law wrote:
> >> On 10/5/18 3:53 PM, Segher Boessenkool wrote:
> >>> There is -Wnull-dereference.
> >> Yea, but our implementation is so lame that it's effectively useless.
> >> It only warns out of the front-end and only does so for an explicit *0
> >> that can be seen without any constant propagation or path isolation.
> > 
> > Ah, that is not so great :-(
> Actually, I'll have to correct myself.  It looks like we do warn for *0
> in the path isolation code, so I guess I was able to push that through
> back when I wrote the path isolation code.  I didn't look to see if it
> was on by default.
> 
> >>> If you want the compiler to warn every time there *could* be a null
> >>> dereference, well, how many false positives will that be?  ;-)
> >> It's really not bad if you do it late and focus just on whether or not
> >> you can see a NULL pointer flowing into a dereference.
> > 
> > If you can see under some given condition there will definitely be a null
> > deref, then yeah that will be useful.  Could put a trap there too.  Or
> > don't do the test for it if not otherwise needed, just warn at compile
> > time.
> That's precisely what the path isolation bits do.  When they see a NULL
> as a PHI arg and the result of the PHI is unconditionally dereferenced,
> we isolate the path with the NULL arg replace the dereference with a
> trap and let DCE clean things up.

That sounds great!

So why is it not in -Wall, and even not in -Wextra?  It probably needs
-fisolate-erroneous-paths-dereference but that is on at -O2 already (at
-Os even).


Segher

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-06  3:51             ` Xi Ruoyao
@ 2018-10-08 20:00               ` Josef Wolf
  2018-10-08 23:11                 ` Jonathan Wakely
  0 siblings, 1 reply; 17+ messages in thread
From: Josef Wolf @ 2018-10-08 20:00 UTC (permalink / raw)
  To: gcc-help

On Sat, Oct 06, 2018 at 11:51:33AM +0800, Xi Ruoyao wrote:
> >   Simply crashing leaves you without any clue. You have no idea where to look
> >   for the bug. The crash could even be by a power outage. That's not exactly
> >   improving the situation.
> 
> You have GDB.

I'd happily do this. But unfortunately, this is an embedded system, and it
doesn't have a debug interface.

So gdb won't help very much here.

> >   Issuing a diagnostic "Oops, NULL-dereference at adsf.c:123" would improve
> >   the situation: you know where to look for the fix.
> 
> You have -fsanitize=undefined.

Ah! This seems to be exactly what I was looking for!

Unfortunatly, the linker complains that it can't find -lubsan.

Do I need to do something special to enable the build/install of the sanitizer
runtime library on an embedded target? AFAICS, the sanitizer should be built
by default?

-- 
Josef Wolf
jw@raven.inka.de

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: m68k/cpu32 TRAP instruction?
  2018-10-08 20:00               ` Josef Wolf
@ 2018-10-08 23:11                 ` Jonathan Wakely
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Wakely @ 2018-10-08 23:11 UTC (permalink / raw)
  To: Josef Wolf, gcc-help

On Mon, 8 Oct 2018 at 21:00, Josef Wolf wrote:
>
> On Sat, Oct 06, 2018 at 11:51:33AM +0800, Xi Ruoyao wrote:
> > >   Simply crashing leaves you without any clue. You have no idea where to look
> > >   for the bug. The crash could even be by a power outage. That's not exactly
> > >   improving the situation.
> >
> > You have GDB.
>
> I'd happily do this. But unfortunately, this is an embedded system, and it
> doesn't have a debug interface.
>
> So gdb won't help very much here.
>
> > >   Issuing a diagnostic "Oops, NULL-dereference at adsf.c:123" would improve
> > >   the situation: you know where to look for the fix.
> >
> > You have -fsanitize=undefined.
>
> Ah! This seems to be exactly what I was looking for!
>
> Unfortunatly, the linker complains that it can't find -lubsan.

You can use -fsanitize-undefined-trap-on-error to avoid the need for
libubsan by inserting traps instead of calls to libubsan, but then
you're back where you started.

> Do I need to do something special to enable the build/install of the sanitizer
> runtime library on an embedded target? AFAICS, the sanitizer should be built
> by default?

I don't think libubsan is built for all targets.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-10-08 23:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 12:20 m68k/cpu32 TRAP instruction? Josef Wolf
2018-10-04 12:34 ` Jeff Law
2018-10-04 17:10   ` Josef Wolf
2018-10-04 17:52     ` Jeff Law
2018-10-04 19:40       ` Josef Wolf
2018-10-04 21:20         ` Jeff Law
2018-10-04 22:07           ` Toby Douglass
2018-10-05 21:20           ` Josef Wolf
2018-10-05 21:31             ` Jeff Law
2018-10-05 21:53               ` Segher Boessenkool
2018-10-06 19:52                 ` Jeff Law
2018-10-06 21:13                   ` Segher Boessenkool
2018-10-07 19:58                     ` Jeff Law
2018-10-08  7:45                       ` Segher Boessenkool
2018-10-06  3:51             ` Xi Ruoyao
2018-10-08 20:00               ` Josef Wolf
2018-10-08 23:11                 ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).