Correct a workaround for vectorized stores.
authorMartin Sebor <msebor@redhat.com>
Wed, 3 Mar 2021 23:56:45 +0000 (16:56 -0700)
committerMartin Sebor <msebor@redhat.com>
Thu, 4 Mar 2021 00:04:48 +0000 (17:04 -0700)
Resolves:
PR middle-end/96963 - -Wstringop-overflow false positive with -ftree-vectorize when assigning consecutive char struct members
PR middle-end/94655 - -Wstringop-overflow on implicit string assignment with vectorized char store

gcc/ChangeLog:

PR middle-end/96963
PR middle-end/94655
* builtins.c (handle_array_ref): New helper.
(handle_mem_ref): New helper.
(compute_objsize_r): Factor out ARRAY_REF and MEM_REF handling
into new helper functions.  Correct a workaround for vectorized
assignments.

gcc/testsuite/ChangeLog:

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

gcc/builtins.c
gcc/testsuite/gcc.dg/Warray-bounds-69.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
gcc/testsuite/gcc.dg/Wstringop-overflow-65.c [new file with mode: 0644]

index da6dac8..41e336c 100644 (file)
@@ -5209,7 +5209,7 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
   return NULL_TREE;
 }
 
-/* A helper of compute_objsize() to determine the size from an assignment
+/* A helper of compute_objsize_r() to determine the size from an assignment
    statement STMT with the RHS of either MIN_EXPR or MAX_EXPR.  */
 
 static bool
@@ -5287,6 +5287,129 @@ handle_min_max_size (gimple *stmt, int ostype, access_ref *pref,
   return true;
 }
 
+/* A helper of compute_objsize_r() to determine the size from ARRAY_REF
+   AREF.  ADDR is true if PTR is the operand of ADDR_EXPR.  Return true
+   on success and false on failure.  */
+
+static bool
+handle_array_ref (tree aref, bool addr, int ostype, access_ref *pref,
+                 ssa_name_limit_t &snlim, pointer_query *qry)
+{
+  gcc_assert (TREE_CODE (aref) == ARRAY_REF);
+
+  ++pref->deref;
+
+  tree arefop = TREE_OPERAND (aref, 0);
+  tree reftype = TREE_TYPE (arefop);
+  if (!addr && TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE)
+    /* Avoid arrays of pointers.  FIXME: Hande pointers to arrays
+       of known bound.  */
+    return false;
+
+  if (!compute_objsize_r (arefop, ostype, pref, snlim, qry))
+    return false;
+
+  offset_int orng[2];
+  tree off = pref->eval (TREE_OPERAND (aref, 1));
+  range_query *const rvals = qry ? qry->rvals : NULL;
+  if (!get_offset_range (off, NULL, orng, rvals))
+    {
+      /* Set ORNG to the maximum offset representable in ptrdiff_t.  */
+      orng[1] = wi::to_offset (TYPE_MAX_VALUE (ptrdiff_type_node));
+      orng[0] = -orng[1] - 1;
+    }
+
+  /* Convert the array index range determined above to a byte
+     offset.  */
+  tree lowbnd = array_ref_low_bound (aref);
+  if (!integer_zerop (lowbnd) && tree_fits_uhwi_p (lowbnd))
+    {
+      /* Adjust the index by the low bound of the array domain
+        (normally zero but 1 in Fortran).  */
+      unsigned HOST_WIDE_INT lb = tree_to_uhwi (lowbnd);
+      orng[0] -= lb;
+      orng[1] -= lb;
+    }
+
+  tree eltype = TREE_TYPE (aref);
+  tree tpsize = TYPE_SIZE_UNIT (eltype);
+  if (!tpsize || TREE_CODE (tpsize) != INTEGER_CST)
+    {
+      pref->add_max_offset ();
+      return true;
+    }
+
+  offset_int sz = wi::to_offset (tpsize);
+  orng[0] *= sz;
+  orng[1] *= sz;
+
+  if (ostype && TREE_CODE (eltype) == ARRAY_TYPE)
+    {
+      /* Except for the permissive raw memory functions which use
+        the size of the whole object determined above, use the size
+        of the referenced array.  Because the overall offset is from
+        the beginning of the complete array object add this overall
+        offset to the size of array.  */
+      offset_int sizrng[2] =
+       {
+        pref->offrng[0] + orng[0] + sz,
+        pref->offrng[1] + orng[1] + sz
+       };
+      if (sizrng[1] < sizrng[0])
+       std::swap (sizrng[0], sizrng[1]);
+      if (sizrng[0] >= 0 && sizrng[0] <= pref->sizrng[0])
+       pref->sizrng[0] = sizrng[0];
+      if (sizrng[1] >= 0 && sizrng[1] <= pref->sizrng[1])
+       pref->sizrng[1] = sizrng[1];
+    }
+
+  pref->add_offset (orng[0], orng[1]);
+  return true;
+}
+
+/* A helper of compute_objsize_r() to determine the size from MEM_REF
+   MREF.  Return true on success and false on failure.  */
+
+static bool
+handle_mem_ref (tree mref, int ostype, access_ref *pref,
+               ssa_name_limit_t &snlim, pointer_query *qry)
+{
+  gcc_assert (TREE_CODE (mref) == MEM_REF);
+
+  ++pref->deref;
+
+  if (VECTOR_TYPE_P (TREE_TYPE (mref)))
+    {
+      /* 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;
+    }
+
+  tree mrefop = TREE_OPERAND (mref, 0);
+  if (!compute_objsize_r (mrefop, ostype, pref, snlim, qry))
+    return false;
+
+  offset_int orng[2];
+  tree off = pref->eval (TREE_OPERAND (mref, 1));
+  range_query *const rvals = qry ? qry->rvals : NULL;
+  if (!get_offset_range (off, NULL, orng, rvals))
+    {
+      /* Set ORNG to the maximum offset representable in ptrdiff_t.  */
+      orng[1] = wi::to_offset (TYPE_MAX_VALUE (ptrdiff_type_node));
+      orng[0] = -orng[1] - 1;
+    }
+
+  pref->add_offset (orng[0], orng[1]);
+  return true;
+}
+
 /* Helper to compute the size of the object referenced by the PTR
    expression which must have pointer type, using Object Size type
    OSTYPE (only the least significant 2 bits are used).
@@ -5419,92 +5542,11 @@ compute_objsize_r (tree ptr, int ostype, access_ref *pref,
       return true;
     }
 
-  if (code == ARRAY_REF || code == MEM_REF)
-    {
-      ++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;
-
-      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)
-           return false;
-       }
-
-      if (!compute_objsize_r (ref, ostype, pref, snlim, qry))
-       return false;
-
-      offset_int orng[2];
-      tree off = pref->eval (TREE_OPERAND (ptr, 1));
-      if (!get_offset_range (off, NULL, orng, rvals))
-       {
-         /* Set ORNG to the maximum offset representable in ptrdiff_t.  */
-         orng[1] = wi::to_offset (TYPE_MAX_VALUE (ptrdiff_type_node));
-         orng[0] = -orng[1] - 1;
-       }
-
-      if (TREE_CODE (ptr) == ARRAY_REF)
-       {
-         /* Convert the array index range determined above to a byte
-            offset.  */
-         tree lowbnd = array_ref_low_bound (ptr);
-         if (!integer_zerop (lowbnd) && tree_fits_uhwi_p (lowbnd))
-           {
-             /* Adjust the index by the low bound of the array domain
-                (normally zero but 1 in Fortran).  */
-             unsigned HOST_WIDE_INT lb = tree_to_uhwi (lowbnd);
-             orng[0] -= lb;
-             orng[1] -= lb;
-           }
-
-         tree eltype = TREE_TYPE (ptr);
-         tree tpsize = TYPE_SIZE_UNIT (eltype);
-         if (!tpsize || TREE_CODE (tpsize) != INTEGER_CST)
-           {
-             pref->add_max_offset ();
-             return true;
-           }
-
-         offset_int sz = wi::to_offset (tpsize);
-         orng[0] *= sz;
-         orng[1] *= sz;
-
-         if (ostype && TREE_CODE (eltype) == ARRAY_TYPE)
-           {
-             /* Except for the permissive raw memory functions which use
-                the size of the whole object determined above, use the size
-                of the referenced array.  Because the overall offset is from
-                the beginning of the complete array object add this overall
-                offset to the size of array.  */
-             offset_int sizrng[2] =
-               {
-                pref->offrng[0] + orng[0] + sz,
-                pref->offrng[1] + orng[1] + sz
-               };
-             if (sizrng[1] < sizrng[0])
-               std::swap (sizrng[0], sizrng[1]);
-             if (sizrng[0] >= 0 && sizrng[0] <= pref->sizrng[0])
-               pref->sizrng[0] = sizrng[0];
-             if (sizrng[1] >= 0 && sizrng[1] <= pref->sizrng[1])
-               pref->sizrng[1] = sizrng[1];
-           }
-       }
+  if (code == ARRAY_REF)
+    return handle_array_ref (ptr, addr, ostype, pref, snlim, qry);
 
-      pref->add_offset (orng[0], orng[1]);
-      return true;
-    }
+  if (code == MEM_REF)
+    return handle_mem_ref (ptr, ostype, pref, snlim, qry);
 
   if (code == TARGET_MEM_REF)
     {
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-69.c b/gcc/testsuite/gcc.dg/Warray-bounds-69.c
new file mode 100644 (file)
index 0000000..5a95577
--- /dev/null
@@ -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);
+}
index cb2c329..9bfc84a 100644 (file)
@@ -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" "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 (file)
index 0000000..9f82d73
--- /dev/null
@@ -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;
+}