[PATCH] correct fix to avoid false positives for vectorized stores (PR 96963, 94655)

[Martin Sebor <msebor@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

The hack I put in compute_objsize() last January for pr93200 isn't
quite correct.  It happened to suppress the false positive there
but, due to what looks like a thinko on my part, not in some other
test cases involving vectorized stores.

The attached change adjusts the hack to have compute_objsize() give
up on all MEM_REFs with a vector type.  This effectively disables
-Wstringop-{overflow,overread} for vector accesses (either written
by the user or synthesized by GCC from ordinary accesses).  It
doesn't affect -Warray-bounds because this warning doesn't use
compute_objsize() yet.  When it does (it should considerably
simplify the code) some additional changes will be needed to
preserve -Warray-bounds for out of bounds vector accesses.
The test this patch adds should serve as a reminder to make
it if we forget.

Tested on x86_64-linux.  Since PR 94655 was reported against GCC
10 I'd like to apply this fix to both the trunk and the 10 branch.

Martin

[-- Attachment #2: gcc-96963.diff --]
[-- Type: text/x-patch, Size: 8384 bytes --]

PR middle-end/96963 - -Wstringop-overflow false positive with -ftree-vectorize when assigning consecutive char struct members

gcc/ChangeLog:

	PR middle-end/96963
	* builtins.c (compute_objsize_r): Correct a workaround for vectorized
	assignments.

gcc/testsuite/ChangeLog:

	PR middle-end/96963
	* gcc.dg/Wstringop-overflow-65.c: New test.
	* gcc.dg/Warray-bounds-69.c: Same.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 0aed008687c..2ffe472d4ea 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5425,24 +5425,33 @@ compute_objsize_r (tree ptr, int ostype, access_ref *pref,
       ++pref->deref;
 
       tree ref = TREE_OPERAND (ptr, 0);
-      tree reftype = TREE_TYPE (ref);
-      if (!addr && code == ARRAY_REF
-	  && TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE)
-	/* Avoid arrays of pointers.  FIXME: Hande pointers to arrays
-	   of known bound.  */
-	return false;
+      {
+	tree reftype = TREE_TYPE (ref);
+	if (!addr && code == ARRAY_REF
+	    && TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE)
+	  /* Avoid arrays of pointers.  FIXME: Hande pointers to arrays
+	     of known bound.  */
+	  return false;
+      }
+      {
+	tree ptrtype = TREE_TYPE (ptr);
+	if (POINTER_TYPE_P (ptrtype))
+	  ptrtype = TREE_TYPE (ptrtype);
 
-      if (code == MEM_REF && TREE_CODE (reftype) == POINTER_TYPE)
-	{
-	  /* Give up for MEM_REFs of vector types; those may be synthesized
-	     from multiple assignments to consecutive data members.  See PR
-	     93200.
-	     FIXME: Deal with this more generally, e.g., by marking up such
-	     MEM_REFs at the time they're created.  */
-	  reftype = TREE_TYPE (reftype);
-	  if (TREE_CODE (reftype) == VECTOR_TYPE)
+	if (code == MEM_REF && TREE_CODE (ptrtype) == VECTOR_TYPE)
+	  {
+	    /* Hack: Give up for MEM_REFs of vector types; those may be
+	       synthesized from multiple assignments to consecutive data
+	       members (see PR 93200 and 96963).
+	       FIXME: Vectorized assignments should only be present after
+	       vectorization so this hack is only necessary after it has
+	       run and could be avoided in calls from prior passes (e.g.,
+	       tree-ssa-strlen.c).
+	       FIXME: Deal with this more generally, e.g., by marking up
+	       such MEM_REFs at the time they're created.  */
 	    return false;
-	}
+	  }
+      }
 
       if (!compute_objsize_r (ref, ostype, pref, snlim, qry))
 	return false;
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-69.c b/gcc/testsuite/gcc.dg/Warray-bounds-69.c
new file mode 100644
index 00000000000..5a955774124
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-69.c
@@ -0,0 +1,74 @@
+/* Verify that storing a bigger vector into smaller space is diagnosed.
+   { dg-do compile }
+   { dg-options "-O2 -Warray-bounds" } */
+
+typedef __INT16_TYPE__                         int16_t;
+typedef __attribute__ ((__vector_size__ (32))) char C32;
+
+typedef __attribute__ ((__vector_size__ (64))) int16_t I16_64;
+
+void sink (void*);
+
+
+void nowarn_c32 (char c)
+{
+  extern char nowarn_a32[32];
+
+  void *p = nowarn_a32;
+  *(C32*)p = (C32){ c };
+  sink (p);
+
+  char a32[32];
+  p = a32;
+  *(C32*)p = (C32){ c };
+  sink (p);
+}
+
+/* The invalid stores below are diagnosed by -Warray-bounds only
+   because it doesn't use compute_objsize().  If/when that changes
+   the function might need adjusting to avoid the hack put in place
+   to avoid false positives due to vectorization.  */
+
+void warn_c32 (char c)
+{
+  extern char warn_a32[32];   // { dg-message "'warn_a32'" "note" }
+
+  void *p = warn_a32 + 1;
+  *(C32*)p = (C32){ c };      // { dg-warning "\\\[-Warray-bounds" }
+
+  /* Verify a local variable too. */
+  char a32[32];               // { dg-message "'a32'" }
+  p = a32 + 1;
+  *(C32*)p = (C32){ c };      // { dg-warning "\\\[-Warray-bounds" }
+  sink (p);
+}
+
+
+void nowarn_i16_64 (int16_t i)
+{
+  extern char nowarn_a64[64];
+
+  void *p = nowarn_a64;
+  I16_64 *q = (I16_64*)p;
+  *q = (I16_64){ i };
+
+  char a64[64];
+  q = (I16_64*)a64;
+  *q = (I16_64){ i };
+  sink (q);
+}
+
+void warn_i16_64 (int16_t i)
+{
+  extern char warn_a64[64];   // { dg-message "'warn_a64'" }
+
+  void *p = warn_a64 + 1;
+  I16_64 *q = (I16_64*)p;
+  *q = (I16_64){ i };         // { dg-warning "\\\[-Warray-bounds" }
+
+  char a64[64];               // { dg-message "'a64'" }
+  p = a64 + 1;
+  q = (I16_64*)p;
+  *q = (I16_64){ i };         // { dg-warning "\\\[-Warray-bounds" }
+  sink (p);
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
index cb2c329aa84..cc28d22864c 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
@@ -24,17 +24,22 @@ void nowarn_c32 (char c)
   sink (p);
 }
 
+/* The tests below fail as a result of the hack for PR 96963.  However,
+   with -Wall, the invalid stores are diagnosed by -Warray-bounds which
+   runs before vectorization and so doesn't need the hack.  If/when
+   -Warray changes to use compute_objsize() this will need adjusting.  */
+
 void warn_c32 (char c)
 {
-  extern char warn_a32[32];   // { dg-message "at offset 32 into destination object 'warn_a32' of size 32" "note" }
+  extern char warn_a32[32];   // { dg-message "at offset 32 into destination object 'warn_a32' of size 32" "note" "pr97027" { xfail *-*-* } }
 
   void *p = warn_a32 + 1;
-  *(C32*)p = (C32){ c };      // { dg-warning "writing 1 byte into a region of size 0" }
+  *(C32*)p = (C32){ c };      // { dg-warning "writing 1 byte into a region of size 0" "pr97027" { xfail *-*-* } }
 
   /* Verify a local variable too. */
   char a32[32];
   p = a32 + 1;
-  *(C32*)p = (C32){ c };      // { dg-warning "writing 1 byte into a region of size 0" }
+  *(C32*)p = (C32){ c };      // { dg-warning "writing 1 byte into a region of size 0"  "pr97027" { xfail *-*-* } }
   sink (p);
 }
 
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-65.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-65.c
new file mode 100644
index 00000000000..d3e301dbc2f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-65.c
@@ -0,0 +1,98 @@
+/* PR middle-end/96963 - -Wstringop-overflow false positive with
+   -ftree-vectorize when assigning consecutive char struct members
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftree-vectorize" } */
+
+void sink (void*);
+
+struct Char
+{
+  int i;
+  char c, d, e, f;
+  char a[2], b[2];
+};
+
+void nowarn_char_assign (struct Char *p)
+{
+  sink (&p->c);
+
+  /* Verify the bogus warning triggered by the tree-ssa-strlen.c pass
+     is not issued.  */
+  p->c = 1;         // { dg-bogus "\\\[-Wstringop-overflow"] }
+  p->d = 2;
+  p->e = 3;
+  p->f = 4;
+}
+
+void nowarn_char_array_assign (struct Char *p)
+{
+  sink (p->a);
+
+  p->a[0] = 1;      // { dg-bogus "\\\[-Wstringop-overflow"] }
+  p->a[1] = 2;
+  p->b[0] = 3;
+  p->b[1] = 4;
+}
+
+void warn_char_array_assign_interior (struct Char *p)
+{
+  sink (p->a);
+
+  p->a[0] = 1;
+  p->a[1] = 2;
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+  /* Warnings are only suppressed for trailing arrays.  Verify
+     one is issued for an interior array.  */
+  p->a[2] = 5;      // { dg-warning "\\\[-Wstringop-overflow" }
+#pragma GCC diagnostic pop
+}
+
+void warn_char_array_assign_trailing (struct Char *p)
+{
+  /* This is separated from warn_char_array_assign_interior because
+     otherwise GCC removes the store to p->a[2] as dead since it's
+     overwritten by p->b[0].  */
+  sink (p->b);
+
+  p->b[0] = 3;
+  p->b[1] = 4;
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+  /* Warnings are only suppressed for trailing arrays with at most
+     one element.  Verify one is issued for a two-element array.  */
+  p->b[2] = 5;      // { dg-warning "\\\[-Wstringop-overflow" }
+#pragma GCC diagnostic pop
+}
+
+
+/* Also verify there's no warning for other types than char (even though
+   the problem was limited to chars and -Wstringop-overflow should only
+   trigger for character accesses).  */
+
+struct Short
+{
+  int i;
+  short c, d, e, f;
+  short a[2], b[2];
+};
+
+void nowarn_short_assign (struct Short *p)
+{
+  sink (&p->c);
+
+  p->c = 1;
+  p->d = 2;
+  p->e = 3;
+  p->f = 4;
+}
+
+void nowarn_short_array_assign (struct Short *p)
+{
+  sink (p->a);
+
+  p->a[0] = 1;
+  p->a[1] = 2;
+  p->b[0] = 3;
+  p->b[1] = 4;
+}