Re: [PATCH] look harder for MEM_REF operand equality to avoid -Wstringop-truncation (PR 84561)

[Jeff Law <law at redhat dot com>]

On 9/14/18 4:11 PM, Martin Sebor wrote:
> On 09/14/2018 03:35 PM, Jeff Law wrote:
>> On 9/12/18 11:46 AM, Martin Sebor wrote:
>>> On 08/31/2018 04:07 AM, Richard Biener wrote:
>>>> On Thu, Aug 30, 2018 at 7:39 PM Martin Sebor <msebor@gmail.com> wrote:
>>>>>
>>>>> On 08/30/2018 11:22 AM, Richard Biener wrote:
>>>>>> On August 30, 2018 6:54:21 PM GMT+02:00, Martin Sebor
>>>>>> <msebor@gmail.com> wrote:
>>>>>>> On 08/30/2018 02:35 AM, Richard Biener wrote:
>>>>>>>> On Thu, Aug 30, 2018 at 2:12 AM Martin Sebor <msebor@gmail.com>
>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> The attached patch adds code to work harder to determine whether
>>>>>>>>> the destination of an assignment involving MEM_REF is the same
>>>>>>>>> as the destination of a prior strncpy call.  The included test
>>>>>>>>> case demonstrates when this situation comes up.  During ccp,
>>>>>>>>> dstbase and lhsbase returned by get_addr_base_and_unit_offset()
>>>>>>>>> end up looking like this:
>>>>>>>>
>>>>>>>> "During CCP" means exactly when?  The CCP lattice tracks copies
>>>>>>>> so CCP should already know that _1 == _8.  I suppose during
>>>>>>>> substitute_and_fold then?  But that replaces uses before folding
>>>>>>>> the stmt.
>>>>>>>
>>>>>>> Yes, when ccp_finalize() performs the final substitution during
>>>>>>> substitute_and_fold().
>>>>>>
>>>>>> But then you shouldn't need the loop but at most look at the pointer
>>>>>> SSA Def to get at the non-invariant ADDR_EXPR.
>>>>>
>>>>> I don't follow.   Are you suggesting to compare
>>>>> SSA_NAME_DEF_STMT (dstbase) to SSA_NAME_DEF_STMT (lhsbase) for
>>>>> equality?  They're not equal.
>>>>
>>>> No.
>>>>
>>>>> The first loop iterates once and retrieves
>>>>>
>>>>>    1.  _8 = &pb_3(D)->a;
>>>>>
>>>>> The second loop iterates three times and retrieves:
>>>>>
>>>>>    1.  _1 = _9
>>>>>    2.  _9 = _8
>>>>>    3.  _8 = &pb_3(D)->a;
>>>>>
>>>>> How do I get from _1 to &pb_3(D)->a without iterating?  Or are
>>>>> you saying to still iterate but compare the SSA_NAME_DEF_STMT?
>>>>
>>>> I say you should retrieve _8 = &pb_3(D)->a immediately since the
>>>> copies should be
>>>> propagated out at this stage.
>>>
>>> The warning is issued as the strncpy call is being folded (during
>>> the dom walk in substitute_and_fold_engine::substitute_and_fold)
>>> but before the subsequent statements have been folded (during
>>> the subsequent loop to eliminate statements).  So at the point
>>> of the strncpy folding the three assignments above are still
>>> there.
>>>
>>> I can't think of a good way to solve this problem that's not
>>> overly intrusive.  Unless you have some suggestions for how
>>> to deal with it, is the patch okay as is?
>> In what pass do you see the the naked copies?  In general those should
>> have been propagated away.
> 
> As I said above, this happens during the dom walk in the ccp
> pass:
My bad.  Sigh. CCP doesn't track copies, just constants, so there's not
going to be any data structure you can exploit.  And I don't think
there's a value number you can use to determine the two objects are the
same.

Hmm, let's back up a bit, what is does the relevant part of the IL look
like before CCP?  Is the real problem here that we have unpropagated
copies lying around in the IL?  Hmm, more likely the IL looksl ike:

   _8 = &pb_3(D)->a;
   _9 = _8;
   _1 = _9;
   strncpy (MEM_REF (&pb_3(D)->a), ...);
   MEM[(struct S *)_1].a[n_7] = 0;

If we were to propagate the copies out we'd at best have:

   _8 = &pb_3(D)->a;
   strncpy (MEM_REF (&pb_3(D)->a), ...);
   MEM[(struct S *)_8].a[n_7] = 0;


Is that in a form you can handle?  Or would we also need to forward
propagate the address computation into the use of _8?


jeff