On 02/26/2018 05:47 PM, Martin Sebor wrote:
> On 02/26/2018 12:13 PM, Jeff Law wrote:
>> On 02/24/2018 05:11 PM, Martin Sebor wrote:
>>> Attached is an updated patch with a fix for a bad assumption
>>> exposed by building the linux kernel.
>>>
>>> On 02/19/2018 07:50 PM, Martin Sebor wrote:
>>>> PR 84468 points out a false positive in -Wstringop-truncation
>>>> in code like this:
>>>>
>>>> struct A { char a[4]; };
>>>>
>>>> void f (struct A *p, const struct A *q)
>>>> {
>>>> if (p->a)
>>>> strncpy (p->a, q->a, sizeof p->a - 1); // warning here
>>>>
>>>> p->a[3] = '\0';
>>>> }
>>>>
>>>> The warning is due to the code checking only the same basic block
>>>> as the one with the strncpy call for an assignment to the destination
>>>> to avoid it, but failing to check the successor basic block if there
>>>> is no subsequent statement in the current block. (Eliminating
>>>> the conditional is being tracked in PR 21474.)
>>>>
>>>> The attached test case adds logic to avoid this false positive.
>>>> I don't know under what circumstances there could be more than
>>>> one successor block here so I don't handle that case.
>> So this is feeling more and more like we need to go back to the ideas
>> behind checking the virtual operand chains.
>>
>> The patch as-written does not properly handle the case where BB has
>> multiple outgoing edges. For gcc-8 you could probably get away with
>> checking that you have precisely one outgoing edge without EDGE_ABNORMAL
>> set in its flags in addition to the checks you're already doing.
>>
>> But again, it's feeling more and more like the right fix is to go back
>> and walk the virtual operands.
>
> I intentionally kept the patch as simple as possible to minimize
> risk at this late stage.
>
> Attached is a more robust version that handles multiple outgoing
> edges and avoids those with the EDGE_ABNORMAL bit set. Retested
> on x86_64 and with the Linux kernel.
>
> Enhancing this code to handle more complex cases is on my to-do
> list for stage 1 (e.g., to handle bug 84561 where MEM_REF defeats
> the detection of the nul assignment).
I don't think handling multiple outgoing edges is advisable here. To do
that you have to start thinking about post-dominator analysis at which
point you're better off walking the memory web via VUSE/VDEFs.
Just verify the block has a single outgoing edge and that the edge is
not marked with EDGE_ABNORMAL. Don't bother with the recursive call.
Assuming you get a suitable block, then look inside.
I glanced over the tests and I didn't see any that would benefit from
handling multiple edges or the recursion (every one of the dg-bogus
markers should be immediately transferring control to the null
termination statement AFAICT).
jeff
