consider successor blocks when avoiding -Wstringop-truncation (PR 84468)

Message ID c130c6ca-7053-e6fd-2e27-088e13395e5f@gmail.com
State Superseded
Headers show
Series
  • consider successor blocks when avoiding -Wstringop-truncation (PR 84468)
Related show

Commit Message

Martin Sebor Feb. 20, 2018, 2:50 a.m.
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.

Tested on x86_64-linux.

Martin

Patch

PR tree-optimization/84468 - bogus -Wstringop-truncation despite assignment after conditional strncpy

gcc/testsuite/ChangeLog:

	PR tree-optimization/84468
	* gcc.dg/Wstringop-truncation.c: New test.

gcc/ChangeLog:

	PR tree-optimization/84468
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Also consider successor
	basic blocks.

Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 257796)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -1851,8 +1851,18 @@  maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
      avoid the truncation warning.  */
   gsi_next_nondebug (&gsi);
   gimple *next_stmt = gsi_stmt (gsi);
+  if (!next_stmt)
+    {
+      /* When there is no statement in the same basic block check
+	 the immediate successor block.  */
+      basic_block bb = gimple_bb (stmt);
+      basic_block nextbb
+	= EDGE_COUNT (bb->succs) ? EDGE_SUCC (bb, 0)->dest : NULL;
+      gimple_stmt_iterator it = gsi_start_bb (nextbb);
+      next_stmt = gsi_stmt (it);
+    }
 
-  if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
+  if (next_stmt && is_gimple_assign (next_stmt))
     {
       tree lhs = gimple_assign_lhs (next_stmt);
       tree_code code = TREE_CODE (lhs);
Index: gcc/testsuite/gcc.dg/Wstringop-truncation.c
===================================================================
--- gcc/testsuite/gcc.dg/Wstringop-truncation.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wstringop-truncation.c	(working copy)
@@ -0,0 +1,116 @@ 
+/* PR tree-optimization/84468 - Inconsistent -Wstringop-truncation warnings
+   with -O2
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-truncation -ftrack-macro-expansion=0" }  */
+
+#define strncpy __builtin_strncpy
+
+struct A
+{
+  char a[4];
+};
+
+void no_pred_succ (struct A *p, const struct A *q)
+{
+  strncpy (p->a, q->a, sizeof p->a - 1);      /* { dg-warning "\\\[-Wstringop-truncation" } */
+}
+
+
+void no_succ (struct A *p, const struct A *q)
+{
+  if (q->a)
+    strncpy (p->a, q->a, sizeof p->a - 1);    /* { dg-warning "\\\[-Wstringop-truncation" } */
+}
+
+
+void succ (struct A *p, const struct A *q)
+{
+  /* Verify that the assignment suppresses the warning for the conditional
+     strcnpy call.  The conditional should be folded to true since the
+     address of an array can never be null (see bug 84470).  */
+  if (q->a)
+    strncpy (p->a, q->a, sizeof p->a - 1);    /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  p->a[sizeof p->a - 1] = 0;
+}
+
+
+void succ_2 (struct A *p, const struct A *q, int i)
+{
+  /* Same as above but with a conditional that cannot be eliminated.  */
+  if (i < 0)
+    strncpy (p->a, q->a, sizeof p->a - 1);    /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  p->a[sizeof p->a - 1] = 0;
+}
+
+
+int next_succ (struct A *p, const struct A *q, int i, int j)
+{
+  /* Same as above but with a nested conditionals with else clauses.  */
+  if (i < 0)
+    {
+      if (j < 0)
+	strncpy (p->a, q->a, sizeof p->a - 1);    /* { dg-bogus "\\\[-Wstringop-truncation" } */
+    }
+  else
+    __builtin_strcpy (p->a, q->a);
+
+  p->a[sizeof p->a - 1] = 0;
+  return 0;
+}
+
+
+int next_succ_1 (struct A *p, const struct A *q, int i, int j)
+{
+  /* Same as above but with a nested conditionals with else clauses.  */
+  if (i < 0)
+    {
+      if (j < 0)
+	strncpy (p->a, q->a, sizeof p->a - 1);    /* { dg-bogus "\\\[-Wstringop-truncation" } */
+      else
+	strncpy (p->a, q->a, sizeof p->a - 2);    /* { dg-bogus "\\\[-Wstringop-truncation" } */
+    }
+
+  p->a[sizeof p->a - 2] = 0;
+  return 1;
+}
+
+
+int next_succ_2 (struct A *p, const struct A *q, int i, int j)
+{
+  /* Same as above but with a nested conditionals with else clauses.  */
+  if (i < 0)
+    {
+      if (j < 0)
+	strncpy (p->a, q->a, sizeof p->a - 1);    /* { dg-bogus "\\\[-Wstringop-truncation" } */
+      else
+	strncpy (p->a, q->a, sizeof p->a - 2);    /* { dg-bogus "\\\[-Wstringop-truncation" } */
+    }
+  else
+    __builtin_strcpy (p->a, q->a);
+
+  p->a[sizeof p->a - 2] = 0;
+  return 2;
+}
+
+
+void cond_succ_warn (struct A *p, const struct A *q, int i)
+{
+  /* Verify that a conditional assignment doesn't suppress the warning.  */
+  strncpy (p->a, q->a, sizeof p->a - 1);      /* { dg-warning "\\\[-Wstringop-truncation" } */
+
+  if (i < 0)
+    p->a[sizeof p->a - 1] = 0;
+}
+
+void cond_succ_nowarn (struct A *p, const struct A *q)
+{
+  /* Verify that distinct but provably equivalent conditionals are
+     recognized and don't trigger the warning.  */
+  if (p != q)
+    strncpy (p->a, q->a, sizeof p->a - 1);
+
+  if (p->a != q->a)
+    p->a[sizeof p->a - 1] = 0;
+}