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

[Martin Sebor <msebor@gmail.com>]

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.

Martin