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

[Jeff Law <law at redhat dot com>]

On 9/17/18 3:15 PM, Martin Sebor wrote:
> On 09/17/2018 11:35 AM, Richard Biener wrote:
>> On September 17, 2018 7:24:16 PM GMT+02:00, Jeff Law <law@redhat.com>
>> wrote:
>>> On 9/15/18 2:14 AM, Bernd Edlinger wrote:
>>>> On 9/14/18, Martin Sebor wrote:
>>>>> As I said above, this happens during the dom walk in the ccp
>>>>> pass:
>>>>>
>>>>>   substitute_and_fold_dom_walker walker (CDI_DOMINATORS, this);
>>>>>   walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun));
>>>>>
>>>>> The warning is issued during the walker.walk() call as
>>>>> strncpy is being folded into memcpy.  The prior assignments are
>>>>> only propagated later, when the next statement after the strncpy
>>>>> call is reached.  It happens in
>>>>> substitute_and_fold_dom_walker::before_dom_children(). So during
>>>>> the strncpy folding we see the next statement as:
>>>>>
>>>>>   MEM[(struct S *)_1].a[n_7] = 0;
>>>>>
>>>>> After the strncpy call is transformed to memcpy, the assignment
>>>>> above is transformed to
>>>>>
>>>>>   MEM[(struct S *)_8].a[3] = 0;
>>>>>
>>>>>
>>>>>>     If they're only discovered as copies within the pass where
>>> you're trying
>>>>>>     to issue the diagnostic, then you'd want to see if the pass has
>>> any
>>>>>>     internal structures that tell you about equivalences.
>>>>>
>>>>>
>>>>> I don't know if this is possible.  I don't see any APIs in
>>>>> tree-ssa-propagate.h that would let me query the internal data
>>>>> somehow to find out during folding (when the warning is issued).
>>>>
>>>>
>>>> Well,
>>>>
>>>> if I see this right, the CCP is doing tree transformations
>>>> while from the folding of strncpy the predicate
>>> maybe_diag_stxncpy_trunc
>>>> is called, and sees inconsistent information, in the tree,
>>>> and therefore it issues a warning.
>>>>
>>>> I understand that walking the references is fragile at least
>>>> in this state.
>>>>
>>>> But why not just prevent warnings when this is called from CCP?
>>>>
>>>>
>>>> Like this?
>>>>
>>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>>> Is it OK for trunk?
>>> No.  That's just hacking around the real problem.
>>
>> The real problem is emitting diagnostics from folding code.
> 
> Strncpy is a particularly dangerous function that's often
> misunderstood and misused.  IMO, it would be a worthwhile
> tradeoff to move the strncpy to memcpy transformation to
> the strlen pass where these bugs could be detected more
> reliably, and with fewer false positives.  I would not
> expect it to have a noticeable impact on code efficiency.
> 
> I'd be happy to modify the patch to do that if you find it
> acceptable.
That natural question is what is the immediate (ie testsuite) fallout?

Jeff