On 02/06/2018 08:59 AM, Martin Sebor wrote:
> On 02/06/2018 06:23 AM, Marek Polacek wrote:
>> On Tue, Feb 06, 2018 at 01:57:36PM +0100, Jakub Jelinek wrote:
>>> On Tue, Feb 06, 2018 at 01:46:21PM +0100, Marek Polacek wrote:
>>>> --- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
>>>> +++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
>>>> @@ -0,0 +1,20 @@
>>>> +/* PR tree-optimization/84228 */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-Wstringop-truncation -O2 -g" } */
>>>> +
>>>> +char *strncpy (char *, const char *, __SIZE_TYPE__);
>>>> +struct S
>>>> +{
>>>> + char arr[64];
>>>> +};
>>>> +
>>>> +int
>>>> +foo (struct S *p1, const char *a)
>>>> +{
>>>> + if (a)
>>>> + goto err;
>>>> + strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified
>>>> bound" } */
>>>
>>> Just curious, what debug stmt is in between those?
>>
>> Just
>>
>> strncpy (_1, 0B, 64);
>> # DEBUG BEGIN_STMT
>> p1_5(D)->arr[3] = 0;
>>
>>> Wouldn't it be better to force them a couple of debug stmts?
>>> Say
>>> int b = 5, c = 6, d = 7;
>>> at the start of the function and
>>> b = 8; c = 9; d = 10;
>>> in between strncpy and the '\0' store?
>>
>> Yep. I tweaked the testcase. Now we have
>>
>> strncpy (_1, 0B, 64);
>> # DEBUG BEGIN_STMT
>> # DEBUG b => 8
>> # DEBUG BEGIN_STMT
>> # DEBUG c => 9
>> # DEBUG BEGIN_STMT
>> # DEBUG d => 10
>> # DEBUG BEGIN_STMT
>> p1_5(D)->arr[3] = 0;
>>
>>>> + p1->arr[3] = '\0';
>>>> +err:
>>>> + return 0;
>>>> +}
>>>> diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
>>>> index c3cf432a921..f0f6535017b 100644
>>>> --- gcc/tree-ssa-strlen.c
>>>> +++ gcc/tree-ssa-strlen.c
>>>> @@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator
>>>> gsi, tree src, tree cnt)
>>>>
>>>> /* Look for dst[i] = '\0'; after the stxncpy() call and if found
>>>> avoid the truncation warning. */
>>>> - gsi_next (&gsi);
>>>> + gsi_next_nondebug (&gsi);
>>>> gimple *next_stmt = gsi_stmt (gsi);
>>>>
>>>> if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
>>>
>>> Ok for trunk, though generally looking at just next stmt is very
>>> fragile, might be
>>> better to look at the strncpy's vuse immediate uses if they are
>>> within the
>>> same basic block and either don't alias with it, or are the store it is
>>> looking for, or something similar.
>>
>> I guess some
>> FOR_EACH_IMM_USE_FAST (...)
>> {
>> if (is_gimple_debug (USE_STMT (use_p)))
>> continue;
>> ...
>> would be better.
>
> Jeff and I had a fairly extensive discussion about how best to
> handle debug statements. I had prototyped his suggestion along
> these lines but ran into false positives and other difficulties.
> The details are here:
>
> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00805.html
>
> I thought I ended up just using gsi_next_nondebug() and added
> a test for it but I guess I must be misremembering. The patch
> review spanned a couple of months so it's even possible I forgot
> to make the change.
Right. I remember that being the ultimate decision (gsi_next_nondebug).
Perhaps the patch got lost in the long cycle times.
Jeff
