Re: RFA: New pass to delete unexecutable paths in the CFG

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Nov 8, 2011 at 8:47 PM, Jeff Law <law@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11/07/11 15:25, Richard Guenther wrote:
>>
>> Indeed. ?We'd have to tell people that they cannot catch *(void *)0
>> = 0 with a SIGSEGV signal handler unless they compile with some
>> magic flag. Thus, the question is whether we want to optimize
>> things in a way that are non-obvious to people viewing things from
>> a POSIX point of view rather than a C standard conforming issue.
> Agreed. ?The path I'd started going down was a flag like
> "-fassume-null-dereference-faults-and-is-uncatchable" or something
> like that. ?-fdelete-null-pointer-checks ?would be subsumed by the new
> flag.
>
>
>>
>>
>> But what we could do by default is transform such stores to a trap
>> representation, or at least making the feeding stmts dead by
>> changing the stored value and the address to a constant (thus, even
>> preserve the trap kind). ?fold_stmt could change the stored value
>> if the address is literal zero, and all passes that know it is zero
>> should propagate it anyway. ?That wouldn't remove the outgoing edge
>> from the store of course, cfgcleanup could be teached to do that
>> though (and we could invent a trap kind argument to
>> __builtin_trap).
> More correctly, we'd wipe the path from the control edge to the *0
> statement and remove the out edges from the block with the *0. ?Then
> just leave the *0 or a __builtin_trap.
>
> We'd leave the control edge itself leading to the *0/trap. ?That
> allows us to get most of the space savings optimization as well as
> most of the secondary benefits. ?The programmer could still catch the
> trapping signal.
>
> That doesn't work in the case of the implied dereference via ?NULL PHI
> arg. ?For that case we'd have to copy the block with the PHI, the
> incoming edges would be partitioned into those implying a null
> dereference vs those not implying a null dereference. ?Effectively
> this isolates the path with the implied NULL dereference and turns it
> into an explicit NULL dereference and we could optimize as noted above.

Hm, indeed.

> So the question is do we want to proceed with any of this work? ?If so
> I can update the patch, if not I'll go back to my warning work :-)

I think we do want to continue with this work - probably not removing
the faulting dereference though.  I'd say we add a noreturn
__builtin_nullptr_deref () in place of it (eventually doing the path
isolation you mentioned above) and warn about the ones that survive
until RTL
expansion (where we'd just expand it as *0 = 0).

In fact, the warning part of it might be the most useful piece for our users ;)

Thanks,
Richard.