public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <law@redhat.com>
To: Martin Sebor <msebor@gmail.com>,
	Richard Biener <richard.guenther@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PING #4][PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028)
Date: Thu, 29 Nov 2018 23:07:00 -0000	[thread overview]
Message-ID: <8a346afb-382f-cac9-a2b7-7107ef678dee@redhat.com> (raw)
In-Reply-To: <d51af7f4-f1bb-fbea-bd7e-b67d4632f0a5@gmail.com>

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 <msebor@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	other threads:[~2018-11-29 23:07 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-24 15:58 [PATCH] " Martin Sebor
2018-08-26  5:25 ` Jeff Law
2018-08-27  8:30   ` Richard Biener
2018-08-27 15:32     ` Jeff Law
2018-08-27 15:43       ` Richard Biener
2018-10-04 15:51         ` Jeff Law
2018-10-04 15:55           ` Martin Sebor
2018-10-08 10:14             ` Richard Biener
2018-10-08 21:40               ` Martin Sebor
2018-10-16 22:42             ` Jeff Law
2018-10-21  8:17               ` Martin Sebor
2018-10-31 17:07                 ` [PING #3][PATCH] " Martin Sebor
2018-11-16  3:12                   ` [PING #4][PATCH] " Martin Sebor
2018-11-16  9:07                     ` Richard Biener
2018-11-29 20:34                       ` Martin Sebor
2018-11-29 23:07                         ` Jeff Law [this message]
2018-11-29 23:43                           ` Martin Sebor
2018-11-30  2:02                             ` Jeff Law
2018-11-30  8:05                               ` Richard Biener
2018-11-30  8:30                                 ` Jakub Jelinek
2018-12-05 23:11                             ` Jeff Law
2018-12-06 13:00                               ` Christophe Lyon
2018-12-06 13:52                                 ` Jeff Law
2018-11-30  7:57                         ` Richard Biener
2018-11-30 15:51                           ` Martin Sebor
2018-11-07 21:28                 ` [PATCH] " Jeff Law
2018-11-09  1:25                   ` Martin Sebor
2018-10-04 19:55           ` Joseph Myers
2018-08-27 16:27     ` Martin Sebor
2018-08-28  4:27       ` Jeff Law
2018-08-28  9:56         ` Richard Biener
2018-08-28  9:57           ` Richard Biener
2018-08-29  0:12           ` Martin Sebor
2018-08-29  7:29             ` Richard Biener
2018-08-29 15:43               ` Martin Sebor
2018-08-30  0:27             ` Jeff Law
2018-08-30  8:48               ` Richard Biener
2018-09-12 15:50             ` Martin Sebor
2018-09-18  1:56             ` Jeff Law
2018-09-21 17:40               ` Martin Sebor
2018-10-01 21:31                 ` [PING] " Martin Sebor
2018-10-08 22:15                   ` Martin Sebor
2018-10-04 15:52             ` Jeff Law
2018-08-28 20:44         ` Martin Sebor
2018-08-28 22:17           ` Jeff Law
2018-08-27 20:31   ` Martin Sebor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8a346afb-382f-cac9-a2b7-7107ef678dee@redhat.com \
    --to=law@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=msebor@gmail.com \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).