diff mbox series

tree-optimization/111519 - strlen optimization skips clobbering store

Message ID 20231010104929.D13B53857709@sourceware.org
State Accepted
Headers
Series tree-optimization/111519 - strlen optimization skips clobbering store |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Richard Biener Oct. 10, 2023, 10:49 a.m. UTC 
  The following fixes a mistake in count_nonzero_bytes which happily
skips over stores clobbering the memory we load a value we store
from and then performs analysis on the memory state before the
intermediate store.

The patch implements the most simple fix - guarantee that there are
no intervening stores by tracking the original active virtual operand
and comparing that to the one of a load we attempt to analyze.

This simple approach breaks two subtests of gcc.dg/Wstringop-overflow-69.c
which I chose to XFAIL.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK?

Thanks,
Richard.

	PR tree-optimization/111519
	* tree-ssa-strlen.cc (strlen_pass::count_nonzero_bytes):
	Add virtual operand argument and pass it through.  Compare
	the memory state we try to analyze to the memory state we
	are going to use the result oon.
	(strlen_pass::count_nonzero_bytes_addr): Add virtual
	operand argument and pass it through.

	* gcc.dg/torture/pr111519.c: New testcase.
	* gcc.dg/Wstringop-overflow-69.c: XFAIL two subtests.
---
 gcc/testsuite/gcc.dg/Wstringop-overflow-69.c |  4 +-
 gcc/testsuite/gcc.dg/torture/pr111519.c      | 47 ++++++++++++++++++++
 gcc/tree-ssa-strlen.cc                       | 27 ++++++-----
 3 files changed, 64 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr111519.c
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c
index be361fe620d..3c17fe13d8e 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c
@@ -57,9 +57,9 @@  void warn_vec_decl (void)
 {
   *(VC2*)a1 = c2;       // { dg-warning "writing 2 bytes into a region of size 1" }
   *(VC4*)a2 = c4;       // { dg-warning "writing 4 bytes into a region of size 2" }
-  *(VC4*)a3 = c4;       // { dg-warning "writing 4 bytes into a region of size 3" }
+  *(VC4*)a3 = c4;       // { dg-warning "writing 4 bytes into a region of size 3" "pr111519" { xfail *-*-* } }
   *(VC8*)a4 = c8;       // { dg-warning "writing 8 bytes into a region of size 4" }
-  *(VC8*)a7 = c8;       // { dg-warning "writing 8 bytes into a region of size 7" }
+  *(VC8*)a7 = c8;       // { dg-warning "writing 8 bytes into a region of size 7" "pr111519" { xfail *-*-* } }
   *(VC16*)a15 = c16;    // { dg-warning "writing 16 bytes into a region of size 15" }
 }
 
diff --git a/gcc/testsuite/gcc.dg/torture/pr111519.c b/gcc/testsuite/gcc.dg/torture/pr111519.c
new file mode 100644
index 00000000000..095bb1cd13b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr111519.c
@@ -0,0 +1,47 @@ 
+/* { dg-do run } */
+
+int a, o;
+char b, f, i;
+long c;
+static signed char d;
+static char g;
+unsigned *h;
+signed char *e = &f;
+static signed char **j = &e;
+static long k[2];
+unsigned **l = &h;
+short m;
+volatile int z;
+
+__attribute__((noipa)) void
+foo (char *p)
+{
+  (void) p;
+}
+
+int
+main ()
+{
+  int p = z;
+  signed char *n = &d;
+  *n = 0;
+  while (c)
+    for (; i; i--)
+      ;
+  for (g = 0; g <= 1; g++)
+    {
+      *n = **j;
+      k[g] = 0 != &m;
+      *e = l && k[0];
+    }
+  if (p)
+    foo (&b);
+  for (; o < 4; o++)
+    {
+      a = d;
+      if (p)
+	foo (&b);
+    }
+  if (a != 1)
+    __builtin_abort ();
+}
diff --git a/gcc/tree-ssa-strlen.cc b/gcc/tree-ssa-strlen.cc
index 8b7ef919826..0ff3f2e308a 100644
--- a/gcc/tree-ssa-strlen.cc
+++ b/gcc/tree-ssa-strlen.cc
@@ -281,14 +281,14 @@  public:
 			    gimple *stmt,
 			    unsigned lenrange[3], bool *nulterm,
 			    bool *allnul, bool *allnonnul);
-  bool count_nonzero_bytes (tree exp,
+  bool count_nonzero_bytes (tree exp, tree vuse,
 			    gimple *stmt,
 			    unsigned HOST_WIDE_INT offset,
 			    unsigned HOST_WIDE_INT nbytes,
 			    unsigned lenrange[3], bool *nulterm,
 			    bool *allnul, bool *allnonnul,
 			    ssa_name_limit_t &snlim);
-  bool count_nonzero_bytes_addr (tree exp,
+  bool count_nonzero_bytes_addr (tree exp, tree vuse,
 				 gimple *stmt,
 				 unsigned HOST_WIDE_INT offset,
 				 unsigned HOST_WIDE_INT nbytes,
@@ -4531,8 +4531,8 @@  nonzero_bytes_for_type (tree type, unsigned lenrange[3],
 }
 
 /* Recursively determine the minimum and maximum number of leading nonzero
-   bytes in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
-   to each.
+   bytes in the representation of EXP at memory state VUSE and set
+   LENRANGE[0] and LENRANGE[1] to each.
    Sets LENRANGE[2] to the total size of the access (which may be less
    than LENRANGE[1] when what's being referenced by EXP is a pointer
    rather than an array).
@@ -4546,7 +4546,7 @@  nonzero_bytes_for_type (tree type, unsigned lenrange[3],
    Returns true on success and false otherwise.  */
 
 bool
-strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
+strlen_pass::count_nonzero_bytes (tree exp, tree vuse, gimple *stmt,
 				  unsigned HOST_WIDE_INT offset,
 				  unsigned HOST_WIDE_INT nbytes,
 				  unsigned lenrange[3], bool *nulterm,
@@ -4566,7 +4566,7 @@  strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
 	     exact value is not known) recurse once to set the range
 	     for an arbitrary constant.  */
 	  exp = build_int_cst (type, 1);
-	  return count_nonzero_bytes (exp, stmt,
+	  return count_nonzero_bytes (exp, vuse, stmt,
 				      offset, 1, lenrange,
 				      nulterm, allnul, allnonnul, snlim);
 	}
@@ -4574,6 +4574,9 @@  strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
       gimple *stmt = SSA_NAME_DEF_STMT (exp);
       if (gimple_assign_single_p (stmt))
 	{
+	  /* Do not look across other stores.  */
+	  if (gimple_vuse (stmt) != vuse)
+	    return false;
 	  exp = gimple_assign_rhs1 (stmt);
 	  if (!DECL_P (exp)
 	      && TREE_CODE (exp) != CONSTRUCTOR
@@ -4594,7 +4597,7 @@  strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
 	  for (unsigned i = 0; i != n; i++)
 	    {
 	      tree def = gimple_phi_arg_def (stmt, i);
-	      if (!count_nonzero_bytes (def, stmt,
+	      if (!count_nonzero_bytes (def, vuse, stmt,
 					offset, nbytes, lenrange, nulterm,
 					allnul, allnonnul, snlim))
 		return false;
@@ -4652,7 +4655,7 @@  strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
 	return false;
 
       /* Handle MEM_REF = SSA_NAME types of assignments.  */
-      return count_nonzero_bytes_addr (arg, stmt,
+      return count_nonzero_bytes_addr (arg, vuse, stmt,
 				       offset, nbytes, lenrange, nulterm,
 				       allnul, allnonnul, snlim);
     }
@@ -4765,7 +4768,7 @@  strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
    bytes that are pointed to by EXP, which should be a pointer.  */
 
 bool
-strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
+strlen_pass::count_nonzero_bytes_addr (tree exp, tree vuse, gimple *stmt,
 				       unsigned HOST_WIDE_INT offset,
 				       unsigned HOST_WIDE_INT nbytes,
 				       unsigned lenrange[3], bool *nulterm,
@@ -4835,7 +4838,7 @@  strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
     }
 
   if (TREE_CODE (exp) == ADDR_EXPR)
-    return count_nonzero_bytes (TREE_OPERAND (exp, 0), stmt,
+    return count_nonzero_bytes (TREE_OPERAND (exp, 0), vuse, stmt,
 				offset, nbytes,
 				lenrange, nulterm, allnul, allnonnul, snlim);
 
@@ -4855,7 +4858,7 @@  strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
 	  for (unsigned i = 0; i != n; i++)
 	    {
 	      tree def = gimple_phi_arg_def (stmt, i);
-	      if (!count_nonzero_bytes_addr (def, stmt,
+	      if (!count_nonzero_bytes_addr (def, NULL_TREE, stmt,
 					     offset, nbytes, lenrange,
 					     nulterm, allnul, allnonnul,
 					     snlim))
@@ -4903,7 +4906,7 @@  strlen_pass::count_nonzero_bytes (tree expr_or_type, gimple *stmt,
 
   ssa_name_limit_t snlim;
   tree expr = expr_or_type;
-  return count_nonzero_bytes (expr, stmt,
+  return count_nonzero_bytes (expr, gimple_vuse (stmt), stmt,
 			      0, 0, lenrange, nulterm, allnul, allnonnul,
 			      snlim);
 }