On 11/29/18 1:34 PM, Martin Sebor wrote:
> On 11/16/2018 02:07 AM, Richard Biener wrote:
>> On Fri, Nov 16, 2018 at 4:12 AM Martin Sebor <mse...@gmail.com> wrote:
>>>
>>> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01818.html
>>>
>>> Please let me know if there is something I need to change here
>>> to make the fix acceptable or if I should stop trying.
>>
>> I have one more comment about
>>
>> +  /* Defer warning (and folding) until the next statement in the basic
>> +     block is reachable.  */
>> +  if (!gimple_bb (stmt))
>> +    return false;
>> +
>>
>> it's not about the next statement in the basic-block being "reachable"
>> (even w/o a CFG you can use gsi_next()) but rather that the next
>> stmt isn't yet gimplified and thus not inserted into the gimple sequence,
>> right?
> 
> No, it's about the current statement not being associated with
> a basic block yet when the warning code runs for the first time
> (during gimplify_expr), and so gsi_next() returning null.
> 
>> You apply this to gimple_fold_builtin_strncpy but I'd rather
>> see us not sprinkling this over gimple-fold.c but instead do this
>> in gimplify.c:maybe_fold_stmt, delaying folding until say lowering.
>>
>> See the attached (untested).
> 
> I would also prefer this solution.  I had tested it (in response
> to you first mentioning it back in September) and it causes quite
> a bit of fallout in tests that look for the folding to take place
> very early.  See the end of my reply here:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01248.html
> 
> But I'm willing to do the test suite cleanup if you think it's
> suitable for GCC 9.  (If you're thinking GCC 10 please let me
> know now.)
The fallout on existing tests is minimal.  What's more concerning is
that it doesn't actually pass the new test from Martin's original
submission.  We get bogus warnings.

At least part of the problem is weakness in maybe_diag_stxncpy_trunc.
It can't handle something like this:

test_literal (char * d, struct S * s)
{
  strncpy (d, "1234567890", 3);
  _1 = d + 3;
  *_1 = 0;
}


Note the pointer arithmetic between the strncpy and storing the NUL
terminator.

jeff

Reply via email to