Re: PATCH to fix bogus warning with -Wstringop-truncation -g (PR tree-optimization/84228)

[Marek Polacek]

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.

Thanks,

2018-02-06  Marek Polacek  <pola...@redhat.com>

        PR tree-optimization/84228
        * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Skip debug statements.

        * c-c++-common/Wstringop-truncation-3.c: New test.

diff --git gcc/testsuite/c-c++-common/Wstringop-truncation-3.c 
gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
index e69de29bb2d..ba6b7de094b 100644
--- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
+++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
@@ -0,0 +1,22 @@
+/* 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)
+{
+  int b = 5, c = 6, d = 7;
+  if (a)
+    goto err;
+  strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } */
+  b = 8; c = 9; d = 10;
+  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))

        Marek