Re: [PR tree-optimization/67955] Exploit PTA in DSE

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

On Sat, Jan 7, 2017 at 7:01 PM, Jeff Law <law@redhat.com> wrote:
> On 01/05/2017 01:34 AM, Richard Biener wrote:
>>
>> On Wed, Jan 4, 2017 at 8:24 PM, Jeff Law <law@redhat.com> wrote:
>>>
>>>
>>> The more I think about this the more I'm sure we need to verify pt.null
>>> is
>>> not in the points-to set.    I've taken the above testcase and added it
>>> as a
>>> negative test.  Bootstrapped, regression tested and committed to the
>>> trunk
>>> along with the other minor cleanups you pointed out.
>>
>>
>> Note disabling this for pt.null == 1 makes it pretty useless given we
>> compute
>> that conservatively to always 1 in points-to analysis (and only VRP ever
>> sets
>> it to zero).  See PTAs find_what_p_points_to.  This is because PTA does
>> not conservatively compute whether a pointer may be NULL (all bugs, I have
>> partly fixed some and have an incomplete patch to fix others -- at the
>> point
>> I looked into this we had no users of pt.null info and thus I decided the
>> extra constraints and complexity wasn't worth the
>> compile-time/memory-use).
>>
>> Without -fnon-call-exceptions removing the *0 = 2 store is IMHO ok, so we
>> only have to make sure to not break the exception case.
>
> I spent a goodly amount of time thinking about this...  I think the key
> point is whether or not removing the store is observable in a conforming
> program.
>
> Essentially if we get a non-call exception or receive a signal between the
> "dead" store and the subsequent store, then we could observe that the "dead"
> store was removed if the object being stored escapes.
>
> This seems to have larger implications than just the cases we're looking at
> (assume "a" is something in memory, of course).
>
>
> a = 1;
> <trigger non-call exception>
> a = 2;
>
>
> If "a" escapes such that its value can be queried in the exception handler,
> then the exception handler would be able to observe the first store and thus
> it should not be removed.

Yes, and it won't as long as the EH is thrown internally (and thus we have
a CFG reflecting it).  When it's only externally catched we lose of course...

We'd need an Ada testcase to actually show behavior that is not conforming
to an existing language specification though.

I suspect we have a similar issue in C++ for sth like

void __attribute__((const)) foo () { throw; }

int x;
void bar ()
{
  x = 1;
  foo ();
  x = 2;
}

where foo is const but not nothrow.

> We also have to be cognizant of systems where there is memory mapped at
> location 0.  When that is true, we must check pt.null and honor it, even if
> it pessimizes code.

With -fno-delete-null-pointer-checks (that's what such systems set) PTA computes
0 as "nonlocal" and thus it won't be a singleton points-to solution.

>
>
>> For
>>
>> int foo (int *p, int b)
>> {
>>   int *q;
>>   int i = 1;
>>   if (b)
>>     q = &i;
>>   else
>>     q = (void *)0;
>>   *q = 2;
>>   i = 3;
>>   return *q;
>> }
>
> So on a system where *0 is a valid memory address, *q = 2 does not make
> anything dead, nor does i = 3 unless we were to isolate the THEN/ELSE
> blocks.
>
> On a system where *0 traps, there is no way to observe the value of "i" in
> the handler.  Thus i = 1 is a dead store.  I believe we must keep the *q = 2
> store because it can trigger a signal/exception which is itself an
> observable side effect?  Right?

But writing to 0 invokes undefined behavior which we have no obligation to
preserve (unless we make it well-defined with -fnon-call-exceptions -fexceptions
as a GCC extension).

>>
>> we remove all stores but the last store to i and the load from q (but we
>> don't
>> replace q with &i here, a missed optimization if removing the other stores
>> is
>> valid).
>
> But if we remove the *q = 2 store, we remove an observable side effect, the
> trap/exception itself if we reach that statement via the ELSE path.

As said above - I don't think we have to care for C/C++ w/o
-fnon-call-exceptions.

Richard.

>
> Jeff