public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
@ 2019-08-16 23:36 Martin Sebor
  2019-08-19 15:08 ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2019-08-16 23:36 UTC (permalink / raw)
  To: gcc-patches

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

With the recent enhancement to the strlen handling of multibyte
stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
started failing on hppa (and probably elsewhere as well).  This
is partly the result of the added detection of past-the-end
writes into the strlen pass which detects more instances of
the problem than -Warray-bounds.  Since the IL each warning
works with varies between targets, the same invalid code can
be diagnosed by one warning one target and different warning
on another.

The attached patch does three things:

1) It enhances compute_objsize to also determine the size of
a flexible array member (and its various variants), including
from its initializer if necessary.  (This resolves 91457 but
introduces another warning where was previously just one.)
2) It guards the new instance of -Wstringop-overflow with
the no-warning bit on the assignment to avoid warning on code
that's already been diagnosed.
3) It arranges for -Warray-bounds to set the no-warning bit on
the enclosing expression to keep -Wstringop-overflow from issuing
another warning for the same problem.

Testing the compute_objsize enhancement to bring it up to par
with -Warray-bounds in turn exposed a weakness in the latter
warning for flexible array members.  Rather than snowballing
additional improvements into this one I decided to put that
off until later, so the new -Warray-bounds test has a bunch
of XFAILs.  I'll see if I can find the time to deal with those
either still in stage 1 or in stage 3 (one of them is actually
an ancient regression).

Martin

PS I imagine the new get_flexarray_size function will probably
need to move somewhere more appropriate so that other warnings
(like -Warray-bounds to remove the XFAILs) and optimizations
can make use of it.

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

PR tree-optimization/91458 - inconsistent warning for writing past the end of an array member

gcc/testsuite/ChangeLog:

	PR tree-optimization/91458
	* g++.dg/warn/Warray-bounds-8.C: New test.
	* g++.dg/warn/Wstringop-overflow-3.C: New test.

gcc/ChangeLog:

	PR tree-optimization/91458
	* builtins.c (get_initializer_for, get_flexarray_size): New functions.
	(compute_objsize): Add argument. Handle ARRAY_REF and COMPONENT_REF.
	* builtins.h ((compute_objsize): Add argument.
	* tree-ssa-strlen.c (handle_store): Handle no-warning bit.
	* tree-vrp.c (vrp_prop::check_array_ref): Return warning result.
	(vrp_prop::check_mem_ref): Same.
	(vrp_prop::search_for_addr_array): Set no-warning bit.
	(check_array_bounds): Same.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 274541)
+++ gcc/builtins.c	(working copy)
@@ -3560,6 +3560,51 @@ check_access (tree exp, tree, tree, tree dstwrite,
   return true;
 }
 
+/* Given the initializer INIT, return the initializer for the field
+   DECL if it exists, otherwise null.  Used to obtain the initializer
+   for a flexible array member and determine its size.  */
+
+static tree
+get_initializer_for (tree init, tree decl)
+{
+  STRIP_NOPS (init);
+
+  tree fld, fld_init;
+  unsigned HOST_WIDE_INT i;
+  FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (init), i, fld, fld_init)
+    {
+      if (decl == fld)
+	return fld_init;
+
+      if (TREE_CODE (fld) == CONSTRUCTOR)
+	{
+	  fld_init = get_initializer_for (fld_init, decl);
+	  if (fld_init)
+	    return fld_init;
+	}
+    }
+
+  return NULL_TREE;
+}
+
+/* Determine the size of the flexible array FLD from the initializer
+   expression for the struct object DECL in which the meber is declared
+   (possibly recursively).  Return the size or zero constant if it isn't
+   initialized.  */
+
+static tree
+get_flexarray_size (tree decl, tree fld)
+{
+  if (tree init = DECL_INITIAL (decl))
+    {
+      init = get_initializer_for (init, fld);
+      if (init)
+	return TYPE_SIZE_UNIT (TREE_TYPE (init));
+    }
+
+  return integer_zero_node;
+}
+
 /* Helper to compute the size of the object referenced by the DEST
    expression which must have pointer type, using Object Size type
    OSTYPE (only the least significant 2 bits are used).  Return
@@ -3567,12 +3612,18 @@ check_access (tree exp, tree, tree, tree dstwrite,
    the size cannot be determined.  When the referenced object involves
    a non-constant offset in some range the returned value represents
    the largest size given the smallest non-negative offset in the
-   range.  The function is intended for diagnostics and should not
-   be used to influence code generation or optimization.  */
+   range.  If nonnull, set *PDECL to the decl of the referenced
+   subobject if it can be determined, or to null otherwise.
+   The function is intended for diagnostics and should not be used
+   to influence code generation or optimization.  */
 
 tree
-compute_objsize (tree dest, int ostype)
+compute_objsize (tree dest, int ostype, tree *pdecl /* = NULL */)
 {
+  tree dummy = NULL_TREE;
+  if (!pdecl)
+    pdecl = &dummy;
+
   unsigned HOST_WIDE_INT size;
 
   /* Only the two least significant bits are meaningful.  */
@@ -3599,7 +3650,7 @@ tree
 	  tree off = gimple_assign_rhs2 (stmt);
 	  if (TREE_CODE (off) == INTEGER_CST)
 	    {
-	      if (tree size = compute_objsize (dest, ostype))
+	      if (tree size = compute_objsize (dest, ostype, pdecl))
 		{
 		  wide_int wioff = wi::to_wide (off);
 		  wide_int wisiz = wi::to_wide (size);
@@ -3624,7 +3675,7 @@ tree
 
 	      if (rng == VR_RANGE)
 		{
-		  if (tree size = compute_objsize (dest, ostype))
+		  if (tree size = compute_objsize (dest, ostype, pdecl))
 		    {
 		      wide_int wisiz = wi::to_wide (size);
 
@@ -3652,12 +3703,32 @@ tree
   if (!ostype)
     return NULL_TREE;
 
-  if (TREE_CODE (dest) == MEM_REF)
+  if (TREE_CODE (dest) == ARRAY_REF
+      || TREE_CODE (dest) == MEM_REF)
     {
       tree ref = TREE_OPERAND (dest, 0);
       tree off = TREE_OPERAND (dest, 1);
-      if (tree size = compute_objsize (ref, ostype))
+      if (tree size = compute_objsize (ref, ostype, pdecl))
 	{
+	  /* If the declaration of the destination object is known to
+	     have zero size, return zero.  Otherwise, if the returned
+	     size is zero and the idex/offset is unsigned, also return
+	     zero.  */
+	  if (integer_zerop (size))
+	    return integer_zero_node;
+
+	  if (TREE_CODE (off) != INTEGER_CST)
+	    return NULL_TREE;
+
+	  if (TREE_CODE (dest) == ARRAY_REF)
+	    {
+	      tree eltype = TREE_TYPE (dest);
+	      if (tree tpsize = TYPE_SIZE_UNIT (eltype))
+		off = fold_build2 (MULT_EXPR, size_type_node, off, tpsize);
+	      else
+		return NULL_TREE;
+	    }
+
 	  if (tree_int_cst_lt (off, size))
 	    return fold_build2 (MINUS_EXPR, size_type_node, size, off);
 	  return integer_zero_node;
@@ -3666,9 +3737,34 @@ tree
       return NULL_TREE;
     }
 
+  if (TREE_CODE (dest) == COMPONENT_REF)
+    {
+      *pdecl = TREE_OPERAND (dest, 1);
+
+      /* If the member has a size return it.  Otherwise it's a flexible
+	 array member.  */
+      if (tree size = DECL_SIZE_UNIT (*pdecl))
+	return size;
+
+      /* If the destination is a declared object with zero size try to
+	 determine its size from its initializer.  */
+      if (tree base = get_base_address (dest))
+	if (VAR_P (base))
+	  return get_flexarray_size (base, *pdecl);
+
+      return NULL_TREE;
+    }
+
   if (TREE_CODE (dest) != ADDR_EXPR)
     return NULL_TREE;
 
+  tree ref = TREE_OPERAND (dest, 0);
+  if (DECL_P (ref))
+    {
+      *pdecl = ref;
+      return DECL_SIZE_UNIT (ref);
+    }
+
   tree type = TREE_TYPE (dest);
   if (TREE_CODE (type) == POINTER_TYPE)
     type = TREE_TYPE (type);
@@ -3678,12 +3774,8 @@ tree
   if (TREE_CODE (type) == ARRAY_TYPE
       && !array_at_struct_end_p (TREE_OPERAND (dest, 0)))
     {
-      /* Return the constant size unless it's zero (that's a zero-length
-	 array likely at the end of a struct).  */
-      tree size = TYPE_SIZE_UNIT (type);
-      if (size && TREE_CODE (size) == INTEGER_CST
-	  && !integer_zerop (size))
-	return size;
+      if (tree size = TYPE_SIZE_UNIT (type))
+	return TREE_CODE (size) == INTEGER_CST ? size : NULL_TREE;
     }
 
   return NULL_TREE;
Index: gcc/builtins.h
===================================================================
--- gcc/builtins.h	(revision 274541)
+++ gcc/builtins.h	(working copy)
@@ -134,7 +134,7 @@ extern tree fold_call_stmt (gcall *, bool);
 extern void set_builtin_user_assembler_name (tree decl, const char *asmspec);
 extern bool is_simple_builtin (tree);
 extern bool is_inexpensive_builtin (tree);
-extern tree compute_objsize (tree, int);
+extern tree compute_objsize (tree, int, tree * = NULL);
 
 extern bool readonly_data_expr (tree exp);
 extern bool init_target_chars (void);
Index: gcc/testsuite/g++.dg/warn/Warray-bounds-8.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Warray-bounds-8.C	(nonexistent)
+++ gcc/testsuite/g++.dg/warn/Warray-bounds-8.C	(working copy)
@@ -0,0 +1,61 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+struct A0
+{
+  char n, a[0];
+
+  A0 () { a[0] = 0; }           // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void f (void*);
+
+void g0 (void)
+{
+  struct A0 a;
+  f (&a);
+}
+
+
+struct A1
+{
+  char n, a[1];
+
+  A1 () { a[1] = 0; }           // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void g1 (void)
+{
+  struct A1 a;
+  f (&a);
+}
+
+
+struct A123
+{
+  char n, a[123];
+
+  A123 () { a[123] = 0; }       // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void g123 (void)
+{
+  struct A123 a;
+  f (&a);
+}
+
+
+struct A234
+{
+  char n, a[234];
+
+  A234 (int i) { a[i] = 0; }    // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void g234 (void)
+{
+  struct A234 a (234);
+  f (&a);
+}
Index: gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C	(nonexistent)
+++ gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C	(working copy)
@@ -0,0 +1,112 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-array-bounds" } */
+
+struct Axi
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }    
+};
+
+Axi ax2 = { 2, { 1, 0 } };
+
+void gx2 ()
+{
+  ax2.a[0] = 0;
+  ax2.a[1] = 0;
+  ax2.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+void gxx (Axi *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+struct Ax
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }
+
+  // Verify the warning for a constant.
+  Ax () { a[0] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+
+  // And also for a non-constant.  Regardless of the subscript, the array
+  // of the object in function gxi() below has a zero size.
+  Ax (int i) { a[i] = 0; }      // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void f (void*);
+
+void gx (void)
+{
+  struct Ax a;
+  f (&a);
+}
+
+void gxi (int i)
+{
+  struct Ax a (i);
+  f (&a);
+}
+
+struct A0
+{
+  char n;
+  char a[0];                    // { dg-message "destination object declared here" }
+
+  A0 () { a[0] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void f (void*);
+
+void g0 (void)
+{
+  struct A0 a;
+  f (&a);
+}
+
+
+struct A1
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+
+  A1 () { a[1] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void g1 (void)
+{
+  struct A1 a;
+  f (&a);
+}
+
+
+struct A123
+{
+  char a[123];                  // { dg-message "destination object declared here" }
+
+  A123 () { a[123] = 0; }       // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void g123 (void)
+{
+  struct A123 a;
+  f (&a);
+}
+
+
+struct A234
+{
+  char a[234];                  // { dg-message "destination object declared here" }
+
+  A234 (int i) { a[i] = 0; }    // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void g234 (void)
+{
+  struct A234 a (234);
+  f (&a);
+}
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 274541)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -3679,16 +3679,30 @@ handle_store (gimple_stmt_iterator *gsi)
       rhs_minlen = lenrange[0];
       storing_nonzero_p = lenrange[1] > 0;
 
-      if (tree dstsize = compute_objsize (lhs, 1))
-	if (compare_tree_int (dstsize, lenrange[2]) < 0)
-	  {
-	    location_t loc = gimple_nonartificial_location (stmt);
-	    warning_n (loc, OPT_Wstringop_overflow_,
-		       lenrange[2],
-		       "%Gwriting %u byte into a region of size %E",
-		       "%Gwriting %u bytes into a region of size %E",
-		       stmt, lenrange[2], dstsize);
-	  }
+      /* Avoid issuing multiple warnings for the same LHS or statement.
+	 For example, -Warray-bounds may have already been issued for
+	 an out-of-bounds subscript.  */
+      if (!TREE_NO_WARNING (lhs) && !gimple_no_warning_p (stmt))
+	{
+	  /* Set to the declaration referenced by LHS (if known).  */
+	  tree decl = NULL_TREE;
+	  if (tree dstsize = compute_objsize (lhs, 1, &decl))
+	    if (compare_tree_int (dstsize, lenrange[2]) < 0)
+	      {
+		location_t loc = gimple_nonartificial_location (stmt);
+		if (warning_n (loc, OPT_Wstringop_overflow_,
+			       lenrange[2],
+			       "%Gwriting %u byte into a region of size %E",
+			       "%Gwriting %u bytes into a region of size %E",
+			       stmt, lenrange[2], dstsize))
+		  {
+		    if (decl)
+		      inform (DECL_SOURCE_LOCATION (decl),
+			      "destination object declared here");
+		    gimple_set_no_warning (stmt, true);
+		  }
+	      }
+	}
     }
   else
     {
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 274541)
+++ gcc/tree-vrp.c	(working copy)
@@ -4354,8 +4354,8 @@ class vrp_prop : public ssa_propagation_engine
   void vrp_initialize (void);
   void vrp_finalize (bool);
   void check_all_array_refs (void);
-  void check_array_ref (location_t, tree, bool);
-  void check_mem_ref (location_t, tree, bool);
+  bool check_array_ref (location_t, tree, bool);
+  bool check_mem_ref (location_t, tree, bool);
   void search_for_addr_array (tree, location_t);
 
   class vr_values vr_values;
@@ -4381,9 +4381,10 @@ class vrp_prop : public ssa_propagation_engine
    array subscript is a constant, check if it is outside valid
    range. If the array subscript is a RANGE, warn if it is
    non-overlapping with valid range.
-   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
+   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.
+   Returns true if a warning has been issued.  */
 
-void
+bool
 vrp_prop::check_array_ref (location_t location, tree ref,
 			   bool ignore_off_by_one)
 {
@@ -4392,7 +4393,7 @@ vrp_prop::check_array_ref (location_t location, tr
   tree low_bound, up_bound, up_bound_p1;
 
   if (TREE_NO_WARNING (ref))
-    return;
+    return false;
 
   low_sub = up_sub = TREE_OPERAND (ref, 1);
   up_bound = array_ref_up_bound (ref);
@@ -4513,6 +4514,8 @@ vrp_prop::check_array_ref (location_t location, tr
 
       TREE_NO_WARNING (ref) = 1;
     }
+
+  return warned;
 }
 
 /* Checks one MEM_REF in REF, located at LOCATION, for out-of-bounds
@@ -4522,14 +4525,15 @@ vrp_prop::check_array_ref (location_t location, tr
    with valid range.
    IGNORE_OFF_BY_ONE is true if the MEM_REF is inside an ADDR_EXPR
    (used to allow one-past-the-end indices for code that takes
-   the address of the just-past-the-end element of an array).  */
+   the address of the just-past-the-end element of an array).
+   Returns true if a warning has been issued.  */
 
-void
+bool
 vrp_prop::check_mem_ref (location_t location, tree ref,
 			 bool ignore_off_by_one)
 {
   if (TREE_NO_WARNING (ref))
-    return;
+    return false;
 
   tree arg = TREE_OPERAND (ref, 0);
   /* The constant and variable offset of the reference.  */
@@ -4581,7 +4585,7 @@ vrp_prop::check_mem_ref (location_t location, tree
 	  continue;
 	}
       else
-	return;
+	return false;
 
       /* VAROFF should always be a SSA_NAME here (and not even
 	 INTEGER_CST) but there's no point in taking chances.  */
@@ -4643,10 +4647,10 @@ vrp_prop::check_mem_ref (location_t location, tree
       arg = TREE_OPERAND (arg, 0);
       if (TREE_CODE (arg) != STRING_CST
 	  && TREE_CODE (arg) != VAR_DECL)
-	return;
+	return false;
     }
   else
-    return;
+    return false;
 
   /* The type of the object being referred to.  It can be an array,
      string literal, or a non-array type when the MEM_REF represents
@@ -4661,7 +4665,7 @@ vrp_prop::check_mem_ref (location_t location, tree
       || !COMPLETE_TYPE_P (reftype)
       || TREE_CODE (TYPE_SIZE_UNIT (reftype)) != INTEGER_CST
       || RECORD_OR_UNION_TYPE_P (reftype))
-    return;
+    return false;
 
   offset_int eltsize;
   if (TREE_CODE (reftype) == ARRAY_TYPE)
@@ -4763,11 +4767,11 @@ vrp_prop::check_mem_ref (location_t location, tree
 
       if (warned)
 	TREE_NO_WARNING (ref) = 1;
-      return;
+      return warned;
     }
 
   if (warn_array_bounds < 2)
-    return;
+    return false;
 
   /* At level 2 check also intermediate offsets.  */
   int i = 0;
@@ -4778,8 +4782,13 @@ vrp_prop::check_mem_ref (location_t location, tree
       if (warning_at (location, OPT_Warray_bounds,
 		      "intermediate array offset %wi is outside array bounds "
 		      "of %qT", tmpidx, reftype))
-	TREE_NO_WARNING (ref) = 1;
+	{
+	  TREE_NO_WARNING (ref) = 1;
+	  return true;
+	}
     }
+
+  return false;
 }
 
 /* Searches if the expr T, located at LOCATION computes
@@ -4791,11 +4800,15 @@ vrp_prop::search_for_addr_array (tree t, location_
   /* Check each ARRAY_REF and MEM_REF in the reference chain. */
   do
     {
+      bool warned = false;
       if (TREE_CODE (t) == ARRAY_REF)
-	check_array_ref (location, t, true /*ignore_off_by_one*/);
+	warned = check_array_ref (location, t, true /*ignore_off_by_one*/);
       else if (TREE_CODE (t) == MEM_REF)
-	check_mem_ref (location, t, true /*ignore_off_by_one*/);
+	warned = check_mem_ref (location, t, true /*ignore_off_by_one*/);
 
+      if (warned)
+	TREE_NO_WARNING (t) = true;
+
       t = TREE_OPERAND (t, 0);
     }
   while (handled_component_p (t) || TREE_CODE (t) == MEM_REF);
@@ -4886,16 +4899,20 @@ check_array_bounds (tree *tp, int *walk_subtree, v
 
   *walk_subtree = TRUE;
 
+  bool warned = false;
   vrp_prop *vrp_prop = (class vrp_prop *)wi->info;
   if (TREE_CODE (t) == ARRAY_REF)
-    vrp_prop->check_array_ref (location, t, false /*ignore_off_by_one*/);
+    warned = vrp_prop->check_array_ref (location, t, false/*ignore_off_by_one*/);
   else if (TREE_CODE (t) == MEM_REF)
-    vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/);
+    warned = vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/);
   else if (TREE_CODE (t) == ADDR_EXPR)
     {
       vrp_prop->search_for_addr_array (t, location);
       *walk_subtree = FALSE;
     }
+  /* Propagate the no-warning bit to the outer expression.  */
+  if (warned)
+    TREE_NO_WARNING (t) = true;
 
   return NULL_TREE;
 }


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-16 23:36 [PATCH] issue consistent warning for past-the-end array stores (PR 91457) Martin Sebor
@ 2019-08-19 15:08 ` Richard Biener
  2019-08-20  8:31   ` Martin Sebor
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2019-08-19 15:08 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
>
> With the recent enhancement to the strlen handling of multibyte
> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
> started failing on hppa (and probably elsewhere as well).  This
> is partly the result of the added detection of past-the-end
> writes into the strlen pass which detects more instances of
> the problem than -Warray-bounds.  Since the IL each warning
> works with varies between targets, the same invalid code can
> be diagnosed by one warning one target and different warning
> on another.
>
> The attached patch does three things:
>
> 1) It enhances compute_objsize to also determine the size of
> a flexible array member (and its various variants), including
> from its initializer if necessary.  (This resolves 91457 but
> introduces another warning where was previously just one.)
> 2) It guards the new instance of -Wstringop-overflow with
> the no-warning bit on the assignment to avoid warning on code
> that's already been diagnosed.
> 3) It arranges for -Warray-bounds to set the no-warning bit on
> the enclosing expression to keep -Wstringop-overflow from issuing
> another warning for the same problem.
>
> Testing the compute_objsize enhancement to bring it up to par
> with -Warray-bounds in turn exposed a weakness in the latter
> warning for flexible array members.  Rather than snowballing
> additional improvements into this one I decided to put that
> off until later, so the new -Warray-bounds test has a bunch
> of XFAILs.  I'll see if I can find the time to deal with those
> either still in stage 1 or in stage 3 (one of them is actually
> an ancient regression).

+static tree
+get_initializer_for (tree init, tree decl)
+{

can't you use fold_ctor_reference here?

+/* Determine the size of the flexible array FLD from the initializer
+   expression for the struct object DECL in which the meber is declared
+   (possibly recursively).  Return the size or zero constant if it isn't
+   initialized.  */
+
+static tree
+get_flexarray_size (tree decl, tree fld)
+{
+  if (tree init = DECL_INITIAL (decl))
+    {
+      init = get_initializer_for (init, fld);
+      if (init)
+       return TYPE_SIZE_UNIT (TREE_TYPE (init));
+    }
+
+  return integer_zero_node;

so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
returns has a complete type but the initialized object didn't get it
completed.  Isnt that wishful thinking?  And why return integer_zero_node
rather than NULL_TREE here?

+  if (TREE_CODE (dest) == COMPONENT_REF)
+    {
+      *pdecl = TREE_OPERAND (dest, 1);
+
+      /* If the member has a size return it.  Otherwise it's a flexible
+        array member.  */
+      if (tree size = DECL_SIZE_UNIT (*pdecl))
+       return size;

because here you do.

Also once you have an underlying VAR_DECL you can compute
the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
Isn't that way cheaper than walking the initializer (possibly many
times?)

Richard.

>
> Martin
>
> PS I imagine the new get_flexarray_size function will probably
> need to move somewhere more appropriate so that other warnings
> (like -Warray-bounds to remove the XFAILs) and optimizations
> can make use of it.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-19 15:08 ` Richard Biener
@ 2019-08-20  8:31   ` Martin Sebor
  2019-08-20  8:47     ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2019-08-20  8:31 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

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

On 8/19/19 8:10 AM, Richard Biener wrote:
> On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> With the recent enhancement to the strlen handling of multibyte
>> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
>> started failing on hppa (and probably elsewhere as well).  This
>> is partly the result of the added detection of past-the-end
>> writes into the strlen pass which detects more instances of
>> the problem than -Warray-bounds.  Since the IL each warning
>> works with varies between targets, the same invalid code can
>> be diagnosed by one warning one target and different warning
>> on another.
>>
>> The attached patch does three things:
>>
>> 1) It enhances compute_objsize to also determine the size of
>> a flexible array member (and its various variants), including
>> from its initializer if necessary.  (This resolves 91457 but
>> introduces another warning where was previously just one.)
>> 2) It guards the new instance of -Wstringop-overflow with
>> the no-warning bit on the assignment to avoid warning on code
>> that's already been diagnosed.
>> 3) It arranges for -Warray-bounds to set the no-warning bit on
>> the enclosing expression to keep -Wstringop-overflow from issuing
>> another warning for the same problem.
>>
>> Testing the compute_objsize enhancement to bring it up to par
>> with -Warray-bounds in turn exposed a weakness in the latter
>> warning for flexible array members.  Rather than snowballing
>> additional improvements into this one I decided to put that
>> off until later, so the new -Warray-bounds test has a bunch
>> of XFAILs.  I'll see if I can find the time to deal with those
>> either still in stage 1 or in stage 3 (one of them is actually
>> an ancient regression).
> 
> +static tree
> +get_initializer_for (tree init, tree decl)
> +{
> 
> can't you use fold_ctor_reference here?

Yes, but only with an additional enhancement.  Char initializers
for flexible array members aren't transformed to STRING_CSTs yet,
so without the size of the initializer specified, the function
returns the initializer for the smallest subobject, or char in
this case.  I've enhanced the function to handle them.

> 
> +/* Determine the size of the flexible array FLD from the initializer
> +   expression for the struct object DECL in which the meber is declared
> +   (possibly recursively).  Return the size or zero constant if it isn't
> +   initialized.  */
> +
> +static tree
> +get_flexarray_size (tree decl, tree fld)
> +{
> +  if (tree init = DECL_INITIAL (decl))
> +    {
> +      init = get_initializer_for (init, fld);
> +      if (init)
> +       return TYPE_SIZE_UNIT (TREE_TYPE (init));
> +    }
> +
> +  return integer_zero_node;
> 
> so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
> returns has a complete type but the initialized object didn't get it
> completed.  Isnt that wishful thinking?

I don't know what you mean.  When might a CONSTRUCTOR not have
a complete type, and if/when it doesn't, why would that be
a problem here?  TYPE_SIZE_UNIT will evaluate to null meaning
"don't know" and that's fine.  Could you try to be more specific
about the problem you're pointing out?

> And why return integer_zero_node
> rather than NULL_TREE here?

Because the size of a flexible array member with no initializer
is zero.

> 
> +  if (TREE_CODE (dest) == COMPONENT_REF)
> +    {
> +      *pdecl = TREE_OPERAND (dest, 1);
> +
> +      /* If the member has a size return it.  Otherwise it's a flexible
> +        array member.  */
> +      if (tree size = DECL_SIZE_UNIT (*pdecl))
> +       return size;
> 
> because here you do.

Not sure what you mean here either.  (This code was also a bit
out of date WRT to the patch I had tested.  Not sure how that
happened.  The attached patch is up to date.)

> 
> Also once you have an underlying VAR_DECL you can compute
> the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
> Isn't that way cheaper than walking the initializer (possibly many
> times?)

It would be nice if it were this easy.  Is the value of DECL_SIZE
(var) supposed to include the size of the flexible array member?
I don't see it mentioned in the comments in tree.h and in my tests
it only does in C but not in C++.  Is that a bug that in C++ it
doesn't?

Attached is an updated patch that uses fold_ctor_reference as you
suggested.  I've also made a few other minor changes to diagnose
a few more invalid strlen calls with out-of-bounds offsets.
(More still remain.)

Martin

> 
> Richard.
> 
>>
>> Martin
>>
>> PS I imagine the new get_flexarray_size function will probably
>> need to move somewhere more appropriate so that other warnings
>> (like -Warray-bounds to remove the XFAILs) and optimizations
>> can make use of it.


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

PR tree-optimization/91457 - inconsistent warning for writing past the end of an array member

gcc/testsuite/ChangeLog:

	PR tree-optimization/91457
	* c-c++-common/Wstringop-overflow-2.c: New test.
	* g++.dg/warn/Warray-bounds-8.C: New test.
	* g++.dg/warn/Wstringop-overflow-3.C: New test.
	* gcc.dg/Wstringop-overflow-15.c: New test.

gcc/ChangeLog:

	PR tree-optimization/91457
	* builtins.c (c_strlen): Rename argument and introduce new local.
	Set no-warning bit on original argument.
	(componen_size): New function.
	(compute_objsize): Add argument. Handle ARRAY_REF and COMPONENT_REF.
	* builtins.h (compute_objsize): Add argument.
	* expr.c (string_constant): Pass argument type to fold_ctor_reference.
	* gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST
	for missing initializers.
	* tree.c (build_string_literal): Handle optional argument.
	* tree.h (build_string_literal): Add defaulted argument.
	* tree-ssa-strlen.c (handle_store): Handle no-warning bit.
	* tree-vrp.c (vrp_prop::check_array_ref): Return warning result.
	(vrp_prop::check_mem_ref): Same.
	(vrp_prop::search_for_addr_array): Set no-warning bit.
	(check_array_bounds): Same.

gcc/c-family/ChangeLog:
	* c-common.c (braced_list_to_string): Add argument and overload.
	Handle flexible length arrays.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 9a766e4ad63..037e4eddf51 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -72,6 +72,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "file-prefix-map.h" /* remap_macro_filename()  */
 #include "gomp-constants.h"
 #include "omp-general.h"
+#include "tree-dfa.h"
 
 struct target_builtins default_target_builtins;
 #if SWITCHABLE_TARGET
@@ -620,7 +621,7 @@ unterminated_array (tree exp, tree *size /* = NULL */, bool *exact /* = NULL */)
    into the instruction stream and zero if it is going to be expanded.
    E.g. with i++ ? "foo" : "bar", if ONLY_VALUE is nonzero, constant 3
    is returned, otherwise NULL, since
-   len = c_strlen (src, 1); if (len) expand_expr (len, ...); would not
+   len = c_strlen (ARG, 1); if (len) expand_expr (len, ...); would not
    evaluate the side-effects.
 
    If ONLY_VALUE is two then we do not emit warnings about out-of-bound
@@ -628,7 +629,7 @@ unterminated_array (tree exp, tree *size /* = NULL */, bool *exact /* = NULL */)
    into the instruction stream.
 
    Additional information about the string accessed may be recorded
-   in DATA.  For example, if SRC references an unterminated string,
+   in DATA.  For example, if ARG references an unterminated string,
    then the declaration will be stored in the DECL field.   If the
    length of the unterminated string can be determined, it'll be
    stored in the LEN field.  Note this length could well be different
@@ -640,7 +641,7 @@ unterminated_array (tree exp, tree *size /* = NULL */, bool *exact /* = NULL */)
    The value returned is of type `ssizetype'.  */
 
 tree
-c_strlen (tree src, int only_value, c_strlen_data *data, unsigned eltsize)
+c_strlen (tree arg, int only_value, c_strlen_data *data, unsigned eltsize)
 {
   /* If we were not passed a DATA pointer, then get one to a local
      structure.  That avoids having to check DATA for NULL before
@@ -650,7 +651,8 @@ c_strlen (tree src, int only_value, c_strlen_data *data, unsigned eltsize)
     data = &local_strlen_data;
 
   gcc_checking_assert (eltsize == 1 || eltsize == 2 || eltsize == 4);
-  STRIP_NOPS (src);
+
+  tree src = STRIP_NOPS (arg);
   if (TREE_CODE (src) == COND_EXPR
       && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0))))
     {
@@ -762,11 +764,11 @@ c_strlen (tree src, int only_value, c_strlen_data *data, unsigned eltsize)
     {
       /* Suppress multiple warnings for propagated constant strings.  */
       if (only_value != 2
-	  && !TREE_NO_WARNING (src)
+	  && !TREE_NO_WARNING (arg)
 	  && warning_at (loc, OPT_Warray_bounds,
 			 "offset %qwi outside bounds of constant string",
 			 eltoff))
-	TREE_NO_WARNING (src) = 1;
+	TREE_NO_WARNING (arg) = 1;
       return NULL_TREE;
     }
 
@@ -3560,6 +3562,54 @@ check_access (tree exp, tree, tree, tree dstwrite,
   return true;
 }
 
+/* Determines the size of the member referenced by the COMPONENT_REF
+   REF, using its initializer expression if necessary in order to
+   determine the size of an initialized flexible array member.
+   Returns the size (which might be zero for an object with
+   an uninitialized flexible array member) or null if the size
+   cannot be determined.  */
+
+static tree
+component_size (tree ref)
+{
+  gcc_assert (TREE_CODE (ref) == COMPONENT_REF);
+
+  tree member = TREE_OPERAND (ref, 1);
+
+  /* If the member is not last or has a size greater than one, return
+     it.  Otherwise it's either a flexible array member or a zero-length
+     array member, or an array of length one treated as such.  */
+  tree size = DECL_SIZE_UNIT (member);
+  if (size
+      && (!array_at_struct_end_p (ref)
+	  || (!integer_zerop (size)
+	      && !integer_onep (size))))
+    return size;
+
+  /* If the reference is to a declared object and the member a true
+     flexible array, try to determine its size from its initializer.  */
+  poly_int64 off = 0;
+  tree base = get_addr_base_and_unit_offset (ref, &off);
+  if (!base || !VAR_P (base))
+    return NULL_TREE;
+
+  /* The size of any member of a declared object other than a flexible
+     array member is that obtained above.  */
+  if (size)
+    return size;
+
+  if (tree init = DECL_INITIAL (base))
+    if (TREE_CODE (init) == CONSTRUCTOR)
+      {
+	off <<= LOG2_BITS_PER_UNIT;
+	init = fold_ctor_reference (NULL_TREE, init, off, 0, base);
+	if (init)
+	  return TYPE_SIZE_UNIT (TREE_TYPE (init));
+      }
+
+  return DECL_EXTERNAL (base) ? NULL_TREE : integer_zero_node;
+}
+
 /* Helper to compute the size of the object referenced by the DEST
    expression which must have pointer type, using Object Size type
    OSTYPE (only the least significant 2 bits are used).  Return
@@ -3567,12 +3617,18 @@ check_access (tree exp, tree, tree, tree dstwrite,
    the size cannot be determined.  When the referenced object involves
    a non-constant offset in some range the returned value represents
    the largest size given the smallest non-negative offset in the
-   range.  The function is intended for diagnostics and should not
-   be used to influence code generation or optimization.  */
+   range.  If nonnull, set *PDECL to the decl of the referenced
+   subobject if it can be determined, or to null otherwise.
+   The function is intended for diagnostics and should not be used
+   to influence code generation or optimization.  */
 
 tree
-compute_objsize (tree dest, int ostype)
+compute_objsize (tree dest, int ostype, tree *pdecl /* = NULL */)
 {
+  tree dummy = NULL_TREE;
+  if (!pdecl)
+    pdecl = &dummy;
+
   unsigned HOST_WIDE_INT size;
 
   /* Only the two least significant bits are meaningful.  */
@@ -3599,7 +3655,7 @@ compute_objsize (tree dest, int ostype)
 	  tree off = gimple_assign_rhs2 (stmt);
 	  if (TREE_CODE (off) == INTEGER_CST)
 	    {
-	      if (tree size = compute_objsize (dest, ostype))
+	      if (tree size = compute_objsize (dest, ostype, pdecl))
 		{
 		  wide_int wioff = wi::to_wide (off);
 		  wide_int wisiz = wi::to_wide (size);
@@ -3624,7 +3680,7 @@ compute_objsize (tree dest, int ostype)
 
 	      if (rng == VR_RANGE)
 		{
-		  if (tree size = compute_objsize (dest, ostype))
+		  if (tree size = compute_objsize (dest, ostype, pdecl))
 		    {
 		      wide_int wisiz = wi::to_wide (size);
 
@@ -3652,12 +3708,31 @@ compute_objsize (tree dest, int ostype)
   if (!ostype)
     return NULL_TREE;
 
-  if (TREE_CODE (dest) == MEM_REF)
+  if (TREE_CODE (dest) == ARRAY_REF
+      || TREE_CODE (dest) == MEM_REF)
     {
       tree ref = TREE_OPERAND (dest, 0);
       tree off = TREE_OPERAND (dest, 1);
-      if (tree size = compute_objsize (ref, ostype))
+      if (tree size = compute_objsize (ref, ostype, pdecl))
 	{
+	  /* If the declaration of the destination object is known
+	     to have zero size, return zero.  */
+	  if (integer_zerop (size))
+	    return integer_zero_node;
+
+	  if (TREE_CODE (off) != INTEGER_CST
+	      || TREE_CODE (size) != INTEGER_CST)
+	    return NULL_TREE;
+
+	  if (TREE_CODE (dest) == ARRAY_REF)
+	    {
+	      tree eltype = TREE_TYPE (dest);
+	      if (tree tpsize = TYPE_SIZE_UNIT (eltype))
+		off = fold_build2 (MULT_EXPR, size_type_node, off, tpsize);
+	      else
+		return NULL_TREE;
+	    }
+
 	  if (tree_int_cst_lt (off, size))
 	    return fold_build2 (MINUS_EXPR, size_type_node, size, off);
 	  return integer_zero_node;
@@ -3666,9 +3741,22 @@ compute_objsize (tree dest, int ostype)
       return NULL_TREE;
     }
 
+  if (TREE_CODE (dest) == COMPONENT_REF)
+    {
+      *pdecl = TREE_OPERAND (dest, 1);
+      return component_size (dest);
+    }
+
   if (TREE_CODE (dest) != ADDR_EXPR)
     return NULL_TREE;
 
+  tree ref = TREE_OPERAND (dest, 0);
+  if (DECL_P (ref))
+    {
+      *pdecl = ref;
+      return DECL_SIZE_UNIT (ref);
+    }
+
   tree type = TREE_TYPE (dest);
   if (TREE_CODE (type) == POINTER_TYPE)
     type = TREE_TYPE (type);
@@ -3676,14 +3764,10 @@ compute_objsize (tree dest, int ostype)
   type = TYPE_MAIN_VARIANT (type);
 
   if (TREE_CODE (type) == ARRAY_TYPE
-      && !array_at_struct_end_p (TREE_OPERAND (dest, 0)))
-    {
-      /* Return the constant size unless it's zero (that's a zero-length
-	 array likely at the end of a struct).  */
-      tree size = TYPE_SIZE_UNIT (type);
-      if (size && TREE_CODE (size) == INTEGER_CST
-	  && !integer_zerop (size))
-	return size;
+      && !array_at_struct_end_p (ref))
+    {
+      if (tree size = TYPE_SIZE_UNIT (type))
+	return TREE_CODE (size) == INTEGER_CST ? size : NULL_TREE;
     }
 
   return NULL_TREE;
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 66c9295ff4a..1ad82e86963 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -134,7 +134,7 @@ extern tree fold_call_stmt (gcall *, bool);
 extern void set_builtin_user_assembler_name (tree decl, const char *asmspec);
 extern bool is_simple_builtin (tree);
 extern bool is_inexpensive_builtin (tree);
-extern tree compute_objsize (tree, int);
+extern tree compute_objsize (tree, int, tree * = NULL);
 
 extern bool readonly_data_expr (tree exp);
 extern bool init_target_chars (void);
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 2810867235d..ef942ffce57 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8747,9 +8747,12 @@ maybe_add_include_fixit (rich_location *richloc, const char *header,
    the converted string on success or the original ctor on failure.  */
 
 static tree
-braced_list_to_string (tree type, tree ctor)
+braced_list_to_string (tree type, tree ctor, bool member)
 {
-  if (!tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
+  /* Ignore non-members with unknown size like arrays with unspecified
+     bound.  */
+  tree typesize = TYPE_SIZE_UNIT (type);
+  if (!member && !tree_fits_uhwi_p (typesize))
     return ctor;
 
   /* If the array has an explicit bound, use it to constrain the size
@@ -8757,10 +8760,17 @@ braced_list_to_string (tree type, tree ctor)
      as long as implied by the index of the last zero specified via
      a designator, as in:
        const char a[] = { [7] = 0 };  */
-  unsigned HOST_WIDE_INT maxelts = tree_to_uhwi (TYPE_SIZE_UNIT (type));
-  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
+  unsigned HOST_WIDE_INT maxelts;
+  if (typesize)
+    {
+      maxelts = tree_to_uhwi (typesize);
+      maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
+    }
+  else
+    maxelts = HOST_WIDE_INT_M1U;
 
-  /* Avoid converting initializers for zero-length arrays.  */
+  /* Avoid converting initializers for zero-length arrays (but do
+     create them for flexible array members).  */
   if (!maxelts)
     return ctor;
 
@@ -8818,7 +8828,7 @@ braced_list_to_string (tree type, tree ctor)
     }
 
   /* Append a nul string termination.  */
-  if (str.length () < maxelts)
+  if (maxelts != HOST_WIDE_INT_M1U && str.length () < maxelts)
     str.safe_push (0);
 
   /* Build a STRING_CST with the same type as the array.  */
@@ -8827,14 +8837,12 @@ braced_list_to_string (tree type, tree ctor)
   return res;
 }
 
-/* Attempt to convert a CTOR containing braced array initializer lists
-   for array TYPE into one containing STRING_CSTs, for convenience and
-   efficiency.  Recurse for arrays of arrays and member initializers.
-   Return the converted CTOR or STRING_CST on success or the original
-   CTOR otherwise.  */
+/* Implementation of the two-argument braced_lists_to_string withe
+   the same arguments plus MEMBER which is set for struct members
+   to allow initializers for flexible member arrays.  */
 
-tree
-braced_lists_to_strings (tree type, tree ctor)
+static tree
+braced_lists_to_strings (tree type, tree ctor, bool member)
 {
   if (TREE_CODE (ctor) != CONSTRUCTOR)
     return ctor;
@@ -8858,7 +8866,7 @@ braced_lists_to_strings (tree type, tree ctor)
 
   if ((TREE_CODE (ttp) == ARRAY_TYPE || TREE_CODE (ttp) == INTEGER_TYPE)
       && TYPE_STRING_FLAG (ttp))
-    return braced_list_to_string (type, ctor);
+    return braced_list_to_string (type, ctor, member);
 
   code = TREE_CODE (ttp);
   if (code == ARRAY_TYPE || code == RECORD_TYPE)
@@ -8868,7 +8876,7 @@ braced_lists_to_strings (tree type, tree ctor)
       unsigned HOST_WIDE_INT idx;
       FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (ctor), idx, val)
 	{
-	  val = braced_lists_to_strings (ttp, val);
+	  val = braced_lists_to_strings (ttp, val, code == RECORD_TYPE);
 	  CONSTRUCTOR_ELT (ctor, idx)->value = val;
 	}
     }
@@ -8876,4 +8884,16 @@ braced_lists_to_strings (tree type, tree ctor)
   return ctor;
 }
 
+/* Attempt to convert a CTOR containing braced array initializer lists
+   for array TYPE into one containing STRING_CSTs, for convenience and
+   efficiency.  Recurse for arrays of arrays and member initializers.
+   Return the converted CTOR or STRING_CST on success or the original
+   CTOR otherwise.  */
+
+tree
+braced_lists_to_strings (tree type, tree ctor)
+{
+  return braced_lists_to_strings (type, ctor, false);
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/expr.c b/gcc/expr.c
index 20e3f9ce337..26c0aec720c 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -11562,7 +11562,7 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl)
 
       base_off = wioff.to_uhwi ();
       unsigned HOST_WIDE_INT fieldoff = 0;
-      init = fold_ctor_reference (NULL_TREE, init, base_off, 0, array,
+      init = fold_ctor_reference (TREE_TYPE (arg), init, base_off, 0, array,
 				  &fieldoff);
       HOST_WIDE_INT cstoff;
       if (!base_off.is_constant (&cstoff))
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index d576b0842f9..bdb1f62ac72 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -6969,12 +6969,33 @@ fold_nonarray_ctor_reference (tree type, tree ctor,
 				      from_decl, suboff);
 	}
     }
-  /* Memory not explicitly mentioned in constructor is 0.  */
-  return type ? build_zero_cst (type) : NULL_TREE;
+
+  if (!type)
+    return NULL_TREE;
+
+  /* Memory not explicitly mentioned in constructor is all zeros.  */
+  if (TREE_CODE (type) == ARRAY_TYPE)
+    {
+      /* Convert a char array to an empty STRING_CST having an array
+	 of the expected type.  */
+      tree eltype = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+      if (eltype == char_type_node)
+	{
+	  *suboff = offset;
+	  tree typesize = TYPE_SIZE_UNIT (type);
+	  unsigned HOST_WIDE_INT size
+	    = typesize ? tree_to_uhwi (typesize) : 0;
+	  tree str = build_string_literal (0, "", eltype, size);
+	  str = TREE_OPERAND (str, 0);
+	  return TREE_OPERAND (str, 0);
+	}
+    }
+
+  return build_zero_cst (type);
 }
 
 /* CTOR is value initializing memory.  Fold a reference of TYPE and
-   bit size POLY_SIZE to the memory at bit POLY_OFFSET.  When SIZE
+   bit size POLY_SIZE to the memory at bit POLY_OFFSET.  When POLY_SIZE
    is zero, attempt to fold a reference to the entire subobject
    which OFFSET refers to.  This is used when folding accesses to
    string members of aggregates.  When non-null, set *SUBOFF to
diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-7.c b/gcc/testsuite/c-c++-common/Warray-bounds-7.c
new file mode 100644
index 00000000000..668e8096200
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-bounds-7.c
@@ -0,0 +1,107 @@
+/* PR middle-end/91490 - bogus argument missing terminating nul warning
+   on strlen of a flexible array member
+   { dg-do compile }
+   { dg-options "-Wall -ftrack-macro-expansion=0" } */
+
+#define INT_MAX       __INT_MAX__
+#define PTRDIFF_MAX   __PTRDIFF_MAX__
+#define SIZE_MAX      __SIZE_MAX__
+
+struct A0 { char n, a[0]; };
+struct A1 { char n, a[1]; };
+struct Ax { char n, a[]; };
+
+const struct A0 a0 = { };
+const struct A0 a0_0 = { 0 };
+const struct A0 a0_0_ = { 0, { } };
+
+const struct A0 a1 = { };
+const struct A0 a1_0 = { 0 };
+const struct A0 a1_0_ = { 0, { } };
+
+const struct Ax ax= { };
+const struct Ax ax_0 = { 0 };
+const struct Ax ax_0_ = { 0, { } };
+
+void sink (unsigned);
+
+#define T(x)   sink (__builtin_strlen (x))
+
+void test_zero_length_array (void)
+{
+  T (a0.a);                   // { dg-warning "\\\[-Warray-bounds" }
+  T (a0.a - 1);               // { dg-warning "\\\[-Warray-bounds" }
+  T (a0.a + 1);               // { dg-warning "\\\[-Warray-bounds" }
+  T (a0.a + 9);               // { dg-warning "\\\[-Warray-bounds" }
+  T (a0.a + INT_MAX);         // { dg-warning "\\\[-Warray-bounds" }
+  T (a0.a + PTRDIFF_MAX);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a0.a + SIZE_MAX);        // { dg-warning "\\\[-Warray-bounds" }
+
+  T (a0_0.a);                 // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0.a - 1);             // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0.a + 1);             // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0.a + 9);             // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0.a + INT_MAX);       // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0.a + PTRDIFF_MAX);   // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0.a + SIZE_MAX);      // { dg-warning "\\\[-Warray-bounds" }
+
+  T (a0_0_.a);                // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0_.a - 1);            // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0_.a + 1);            // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0_.a + 9);            // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0_.a + INT_MAX);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0_.a + PTRDIFF_MAX);  // { dg-warning "\\\[-Warray-bounds" }
+  T (a0_0_.a + SIZE_MAX);     // { dg-warning "\\\[-Warray-bounds" }
+}
+
+void test_one_element_array (void)
+{
+  T (a1.a - 1);               // { dg-warning "\\\[-Warray-bounds" }
+  T (a1.a + 1);               // { dg-warning "\\\[-Warray-bounds" }
+  T (a1.a + 9);               // { dg-warning "\\\[-Warray-bounds" }
+  T (a1.a + INT_MAX);         // { dg-warning "\\\[-Warray-bounds" }
+  T (a1.a + PTRDIFF_MAX);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a1.a + SIZE_MAX);        // { dg-warning "\\\[-Warray-bounds" }
+
+  T (a1_0.a - 1);             // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0.a + 1);             // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0.a + 9);             // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0.a + INT_MAX);       // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0.a + PTRDIFF_MAX);   // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0.a + SIZE_MAX);      // { dg-warning "\\\[-Warray-bounds" }
+
+  T (a1_0_.a - 1);            // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0_.a + 1);            // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0_.a + 9);            // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0_.a + INT_MAX);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0_.a + PTRDIFF_MAX);  // { dg-warning "\\\[-Warray-bounds" }
+  T (a1_0_.a + SIZE_MAX);     // { dg-warning "\\\[-Warray-bounds" }
+}
+
+void test_flexible_array_member (void)
+{
+  T (ax.a);                   // { dg-warning "\\\[-Warray-bounds" }
+  T (ax.a - 1);               // { dg-warning "\\\[-Warray-bounds" }
+  T (ax.a + 1);               // { dg-warning "\\\[-Warray-bounds" }
+  T (ax.a + 9);               // { dg-warning "\\\[-Warray-bounds" }
+  T (ax.a + INT_MAX);         // { dg-warning "\\\[-Warray-bounds" }
+  T (ax.a + PTRDIFF_MAX);     // { dg-warning "\\\[-Warray-bounds" }
+  T (ax.a + SIZE_MAX);        // { dg-warning "\\\[-Warray-bounds" }
+
+  T (ax_0.a);                 // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0.a - 1);             // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0.a + 1);             // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0.a + 9);             // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0.a + INT_MAX);       // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0.a + PTRDIFF_MAX);   // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0.a + SIZE_MAX);      // { dg-warning "\\\[-Warray-bounds" }
+
+  T (ax_0_.a);                // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0_.a - 1);            // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0_.a + 1);            // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0_.a + 9);            // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0_.a + INT_MAX);      // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0_.a + PTRDIFF_MAX);  // { dg-warning "\\\[-Warray-bounds" }
+  T (ax_0_.a + SIZE_MAX);     // { dg-warning "\\\[-Warray-bounds" }
+}
+
diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
new file mode 100644
index 00000000000..9ed1109bca6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
@@ -0,0 +1,348 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-array-bounds" } */
+
+void sink (void*);
+
+// Exercise flexible array members.
+
+struct Ax
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+struct Ax ax_;
+
+void gax_ (void)
+{
+  ax_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+struct Ax ax0 = { 0 };
+
+void gax0 (void)
+{
+  ax0.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+struct Ax ax0_ = { 0, { } };
+
+void gax0_ (void)
+{
+  ax0_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for out-of-bounds accesses to a definition with
+// an initializer.
+struct Ax ax1 = { 1, { 0 } };
+
+void gax1 (void)
+{
+  ax1.a[0] = 0;
+  ax1.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax1.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+struct Ax ax2 = { 2, { 1, 0 } };
+
+void gax2 (void)
+{
+  ax2.a[0] = 0;
+  ax2.a[1] = 1;
+  ax2.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void gaxp (struct Ax *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify no warning for an extern struct object whose array may be
+// initialized to any number of elements.
+extern struct Ax axx;
+
+void gaxx (void)
+{
+  axx.a[0] = 0;
+  axx.a[3] = 0;
+  axx.a[9] = 0;
+}
+
+// Exercise zero-length array members.
+
+struct A0
+{
+  char n;
+  char a[0];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+struct A0 a0_;
+
+void ga0_ (void)
+{
+  a0_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+struct A0 a00 = { 0 };
+
+void ga00 (void)
+{
+  a00.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+struct A0 a00_ = { 0, { } };
+
+void ga00_ (void)
+{
+  a00_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// The following are rejected with
+//   error: too many initializers for 'char [0]'
+// A0 a01 = { 1, { 0 } };
+// A0 a02 = { 2, { 1, 0 } };
+
+
+// Verify no warning for an unknown struct object.
+void ga0p (struct A0 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object which (unlike a true
+// flexible array member) may not be initialized.
+extern struct A0 a0x;
+
+void ga0x (void)
+{
+  a0x.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Exercise trailing one-element array members.
+
+struct A1
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+struct A1 a1_;
+
+void ga1_ (void)
+{
+  a1_.a[0] = 0;
+  a1_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a;
+  a.a[0] = 0;
+  a.a[1] = 0;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 0;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+struct A1 a1__ = { 0 };
+
+void ga1__ (void)
+{
+  a1__.a[0] = 0;
+  a1__.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1__.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 1 };
+  a.a[0] = 0;
+  a.a[1] = 0;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 0;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+struct A1 a1_0 = { 0, { } };
+
+void ga1_0_ (void)
+{
+  a1_0.a[0] = 0;
+  a1_0.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_0.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 1, { } };
+  a.a[0] = 0;
+  a.a[1] = 0;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 0;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+struct A1 a1_1 = { 0, { 1 } };
+
+void ga1_1 (void)
+{
+  a1_1.a[0] = 0;
+  a1_1.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_1.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 0, { 1 } };
+  a.a[0] = 0;
+  a.a[1] = 0;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 0;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1p (struct A1 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object.  Similar to the zero-length
+// array case, a one-element trailing array can be initialized to at most
+// a single element.
+extern struct A1 a1x;
+
+void ga1x (void)
+{
+  a1x.a[0] = 0;
+  a1x.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1x.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Exercise interior one-element array members (verify they're not
+// treated as trailing.
+
+struct A1i
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+  char x;
+};
+
+// Verify warning for a definition with no initializer.
+struct A1i a1i_;
+
+void ga1i_ (void)
+{
+  a1i_.a[0] = 0;
+  a1i_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1i a;
+  a.a[0] = 1;
+  a.a[1] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 3;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+struct A1i a1i__ = { 0 };
+
+void ga1i__ (void)
+{
+  a1i__.a[0] = 0;
+  a1i__.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i__.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1i a = { 0 };
+  a.a[0] = 0;
+  a.a[1] = 0;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 0;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+struct A1 a1i_0 = { 0, { } };
+
+void ga1i_0_ (void)
+{
+  a1i_0.a[0] = 0;
+  a1i_0.a[1] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_0.a[2] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 0, { } };
+  a.a[0] = 1;
+  a.a[1] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 3;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+struct A1 a1i_1 = { 0, { 1 } };
+
+void ga1i_1 (void)
+{
+  a1i_1.a[0] = 0;
+  a1i_1.a[1] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_1.a[2] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 0, { 1 } };
+  a.a[0] = 1;
+  a.a[1] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 3;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1ip (struct A1i *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;                  // { dg-warning "\\\[-Wstringop-overflow" }
+  p->a[9] = 0;                  // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an extern struct object.
+extern struct A1i a1ix;
+
+void ga1ix (void)
+{
+  a1ix.a[0] = 0;
+  a1ix.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1ix.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-8.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-8.C
new file mode 100644
index 00000000000..850414ede4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-8.C
@@ -0,0 +1,388 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   See Wstringop-overflow-3.C for the same test that exercises the other
+   warning.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-stringop-overflow" } */
+
+void sink (void*);
+
+// Exercise flexible array members.
+
+struct Ax
+{
+  char n;
+  char a[];                     // { dg-message "while referencing .Ax::a." "pr91463" { xfail *-*-* } }
+};
+
+// Verify warning for a definition with no initializer.
+Ax ax_;
+
+void gax_ ()
+{
+  ax_.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax_.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax_.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+Ax ax0 = { 0 };
+
+void gax0 ()
+{
+  ax0.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+Ax ax0_ = { 0, { } };
+
+void gax0_ ()
+{
+  ax0_.a[0] = 0;                // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0_.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0_.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+// Verify warning for out-of-bounds accesses to a definition with
+// an initializer.
+Ax ax1 = { 1, { 0 } };
+
+void gax1 ()
+{
+  ax1.a[0] = 0;
+  ax1.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax1.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+Ax ax2 = { 2, { 1, 0 } };
+
+void gax2 ()
+{
+  ax2.a[0] = 0;
+  ax2.a[1] = 0;
+  ax2.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+
+// Verify no warning for an unknown struct object.
+void gaxp (Ax *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify no warning for an extern struct object whose array may be
+// initialized to any number of elements.
+extern Ax axx;
+
+void gaxx ()
+{
+  axx.a[0] = 0;
+  axx.a[3] = 0;
+  axx.a[9] = 0;
+}
+
+// Exercise zero-length array members.
+
+struct A0
+{
+  char n;
+  char a[0];                    // { dg-message "while referencing .A0::a." }
+};
+
+// Verify warning for a definition with no initializer.
+A0 a0_;
+
+void ga0_ ()
+{
+  a0_.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0_.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0_.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+A0 a00 = { 0 };
+
+void ga00 ()
+{
+  a00.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a00.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a00.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+A0 a00_ = { 0, { } };
+
+void ga00_ ()
+{
+  a00_.a[0] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a00_.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a00_.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// The following are rejected with
+//   error: too many initializers for 'char [0]'
+// A0 a01 = { 1, { 0 } };
+// A0 a02 = { 2, { 1, 0 } };
+
+
+// Verify no warning for an unknown struct object.
+void ga0p (A0 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object which (unlike a true
+// flexible array member) may not be initialized.
+extern A0 a0x;
+
+void ga0x ()
+{
+  a0x.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0x.a[3] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0x.a[9] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Exercise trailing one-element array members.
+
+struct A1
+{
+  char n;
+  char a[1];                    // { dg-message "while referencing .A1::a." }
+};
+
+// Verify warning for a definition with no initializer.
+A1 a1_;
+
+void ga1_ ()
+{
+  a1_.a[0] = 0;
+  a1_.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1_.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1 a1__ = { 0 };
+
+void ga1__ ()
+{
+  a1__.a[0] = 0;
+  a1__.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1__.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1_0 = { 0, { } };
+
+void ga1_0_ ()
+{
+  a1_0.a[0] = 0;
+  a1_0.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1_0.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1_1 = { 0, { 1 } };
+
+void ga1_1 ()
+{
+  a1_1.a[0] = 0;
+  a1_1.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1_1.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1p (A1 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object.  Similar to the zero-length
+// array case, a one-element trailing array can be initialized to at most
+// a single element.
+extern A1 a1x;
+
+void ga1x ()
+{
+  a1x.a[0] = 0;
+  a1x.a[3] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1x.a[9] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Exercise interior one-element array members (verify they're not
+// treated as trailing.
+
+struct A1i
+{
+  char n;
+  char a[1];                    // { dg-message "while referencing .A1i::a." }
+  char x;
+};
+
+// Verify warning for a definition with no initializer.
+A1i a1i_;
+
+void ga1i_ ()
+{
+  a1i_.a[0] = 0;
+  a1i_.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1i_.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1i a1i__ = { 0 };
+
+void ga1i__ ()
+{
+  a1i__.a[0] = 0;
+  a1i__.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1i__.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1i_0 = { 0, { } };
+
+void ga1i_0_ ()
+{
+  a1i_0.a[0] = 0;
+  a1i_0.a[1] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+  a1i_0.a[2] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1i_1 = { 0, { 1 } };
+
+void ga1i_1 ()
+{
+  a1i_1.a[0] = 0;
+  a1i_1.a[1] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+  a1i_1.a[2] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1ip (A1i *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;                  // { dg-warning "\\\[-Warray-bounds" }
+  p->a[9] = 0;                  // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify no warning for an extern struct object.
+extern A1i a1ix;
+
+void ga1ix ()
+{
+  a1ix.a[0] = 0;
+  a1ix.a[3] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1ix.a[9] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify non-POD classes with flexible array members.
+
+struct Bx
+{
+  char n;
+  char a[];                     // { dg-message "while referencing .Bx::a." "pr91463" { xfail *-*-* } }
+
+  // Verify the warning for a constant.
+  Bx () { a[0] = 0; }           // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+
+  // And also for a non-constant.  Regardless of the subscript, the array
+  // of the object in function gxi() below has a zero size.
+  Bx (int i) { a[i] = 0; }      // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+};
+
+void gbx (void)
+{
+  struct Bx bx;
+  sink (&bx);
+}
+
+void gbxi (int i)
+{
+  struct Bx bxi (i);
+  sink (&bxi);
+}
+
+struct B0
+{
+  char n;
+  char a[0];                    // { dg-message "while referencing .B0::a." }
+
+  B0 () { a[0] = 0; }           // { dg-warning "\\\[-Warray-bounds" }
+};
+
+
+void gb0 (void)
+{
+  struct B0 b0;
+  sink (&b0);
+}
+
+
+struct B1
+{
+  char n;
+  char a[1];                    // { dg-message "while referencing .B1::a." }
+
+  B1 () { a[1] = 0; }           // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void gb1 (void)
+{
+  struct B1 b1;
+  sink (&b1);
+}
+
+
+struct B123
+{
+  char a[123];                  // { dg-message "while referencing .B123::a." }
+
+  B123 () { a[123] = 0; }       // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void gb123 (void)
+{
+  struct B123 b123;
+  sink (&b123);
+}
+
+
+struct B234
+{
+  char a[234];                  // { dg-message "while referencing .B234::a." }
+
+  B234 (int i) { a[i] = 0; }    // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void g234 (void)
+{
+  struct B234 b234 (234);
+  sink (&b234);
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C
new file mode 100644
index 00000000000..99ce427c1b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C
@@ -0,0 +1,386 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-array-bounds" } */
+
+void sink (void*);
+
+// Exercise flexible array members.
+
+struct Ax
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+Ax ax_;
+
+void gax_ ()
+{
+  ax_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+Ax ax0 = { 0 };
+
+void gax0 ()
+{
+  ax0.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+Ax ax0_ = { 0, { } };
+
+void gax0_ ()
+{
+  ax0_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for out-of-bounds accesses to a definition with
+// an initializer.
+Ax ax1 = { 1, { 0 } };
+
+void gax1 ()
+{
+  ax1.a[0] = 0;
+  ax1.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax1.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+Ax ax2 = { 2, { 1, 0 } };
+
+void gax2 ()
+{
+  ax2.a[0] = 0;
+  ax2.a[1] = 0;
+  ax2.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void gaxp (Ax *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify no warning for an extern struct object whose array may be
+// initialized to any number of elements.
+extern Ax axx;
+
+void gaxx ()
+{
+  axx.a[0] = 0;
+  axx.a[3] = 0;
+  axx.a[9] = 0;
+}
+
+// Exercise zero-length array members.
+
+struct A0
+{
+  char n;
+  char a[0];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+A0 a0_;
+
+void ga0_ ()
+{
+  a0_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+A0 a00 = { 0 };
+
+void ga00 ()
+{
+  a00.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+A0 a00_ = { 0, { } };
+
+void ga00_ ()
+{
+  a00_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// The following are rejected with
+//   error: too many initializers for 'char [0]'
+// A0 a01 = { 1, { 0 } };
+// A0 a02 = { 2, { 1, 0 } };
+
+
+// Verify no warning for an unknown struct object.
+void ga0p (A0 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object which (unlike a true
+// flexible array member) may not be initialized.
+extern A0 a0x;
+
+void ga0x ()
+{
+  a0x.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Exercise trailing one-element array members.
+
+struct A1
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+A1 a1_;
+
+void ga1_ ()
+{
+  a1_.a[0] = 0;
+  a1_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1 a1__ = { 0 };
+
+void ga1__ ()
+{
+  a1__.a[0] = 0;
+  a1__.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1__.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1_0 = { 0, { } };
+
+void ga1_0_ ()
+{
+  a1_0.a[0] = 0;
+  a1_0.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_0.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1_1 = { 0, { 1 } };
+
+void ga1_1 ()
+{
+  a1_1.a[0] = 0;
+  a1_1.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_1.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1p (A1 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object.  Similar to the zero-length
+// array case, a one-element trailing array can be initialized to at most
+// a single element.
+extern A1 a1x;
+
+void ga1x ()
+{
+  a1x.a[0] = 0;
+  a1x.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1x.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Exercise interior one-element array members (verify they're not
+// treated as trailing.
+
+struct A1i
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+  char x;
+};
+
+// Verify warning for a definition with no initializer.
+A1i a1i_;
+
+void ga1i_ ()
+{
+  a1i_.a[0] = 0;
+  a1i_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1i a1i__ = { 0 };
+
+void ga1i__ ()
+{
+  a1i__.a[0] = 0;
+  a1i__.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i__.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1i_0 = { 0, { } };
+
+void ga1i_0_ ()
+{
+  a1i_0.a[0] = 0;
+  a1i_0.a[1] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_0.a[2] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1i_1 = { 0, { 1 } };
+
+void ga1i_1 ()
+{
+  a1i_1.a[0] = 0;
+  a1i_1.a[1] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_1.a[2] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1ip (A1i *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;                  // { dg-warning "\\\[-Wstringop-overflow" }
+  p->a[9] = 0;                  // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an extern struct object.
+extern A1i a1ix;
+
+void ga1ix ()
+{
+  a1ix.a[0] = 0;
+  a1ix.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1ix.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify non-POD classes with flexible array members.
+
+struct Bx
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }
+
+  // Verify the warning for a constant.
+  Bx () { a[0] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+
+  // And also for a non-constant.  Regardless of the subscript, the array
+  // of the object in function gxi() below has a zero size.
+  Bx (int i) { a[i] = 0; }      // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void gbx (void)
+{
+  struct Bx bx;
+  sink (&bx);
+}
+
+void gbxi (int i)
+{
+  struct Bx bxi (i);
+  sink (&bxi);
+}
+
+struct B0
+{
+  char n;
+  char a[0];                    // { dg-message "destination object declared here" }
+
+  B0 () { a[0] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+
+void gb0 (void)
+{
+  struct B0 b0;
+  sink (&b0);
+}
+
+
+struct B1
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+
+  B1 () { a[1] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void gb1 (void)
+{
+  struct B1 b1;
+  sink (&b1);
+}
+
+
+struct B123
+{
+  char a[123];                  // { dg-message "destination object declared here" }
+
+  B123 () { a[123] = 0; }       // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void gb123 (void)
+{
+  struct B123 b123;
+  sink (&b123);
+}
+
+
+struct B234
+{
+  char a[234];                  // { dg-message "destination object declared here" }
+
+  B234 (int i) { a[i] = 0; }    // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void g234 (void)
+{
+  struct B234 b234 (234);
+  sink (&b234);
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
new file mode 100644
index 00000000000..12f8f9d353b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
@@ -0,0 +1,62 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   Verify that the -Wstringop-overflow detection doesn't cause an ICE
+   for either kind of VLAs (member and non-member).
+   Diagnosing the accesses is the subject of pr82608.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-array-bounds" } */
+
+void sink (void*);
+
+void vla_unbounded (int n)
+{
+  char a[n];
+
+  a[0] = 0;
+  a[1] = 1;
+  a[n] = n;         // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&a);
+}
+
+void vla_bounded (int n)
+{
+  if (n > 32)
+    n = 32;
+
+  char a[n];
+
+  a[0] = 0;
+  a[1] = 1;
+  a[n] = n;         // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+  a[69] = n;        // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&a);
+}
+
+
+void member_vla_unbounded (int n)
+{
+  struct S { char i, a[n]; } s;
+
+  s.a[0] = 0;
+  s.a[1] = 1;
+  s.a[n] = n;       // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&s);
+}
+
+void member_vla_bounded (int n)
+{
+  if (n > 32)
+    n = 32;
+
+  struct S { char i, a[n]; } s;
+
+  s.a[0] = 0;
+  s.a[1] = 1;
+  s.a[n] = n;       // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+  s.a[69] = n;      // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&s);
+}
diff --git a/gcc/testsuite/gcc.dg/strlenopt-78.c b/gcc/testsuite/gcc.dg/strlenopt-78.c
new file mode 100644
index 00000000000..fcd64cd0d68
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strlenopt-78.c
@@ -0,0 +1,45 @@
+/* PR middle-end/91490 - bogus argument missing terminating nul warning
+   on strlen of a flexible array member
+   Verify that strlen calls with a flexible array member (and its common
+   forms) of declaraed objects are folded.
+   { dg-do compile }
+   { dg-options "-Wall -fdump-tree-gimple" } */
+
+#include "strlenopt.h"
+
+struct A1 { char n, a[1]; };
+struct A2 { char n, a[2]; };
+struct Ax { char n, a[]; };
+
+const struct A1 a1 = { 0 };
+const struct A1 a1__ = { 0, { } };
+const struct A1 a1_0 = { 0, { 0 } };
+
+const struct A2 a2 = { 1 };
+const struct A2 a2_1 = { 1, { 1 } };
+const struct A2 a2_1_0 = { 1, { 1, 0 } };
+
+const struct Ax ax = { 3, { 3, 2, 1, 0 } };
+
+int len1, len1__, len1_0, len2_1, len2, len2_1_0, lenx;
+
+void test_flexarray (void)
+{
+  len1 = strlen (a1.a);
+  len1__ = strlen (a1__.a);
+  len1_0 = strlen (a1_0.a);
+
+  len2 = strlen (a2.a);
+  len2_1 = strlen (a2_1.a);
+  len2_1_0 = strlen (a2_1_0.a);
+
+  lenx = strlen (ax.a);
+}
+
+/* { dg-final { scan-tree-dump-not "strlen" "gimple" } }
+   { dg-final { scan-tree-dump "len1 = 0;" "gimple" } }
+   { dg-final { scan-tree-dump "len1__ = 0;" "gimple" } }
+   { dg-final { scan-tree-dump "len2 = 0;" "gimple" } }
+   { dg-final { scan-tree-dump "len2_1 = 1;" "gimple" } }
+   { dg-final { scan-tree-dump "len2_1_0 = 1;" "gimple" } }
+   { dg-final { scan-tree-dump "lenx = 3;" "gimple" } } */
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index ef2b6ae65f2..840b242c0ce 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -3679,16 +3679,30 @@ handle_store (gimple_stmt_iterator *gsi)
       rhs_minlen = lenrange[0];
       storing_nonzero_p = lenrange[1] > 0;
 
-      if (tree dstsize = compute_objsize (lhs, 1))
-	if (compare_tree_int (dstsize, lenrange[2]) < 0)
-	  {
-	    location_t loc = gimple_nonartificial_location (stmt);
-	    warning_n (loc, OPT_Wstringop_overflow_,
-		       lenrange[2],
-		       "%Gwriting %u byte into a region of size %E",
-		       "%Gwriting %u bytes into a region of size %E",
-		       stmt, lenrange[2], dstsize);
-	  }
+      /* Avoid issuing multiple warnings for the same LHS or statement.
+	 For example, -Warray-bounds may have already been issued for
+	 an out-of-bounds subscript.  */
+      if (!TREE_NO_WARNING (lhs) && !gimple_no_warning_p (stmt))
+	{
+	  /* Set to the declaration referenced by LHS (if known).  */
+	  tree decl = NULL_TREE;
+	  if (tree dstsize = compute_objsize (lhs, 1, &decl))
+	    if (compare_tree_int (dstsize, lenrange[2]) < 0)
+	      {
+		location_t loc = gimple_nonartificial_location (stmt);
+		if (warning_n (loc, OPT_Wstringop_overflow_,
+			       lenrange[2],
+			       "%Gwriting %u byte into a region of size %E",
+			       "%Gwriting %u bytes into a region of size %E",
+			       stmt, lenrange[2], dstsize))
+		  {
+		    if (decl)
+		      inform (DECL_SOURCE_LOCATION (decl),
+			      "destination object declared here");
+		    gimple_set_no_warning (stmt, true);
+		  }
+	      }
+	}
     }
   else
     {
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 8067f8560cd..3e4a09a2e76 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -4388,8 +4388,8 @@ class vrp_prop : public ssa_propagation_engine
   void vrp_initialize (void);
   void vrp_finalize (bool);
   void check_all_array_refs (void);
-  void check_array_ref (location_t, tree, bool);
-  void check_mem_ref (location_t, tree, bool);
+  bool check_array_ref (location_t, tree, bool);
+  bool check_mem_ref (location_t, tree, bool);
   void search_for_addr_array (tree, location_t);
 
   class vr_values vr_values;
@@ -4415,9 +4415,10 @@ class vrp_prop : public ssa_propagation_engine
    array subscript is a constant, check if it is outside valid
    range. If the array subscript is a RANGE, warn if it is
    non-overlapping with valid range.
-   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
+   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.
+   Returns true if a warning has been issued.  */
 
-void
+bool
 vrp_prop::check_array_ref (location_t location, tree ref,
 			   bool ignore_off_by_one)
 {
@@ -4426,7 +4427,7 @@ vrp_prop::check_array_ref (location_t location, tree ref,
   tree low_bound, up_bound, up_bound_p1;
 
   if (TREE_NO_WARNING (ref))
-    return;
+    return false;
 
   low_sub = up_sub = TREE_OPERAND (ref, 1);
   up_bound = array_ref_up_bound (ref);
@@ -4541,12 +4542,16 @@ vrp_prop::check_array_ref (location_t location, tree ref,
   if (warned)
     {
       ref = TREE_OPERAND (ref, 0);
+      if (TREE_CODE (ref) == COMPONENT_REF)
+	ref = TREE_OPERAND (ref, 1);
 
       if (DECL_P (ref))
 	inform (DECL_SOURCE_LOCATION (ref), "while referencing %qD", ref);
 
       TREE_NO_WARNING (ref) = 1;
     }
+
+  return warned;
 }
 
 /* Checks one MEM_REF in REF, located at LOCATION, for out-of-bounds
@@ -4556,14 +4561,15 @@ vrp_prop::check_array_ref (location_t location, tree ref,
    with valid range.
    IGNORE_OFF_BY_ONE is true if the MEM_REF is inside an ADDR_EXPR
    (used to allow one-past-the-end indices for code that takes
-   the address of the just-past-the-end element of an array).  */
+   the address of the just-past-the-end element of an array).
+   Returns true if a warning has been issued.  */
 
-void
+bool
 vrp_prop::check_mem_ref (location_t location, tree ref,
 			 bool ignore_off_by_one)
 {
   if (TREE_NO_WARNING (ref))
-    return;
+    return false;
 
   tree arg = TREE_OPERAND (ref, 0);
   /* The constant and variable offset of the reference.  */
@@ -4615,7 +4621,7 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
 	  continue;
 	}
       else
-	return;
+	return false;
 
       /* VAROFF should always be a SSA_NAME here (and not even
 	 INTEGER_CST) but there's no point in taking chances.  */
@@ -4677,10 +4683,10 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
       arg = TREE_OPERAND (arg, 0);
       if (TREE_CODE (arg) != STRING_CST
 	  && TREE_CODE (arg) != VAR_DECL)
-	return;
+	return false;
     }
   else
-    return;
+    return false;
 
   /* The type of the object being referred to.  It can be an array,
      string literal, or a non-array type when the MEM_REF represents
@@ -4695,7 +4701,7 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
       || !COMPLETE_TYPE_P (reftype)
       || TREE_CODE (TYPE_SIZE_UNIT (reftype)) != INTEGER_CST
       || RECORD_OR_UNION_TYPE_P (reftype))
-    return;
+    return false;
 
   offset_int eltsize;
   if (TREE_CODE (reftype) == ARRAY_TYPE)
@@ -4797,11 +4803,11 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
 
       if (warned)
 	TREE_NO_WARNING (ref) = 1;
-      return;
+      return warned;
     }
 
   if (warn_array_bounds < 2)
-    return;
+    return false;
 
   /* At level 2 check also intermediate offsets.  */
   int i = 0;
@@ -4812,8 +4818,13 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
       if (warning_at (location, OPT_Warray_bounds,
 		      "intermediate array offset %wi is outside array bounds "
 		      "of %qT", tmpidx, reftype))
-	TREE_NO_WARNING (ref) = 1;
+	{
+	  TREE_NO_WARNING (ref) = 1;
+	  return true;
+	}
     }
+
+  return false;
 }
 
 /* Searches if the expr T, located at LOCATION computes
@@ -4825,10 +4836,14 @@ vrp_prop::search_for_addr_array (tree t, location_t location)
   /* Check each ARRAY_REF and MEM_REF in the reference chain. */
   do
     {
+      bool warned = false;
       if (TREE_CODE (t) == ARRAY_REF)
-	check_array_ref (location, t, true /*ignore_off_by_one*/);
+	warned = check_array_ref (location, t, true /*ignore_off_by_one*/);
       else if (TREE_CODE (t) == MEM_REF)
-	check_mem_ref (location, t, true /*ignore_off_by_one*/);
+	warned = check_mem_ref (location, t, true /*ignore_off_by_one*/);
+
+      if (warned)
+	TREE_NO_WARNING (t) = true;
 
       t = TREE_OPERAND (t, 0);
     }
@@ -4920,16 +4935,20 @@ check_array_bounds (tree *tp, int *walk_subtree, void *data)
 
   *walk_subtree = TRUE;
 
+  bool warned = false;
   vrp_prop *vrp_prop = (class vrp_prop *)wi->info;
   if (TREE_CODE (t) == ARRAY_REF)
-    vrp_prop->check_array_ref (location, t, false /*ignore_off_by_one*/);
+    warned = vrp_prop->check_array_ref (location, t, false/*ignore_off_by_one*/);
   else if (TREE_CODE (t) == MEM_REF)
-    vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/);
+    warned = vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/);
   else if (TREE_CODE (t) == ADDR_EXPR)
     {
       vrp_prop->search_for_addr_array (t, location);
       *walk_subtree = FALSE;
     }
+  /* Propagate the no-warning bit to the outer expression.  */
+  if (warned)
+    TREE_NO_WARNING (t) = true;
 
   return NULL_TREE;
 }
diff --git a/gcc/tree.c b/gcc/tree.c
index ae292281b1f..613efa5f2b0 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -11872,18 +11872,23 @@ build_alloca_call_expr (tree size, unsigned int align, HOST_WIDE_INT max_size)
     }
 }
 
-/* Create a new constant string literal consisting of elements of type
-   ELTYPE and return a tree node representing char* pointer to it as
-   an ADDR_EXPR (ARRAY_REF (ELTYPE, ...)).  The STRING_CST value is
-   the LEN bytes at STR (the representation of the string, which may
+/* Create a new constant string literal of type ELTYPE[SIZE] (or LEN
+   if SIZE == -1) and return a tree node representing char* pointer to
+   it as an ADDR_EXPR (ARRAY_REF (ELTYPE, ...)).  The STRING_CST value
+   is the LEN bytes at STR (the representation of the string, which may
    be wide).  */
 
 tree
 build_string_literal (int len, const char *str,
-		      tree eltype /* = char_type_node */)
+		      tree eltype /* = char_type_node */,
+		      unsigned HOST_WIDE_INT size /* = -1 */)
 {
   tree t = build_string (len, str);
-  tree index = build_index_type (size_int (len - 1));
+  /* Set the maximum valid index based on the string length or SIZE.  */
+  unsigned HOST_WIDE_INT maxidx
+    = (size == HOST_WIDE_INT_M1U ? len : size) - 1;
+
+  tree index = build_index_type (size_int (maxidx));
   eltype = build_type_variant (eltype, 1, 0);
   tree type = build_array_type (eltype, index);
   TREE_TYPE (t) = type;
diff --git a/gcc/tree.h b/gcc/tree.h
index 37f6d57c034..fc85572dffa 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4418,7 +4418,8 @@ extern tree build_call_expr_internal_loc_array (location_t, enum internal_fn,
 extern tree maybe_build_call_expr_loc (location_t, combined_fn, tree,
 				       int, ...);
 extern tree build_alloca_call_expr (tree, unsigned int, HOST_WIDE_INT);
-extern tree build_string_literal (int, const char *, tree = char_type_node);
+extern tree build_string_literal (int, const char *, tree = char_type_node,
+				  unsigned HOST_WIDE_INT = HOST_WIDE_INT_M1U);
 
 /* Construct various nodes representing data types.  */
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-20  8:31   ` Martin Sebor
@ 2019-08-20  8:47     ` Richard Biener
  2019-08-21 21:18       ` Martin Sebor
  2019-08-22 18:50       ` Jeff Law
  0 siblings, 2 replies; 10+ messages in thread
From: Richard Biener @ 2019-08-20  8:47 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Tue, Aug 20, 2019 at 4:32 AM Martin Sebor <msebor@gmail.com> wrote:
>
> On 8/19/19 8:10 AM, Richard Biener wrote:
> > On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> With the recent enhancement to the strlen handling of multibyte
> >> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
> >> started failing on hppa (and probably elsewhere as well).  This
> >> is partly the result of the added detection of past-the-end
> >> writes into the strlen pass which detects more instances of
> >> the problem than -Warray-bounds.  Since the IL each warning
> >> works with varies between targets, the same invalid code can
> >> be diagnosed by one warning one target and different warning
> >> on another.
> >>
> >> The attached patch does three things:
> >>
> >> 1) It enhances compute_objsize to also determine the size of
> >> a flexible array member (and its various variants), including
> >> from its initializer if necessary.  (This resolves 91457 but
> >> introduces another warning where was previously just one.)
> >> 2) It guards the new instance of -Wstringop-overflow with
> >> the no-warning bit on the assignment to avoid warning on code
> >> that's already been diagnosed.
> >> 3) It arranges for -Warray-bounds to set the no-warning bit on
> >> the enclosing expression to keep -Wstringop-overflow from issuing
> >> another warning for the same problem.
> >>
> >> Testing the compute_objsize enhancement to bring it up to par
> >> with -Warray-bounds in turn exposed a weakness in the latter
> >> warning for flexible array members.  Rather than snowballing
> >> additional improvements into this one I decided to put that
> >> off until later, so the new -Warray-bounds test has a bunch
> >> of XFAILs.  I'll see if I can find the time to deal with those
> >> either still in stage 1 or in stage 3 (one of them is actually
> >> an ancient regression).
> >
> > +static tree
> > +get_initializer_for (tree init, tree decl)
> > +{
> >
> > can't you use fold_ctor_reference here?
>
> Yes, but only with an additional enhancement.  Char initializers
> for flexible array members aren't transformed to STRING_CSTs yet,
> so without the size of the initializer specified, the function
> returns the initializer for the smallest subobject, or char in
> this case.  I've enhanced the function to handle them.

So at the moment it returns an empty array constructor, correct?
Isn't that the more canonical representation?  The STRING_CST
index type doesn't really handle "empty" strings and I expect code
more confused about that than about an empty CTOR?

> >
> > +/* Determine the size of the flexible array FLD from the initializer
> > +   expression for the struct object DECL in which the meber is declared
> > +   (possibly recursively).  Return the size or zero constant if it isn't
> > +   initialized.  */
> > +
> > +static tree
> > +get_flexarray_size (tree decl, tree fld)
> > +{
> > +  if (tree init = DECL_INITIAL (decl))
> > +    {
> > +      init = get_initializer_for (init, fld);
> > +      if (init)
> > +       return TYPE_SIZE_UNIT (TREE_TYPE (init));
> > +    }
> > +
> > +  return integer_zero_node;
> >
> > so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
> > returns has a complete type but the initialized object didn't get it
> > completed.  Isnt that wishful thinking?
>
> I don't know what you mean.  When might a CONSTRUCTOR not have
> a complete type, and if/when it doesn't, why would that be
> a problem here?  TYPE_SIZE_UNIT will evaluate to null meaning
> "don't know" and that's fine.  Could you try to be more specific
> about the problem you're pointing out?
>
> > And why return integer_zero_node
> > rather than NULL_TREE here?
>
> Because the size of a flexible array member with no initializer
> is zero.
>
> >
> > +  if (TREE_CODE (dest) == COMPONENT_REF)
> > +    {
> > +      *pdecl = TREE_OPERAND (dest, 1);
> > +
> > +      /* If the member has a size return it.  Otherwise it's a flexible
> > +        array member.  */
> > +      if (tree size = DECL_SIZE_UNIT (*pdecl))
> > +       return size;
> >
> > because here you do.
>
> Not sure what you mean here either.  (This code was also a bit

You return NULL_TREE.

> out of date WRT to the patch I had tested.  Not sure how that
> happened.  The attached patch is up to date.)
>
> >
> > Also once you have an underlying VAR_DECL you can compute
> > the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
> > Isn't that way cheaper than walking the initializer (possibly many
> > times?)
>
> It would be nice if it were this easy.  Is the value of DECL_SIZE
> (var) supposed to include the size of the flexible array member?

Yes, DECL_SIZE of the VAR_DECL is the size we use for assembling.
It is usually only the types that remain incomplete (or too small) since
the FE doesn't create many variants of a struct S { int n; char x[]; }
when "instantiating" it via struct S s = { 5, "abcde" };  that then holds
true for the DECL_SIZE of the FIELD_DECL of x as well.

> I don't see it mentioned in the comments in tree.h and in my tests
> it only does in C but not in C++.  Is that a bug that in C++ it
> doesn't?

tree.h dcoumentation isn't complete.  And for me the C++ FE for
the above simple example has

 <var_decl 0x7ffff7fedb40 s
    type <record_type 0x7ffff6d94f18 S cxx-odr-p type_5 type_6 BLK
        size <integer_cst 0x7ffff6c5e0a8 constant 32>
        unit-size <integer_cst 0x7ffff6c5e0c0 constant 4>
        align:32 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x7ffff6d94f18
        fields <function_decl 0x7ffff6d98b00 __dt  type <method_type
0x7ffff6da8150>
            public abstract external autoinline decl_3 QI t.ii:1:8
align:16 warn_if_not_align:0 context <record_type 0x7ffff6d94f18 S>
            full-name "S::~S() noexcept (<uninstantiated>)"
            not-really-extern chain <function_decl 0x7ffff6d98d00
__dt_base >> context <translation_unit_decl 0x7ffff6c4a168 t.ii>
        full-name "struct S"
        X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
        pointer_to_this <pointer_type 0x7ffff6da85e8>
reference_to_this <reference_type 0x7ffff6da8a80> chain <type_decl
0x7ffff6c6b850 S>>
    public static tree_1 tree_2 tree_3 BLK t.ii:3:3 size <integer_cst
0x7ffff6c5e0a8 32> unit-size <integer_cst 0x7ffff6c5e0c0 4>
    align:32 warn_if_not_align:0 context <translation_unit_decl
0x7ffff6c4a168 t.ii> initial <constructor 0x7ffff6da52a0> chain
<type_decl 0x7ffff6c6b850 S>>

and we assemble it like

        .globl  s
        .data
        .align 4
        .type   s, @object
        .size   s, 4
s:
        .long   5
        .string "abcde"

note how we have .size s, 4 here...

of course in the end nobody cares about a symbols size ...(?)

I expected layout_decl to fixup DECL_SIZE according to the iniitalizer
but it looks like the initializer isn't present when it is layouted.

I think this is a middle-end flaw we should try to fix that either in
the FEs or in the middle-end.  The C FE calls add_flexible_array_elts_to_size
and complete_flexible_array_elts in its finish_decl routine (layouting happens
at build_decl time, too early, but if we want to move this functionality then
re-layouting might be posible).  I'm not sure about other FEs but I guess
the easiest point we could do sanity checking is when assemble_variable
outputs the constructor.

> Attached is an updated patch that uses fold_ctor_reference as you
> suggested.  I've also made a few other minor changes to diagnose
> a few more invalid strlen calls with out-of-bounds offsets.
> (More still remain.)

Thanks, but the patch was already large ... it would be nice to strip it
down to the bare bones again and do followup adjustments instead.

Richard.

> Martin
>
> >
> > Richard.
> >
> >>
> >> Martin
> >>
> >> PS I imagine the new get_flexarray_size function will probably
> >> need to move somewhere more appropriate so that other warnings
> >> (like -Warray-bounds to remove the XFAILs) and optimizations
> >> can make use of it.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-20  8:47     ` Richard Biener
@ 2019-08-21 21:18       ` Martin Sebor
  2019-08-22 11:03         ` Richard Biener
  2019-08-28 18:33         ` Martin Sebor
  2019-08-22 18:50       ` Jeff Law
  1 sibling, 2 replies; 10+ messages in thread
From: Martin Sebor @ 2019-08-21 21:18 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On 8/20/19 1:26 AM, Richard Biener wrote:
> On Tue, Aug 20, 2019 at 4:32 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 8/19/19 8:10 AM, Richard Biener wrote:
>>> On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
>>>>
>>>> With the recent enhancement to the strlen handling of multibyte
>>>> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
>>>> started failing on hppa (and probably elsewhere as well).  This
>>>> is partly the result of the added detection of past-the-end
>>>> writes into the strlen pass which detects more instances of
>>>> the problem than -Warray-bounds.  Since the IL each warning
>>>> works with varies between targets, the same invalid code can
>>>> be diagnosed by one warning one target and different warning
>>>> on another.
>>>>
>>>> The attached patch does three things:
>>>>
>>>> 1) It enhances compute_objsize to also determine the size of
>>>> a flexible array member (and its various variants), including
>>>> from its initializer if necessary.  (This resolves 91457 but
>>>> introduces another warning where was previously just one.)
>>>> 2) It guards the new instance of -Wstringop-overflow with
>>>> the no-warning bit on the assignment to avoid warning on code
>>>> that's already been diagnosed.
>>>> 3) It arranges for -Warray-bounds to set the no-warning bit on
>>>> the enclosing expression to keep -Wstringop-overflow from issuing
>>>> another warning for the same problem.
>>>>
>>>> Testing the compute_objsize enhancement to bring it up to par
>>>> with -Warray-bounds in turn exposed a weakness in the latter
>>>> warning for flexible array members.  Rather than snowballing
>>>> additional improvements into this one I decided to put that
>>>> off until later, so the new -Warray-bounds test has a bunch
>>>> of XFAILs.  I'll see if I can find the time to deal with those
>>>> either still in stage 1 or in stage 3 (one of them is actually
>>>> an ancient regression).
>>>
>>> +static tree
>>> +get_initializer_for (tree init, tree decl)
>>> +{
>>>
>>> can't you use fold_ctor_reference here?
>>
>> Yes, but only with an additional enhancement.  Char initializers
>> for flexible array members aren't transformed to STRING_CSTs yet,
>> so without the size of the initializer specified, the function
>> returns the initializer for the smallest subobject, or char in
>> this case.  I've enhanced the function to handle them.
> 
> So at the moment it returns an empty array constructor, correct?
> Isn't that the more canonical representation?  The STRING_CST
> index type doesn't really handle "empty" strings and I expect code
> more confused about that than about an empty CTOR?

Yes.  Returning an empty CTOR is more general than an empty
string and enables more optimizations.  It requires changing
the caller(s) that look for a string but I think that's fine.
Thanks for the hint!

>>> +/* Determine the size of the flexible array FLD from the initializer
>>> +   expression for the struct object DECL in which the meber is declared
>>> +   (possibly recursively).  Return the size or zero constant if it isn't
>>> +   initialized.  */
>>> +
>>> +static tree
>>> +get_flexarray_size (tree decl, tree fld)
>>> +{
>>> +  if (tree init = DECL_INITIAL (decl))
>>> +    {
>>> +      init = get_initializer_for (init, fld);
>>> +      if (init)
>>> +       return TYPE_SIZE_UNIT (TREE_TYPE (init));
>>> +    }
>>> +
>>> +  return integer_zero_node;
>>>
>>> so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
>>> returns has a complete type but the initialized object didn't get it
>>> completed.  Isnt that wishful thinking?
>>
>> I don't know what you mean.  When might a CONSTRUCTOR not have
>> a complete type, and if/when it doesn't, why would that be
>> a problem here?  TYPE_SIZE_UNIT will evaluate to null meaning
>> "don't know" and that's fine.  Could you try to be more specific
>> about the problem you're pointing out?
>>
>>> And why return integer_zero_node
>>> rather than NULL_TREE here?
>>
>> Because the size of a flexible array member with no initializer
>> is zero.
>>
>>>
>>> +  if (TREE_CODE (dest) == COMPONENT_REF)
>>> +    {
>>> +      *pdecl = TREE_OPERAND (dest, 1);
>>> +
>>> +      /* If the member has a size return it.  Otherwise it's a flexible
>>> +        array member.  */
>>> +      if (tree size = DECL_SIZE_UNIT (*pdecl))
>>> +       return size;
>>>
>>> because here you do.
>>
>> Not sure what you mean here either.  (This code was also a bit
> 
> You return NULL_TREE.
> 
>> out of date WRT to the patch I had tested.  Not sure how that
>> happened.  The attached patch is up to date.)
>>
>>>
>>> Also once you have an underlying VAR_DECL you can compute
>>> the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
>>> Isn't that way cheaper than walking the initializer (possibly many
>>> times?)
>>
>> It would be nice if it were this easy.  Is the value of DECL_SIZE
>> (var) supposed to include the size of the flexible array member?
> 
> Yes, DECL_SIZE of the VAR_DECL is the size we use for assembling.
> It is usually only the types that remain incomplete (or too small) since
> the FE doesn't create many variants of a struct S { int n; char x[]; }
> when "instantiating" it via struct S s = { 5, "abcde" };  that then holds
> true for the DECL_SIZE of the FIELD_DECL of x as well.
> 
>> I don't see it mentioned in the comments in tree.h and in my tests
>> it only does in C but not in C++.  Is that a bug that in C++ it
>> doesn't?
> 
> tree.h dcoumentation isn't complete.  And for me the C++ FE for
> the above simple example has
> 
>   <var_decl 0x7ffff7fedb40 s
>      type <record_type 0x7ffff6d94f18 S cxx-odr-p type_5 type_6 BLK
>          size <integer_cst 0x7ffff6c5e0a8 constant 32>
>          unit-size <integer_cst 0x7ffff6c5e0c0 constant 4>
>          align:32 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x7ffff6d94f18
>          fields <function_decl 0x7ffff6d98b00 __dt  type <method_type
> 0x7ffff6da8150>
>              public abstract external autoinline decl_3 QI t.ii:1:8
> align:16 warn_if_not_align:0 context <record_type 0x7ffff6d94f18 S>
>              full-name "S::~S() noexcept (<uninstantiated>)"
>              not-really-extern chain <function_decl 0x7ffff6d98d00
> __dt_base >> context <translation_unit_decl 0x7ffff6c4a168 t.ii>
>          full-name "struct S"
>          X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
>          pointer_to_this <pointer_type 0x7ffff6da85e8>
> reference_to_this <reference_type 0x7ffff6da8a80> chain <type_decl
> 0x7ffff6c6b850 S>>
>      public static tree_1 tree_2 tree_3 BLK t.ii:3:3 size <integer_cst
> 0x7ffff6c5e0a8 32> unit-size <integer_cst 0x7ffff6c5e0c0 4>
>      align:32 warn_if_not_align:0 context <translation_unit_decl
> 0x7ffff6c4a168 t.ii> initial <constructor 0x7ffff6da52a0> chain
> <type_decl 0x7ffff6c6b850 S>>
> 
> and we assemble it like
> 
>          .globl  s
>          .data
>          .align 4
>          .type   s, @object
>          .size   s, 4
> s:
>          .long   5
>          .string "abcde"
> 
> note how we have .size s, 4 here...
> 
> of course in the end nobody cares about a symbols size ...(?)
> 
> I expected layout_decl to fixup DECL_SIZE according to the iniitalizer
> but it looks like the initializer isn't present when it is layouted.
> 
> I think this is a middle-end flaw we should try to fix that either in
> the FEs or in the middle-end.  The C FE calls add_flexible_array_elts_to_size
> and complete_flexible_array_elts in its finish_decl routine (layouting happens
> at build_decl time, too early, but if we want to move this functionality then
> re-layouting might be posible).  I'm not sure about other FEs but I guess
> the easiest point we could do sanity checking is when assemble_variable
> outputs the constructor.

I don't know if I followed all this but it sounds like you're
saying this is a separate issue.

> 
>> Attached is an updated patch that uses fold_ctor_reference as you
>> suggested.  I've also made a few other minor changes to diagnose
>> a few more invalid strlen calls with out-of-bounds offsets.
>> (More still remain.)
> 
> Thanks, but the patch was already large ... it would be nice to strip it
> down to the bare bones again and do followup adjustments instead.

I'm not sure what you view as the bare bones bits but I took
a guess and split off the changes for PR 91490 (missing folding
of constant flexible array members) into a separate patch and
posted that.  Once that's approved I'll update this one and post
what's left of it.

Martin

> 
> Richard.
> 
>> Martin
>>
>>>
>>> Richard.
>>>
>>>>
>>>> Martin
>>>>
>>>> PS I imagine the new get_flexarray_size function will probably
>>>> need to move somewhere more appropriate so that other warnings
>>>> (like -Warray-bounds to remove the XFAILs) and optimizations
>>>> can make use of it.
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-21 21:18       ` Martin Sebor
@ 2019-08-22 11:03         ` Richard Biener
  2019-08-22 19:59           ` Jason Merrill
  2019-08-28 18:33         ` Martin Sebor
  1 sibling, 1 reply; 10+ messages in thread
From: Richard Biener @ 2019-08-22 11:03 UTC (permalink / raw)
  To: Martin Sebor, Jason Merrill; +Cc: gcc-patches

On Wed, Aug 21, 2019 at 10:10 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 8/20/19 1:26 AM, Richard Biener wrote:
> > On Tue, Aug 20, 2019 at 4:32 AM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> On 8/19/19 8:10 AM, Richard Biener wrote:
> >>> On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
> >>>>
> >>>> With the recent enhancement to the strlen handling of multibyte
> >>>> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
> >>>> started failing on hppa (and probably elsewhere as well).  This
> >>>> is partly the result of the added detection of past-the-end
> >>>> writes into the strlen pass which detects more instances of
> >>>> the problem than -Warray-bounds.  Since the IL each warning
> >>>> works with varies between targets, the same invalid code can
> >>>> be diagnosed by one warning one target and different warning
> >>>> on another.
> >>>>
> >>>> The attached patch does three things:
> >>>>
> >>>> 1) It enhances compute_objsize to also determine the size of
> >>>> a flexible array member (and its various variants), including
> >>>> from its initializer if necessary.  (This resolves 91457 but
> >>>> introduces another warning where was previously just one.)
> >>>> 2) It guards the new instance of -Wstringop-overflow with
> >>>> the no-warning bit on the assignment to avoid warning on code
> >>>> that's already been diagnosed.
> >>>> 3) It arranges for -Warray-bounds to set the no-warning bit on
> >>>> the enclosing expression to keep -Wstringop-overflow from issuing
> >>>> another warning for the same problem.
> >>>>
> >>>> Testing the compute_objsize enhancement to bring it up to par
> >>>> with -Warray-bounds in turn exposed a weakness in the latter
> >>>> warning for flexible array members.  Rather than snowballing
> >>>> additional improvements into this one I decided to put that
> >>>> off until later, so the new -Warray-bounds test has a bunch
> >>>> of XFAILs.  I'll see if I can find the time to deal with those
> >>>> either still in stage 1 or in stage 3 (one of them is actually
> >>>> an ancient regression).
> >>>
> >>> +static tree
> >>> +get_initializer_for (tree init, tree decl)
> >>> +{
> >>>
> >>> can't you use fold_ctor_reference here?
> >>
> >> Yes, but only with an additional enhancement.  Char initializers
> >> for flexible array members aren't transformed to STRING_CSTs yet,
> >> so without the size of the initializer specified, the function
> >> returns the initializer for the smallest subobject, or char in
> >> this case.  I've enhanced the function to handle them.
> >
> > So at the moment it returns an empty array constructor, correct?
> > Isn't that the more canonical representation?  The STRING_CST
> > index type doesn't really handle "empty" strings and I expect code
> > more confused about that than about an empty CTOR?
>
> Yes.  Returning an empty CTOR is more general than an empty
> string and enables more optimizations.  It requires changing
> the caller(s) that look for a string but I think that's fine.
> Thanks for the hint!
>
> >>> +/* Determine the size of the flexible array FLD from the initializer
> >>> +   expression for the struct object DECL in which the meber is declared
> >>> +   (possibly recursively).  Return the size or zero constant if it isn't
> >>> +   initialized.  */
> >>> +
> >>> +static tree
> >>> +get_flexarray_size (tree decl, tree fld)
> >>> +{
> >>> +  if (tree init = DECL_INITIAL (decl))
> >>> +    {
> >>> +      init = get_initializer_for (init, fld);
> >>> +      if (init)
> >>> +       return TYPE_SIZE_UNIT (TREE_TYPE (init));
> >>> +    }
> >>> +
> >>> +  return integer_zero_node;
> >>>
> >>> so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
> >>> returns has a complete type but the initialized object didn't get it
> >>> completed.  Isnt that wishful thinking?
> >>
> >> I don't know what you mean.  When might a CONSTRUCTOR not have
> >> a complete type, and if/when it doesn't, why would that be
> >> a problem here?  TYPE_SIZE_UNIT will evaluate to null meaning
> >> "don't know" and that's fine.  Could you try to be more specific
> >> about the problem you're pointing out?
> >>
> >>> And why return integer_zero_node
> >>> rather than NULL_TREE here?
> >>
> >> Because the size of a flexible array member with no initializer
> >> is zero.
> >>
> >>>
> >>> +  if (TREE_CODE (dest) == COMPONENT_REF)
> >>> +    {
> >>> +      *pdecl = TREE_OPERAND (dest, 1);
> >>> +
> >>> +      /* If the member has a size return it.  Otherwise it's a flexible
> >>> +        array member.  */
> >>> +      if (tree size = DECL_SIZE_UNIT (*pdecl))
> >>> +       return size;
> >>>
> >>> because here you do.
> >>
> >> Not sure what you mean here either.  (This code was also a bit
> >
> > You return NULL_TREE.
> >
> >> out of date WRT to the patch I had tested.  Not sure how that
> >> happened.  The attached patch is up to date.)
> >>
> >>>
> >>> Also once you have an underlying VAR_DECL you can compute
> >>> the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
> >>> Isn't that way cheaper than walking the initializer (possibly many
> >>> times?)
> >>
> >> It would be nice if it were this easy.  Is the value of DECL_SIZE
> >> (var) supposed to include the size of the flexible array member?
> >
> > Yes, DECL_SIZE of the VAR_DECL is the size we use for assembling.
> > It is usually only the types that remain incomplete (or too small) since
> > the FE doesn't create many variants of a struct S { int n; char x[]; }
> > when "instantiating" it via struct S s = { 5, "abcde" };  that then holds
> > true for the DECL_SIZE of the FIELD_DECL of x as well.
> >
> >> I don't see it mentioned in the comments in tree.h and in my tests
> >> it only does in C but not in C++.  Is that a bug that in C++ it
> >> doesn't?
> >
> > tree.h dcoumentation isn't complete.  And for me the C++ FE for
> > the above simple example has
> >
> >   <var_decl 0x7ffff7fedb40 s
> >      type <record_type 0x7ffff6d94f18 S cxx-odr-p type_5 type_6 BLK
> >          size <integer_cst 0x7ffff6c5e0a8 constant 32>
> >          unit-size <integer_cst 0x7ffff6c5e0c0 constant 4>
> >          align:32 warn_if_not_align:0 symtab:0 alias-set -1
> > canonical-type 0x7ffff6d94f18
> >          fields <function_decl 0x7ffff6d98b00 __dt  type <method_type
> > 0x7ffff6da8150>
> >              public abstract external autoinline decl_3 QI t.ii:1:8
> > align:16 warn_if_not_align:0 context <record_type 0x7ffff6d94f18 S>
> >              full-name "S::~S() noexcept (<uninstantiated>)"
> >              not-really-extern chain <function_decl 0x7ffff6d98d00
> > __dt_base >> context <translation_unit_decl 0x7ffff6c4a168 t.ii>
> >          full-name "struct S"
> >          X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
> >          pointer_to_this <pointer_type 0x7ffff6da85e8>
> > reference_to_this <reference_type 0x7ffff6da8a80> chain <type_decl
> > 0x7ffff6c6b850 S>>
> >      public static tree_1 tree_2 tree_3 BLK t.ii:3:3 size <integer_cst
> > 0x7ffff6c5e0a8 32> unit-size <integer_cst 0x7ffff6c5e0c0 4>
> >      align:32 warn_if_not_align:0 context <translation_unit_decl
> > 0x7ffff6c4a168 t.ii> initial <constructor 0x7ffff6da52a0> chain
> > <type_decl 0x7ffff6c6b850 S>>
> >
> > and we assemble it like
> >
> >          .globl  s
> >          .data
> >          .align 4
> >          .type   s, @object
> >          .size   s, 4
> > s:
> >          .long   5
> >          .string "abcde"
> >
> > note how we have .size s, 4 here...
> >
> > of course in the end nobody cares about a symbols size ...(?)
> >
> > I expected layout_decl to fixup DECL_SIZE according to the iniitalizer
> > but it looks like the initializer isn't present when it is layouted.
> >
> > I think this is a middle-end flaw we should try to fix that either in
> > the FEs or in the middle-end.  The C FE calls add_flexible_array_elts_to_size
> > and complete_flexible_array_elts in its finish_decl routine (layouting happens
> > at build_decl time, too early, but if we want to move this functionality then
> > re-layouting might be posible).  I'm not sure about other FEs but I guess
> > the easiest point we could do sanity checking is when assemble_variable
> > outputs the constructor.
>
> I don't know if I followed all this but it sounds like you're
> saying this is a separate issue.

Yeah, it looks like a separate issue to me but one that would be
nice to have fixed so we can compute the length of the trailing
array without needing to parse the initializer.  I'd even say we should
delay that effort if the above is confirmed as a bug and a fix is
possible.  Jason, is the above intended behavior?  I realize
"trailing" arrays is a GNU extension in C++ but as seen above
it doesn't match GNU C behavior here.

As said, I'm not sure if a wrong symbol size has an impact on
any target binary format we support.

> >
> >> Attached is an updated patch that uses fold_ctor_reference as you
> >> suggested.  I've also made a few other minor changes to diagnose
> >> a few more invalid strlen calls with out-of-bounds offsets.
> >> (More still remain.)
> >
> > Thanks, but the patch was already large ... it would be nice to strip it
> > down to the bare bones again and do followup adjustments instead.
>
> I'm not sure what you view as the bare bones bits but I took
> a guess and split off the changes for PR 91490 (missing folding
> of constant flexible array members) into a separate patch and
> posted that.  Once that's approved I'll update this one and post
> what's left of it.

Thanks,
Richard.

> Martin
>
> >
> > Richard.
> >
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
> >>>>
> >>>> Martin
> >>>>
> >>>> PS I imagine the new get_flexarray_size function will probably
> >>>> need to move somewhere more appropriate so that other warnings
> >>>> (like -Warray-bounds to remove the XFAILs) and optimizations
> >>>> can make use of it.
> >>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-20  8:47     ` Richard Biener
  2019-08-21 21:18       ` Martin Sebor
@ 2019-08-22 18:50       ` Jeff Law
  1 sibling, 0 replies; 10+ messages in thread
From: Jeff Law @ 2019-08-22 18:50 UTC (permalink / raw)
  To: Richard Biener, Martin Sebor; +Cc: gcc-patches

On 8/20/19 1:26 AM, Richard Biener wrote:
> On Tue, Aug 20, 2019 at 4:32 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 8/19/19 8:10 AM, Richard Biener wrote:
>>> On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
>>>>
>>>> With the recent enhancement to the strlen handling of multibyte
>>>> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
>>>> started failing on hppa (and probably elsewhere as well).  This
>>>> is partly the result of the added detection of past-the-end
>>>> writes into the strlen pass which detects more instances of
>>>> the problem than -Warray-bounds.  Since the IL each warning
>>>> works with varies between targets, the same invalid code can
>>>> be diagnosed by one warning one target and different warning
>>>> on another.
>>>>
>>>> The attached patch does three things:
>>>>
>>>> 1) It enhances compute_objsize to also determine the size of
>>>> a flexible array member (and its various variants), including
>>>> from its initializer if necessary.  (This resolves 91457 but
>>>> introduces another warning where was previously just one.)
>>>> 2) It guards the new instance of -Wstringop-overflow with
>>>> the no-warning bit on the assignment to avoid warning on code
>>>> that's already been diagnosed.
>>>> 3) It arranges for -Warray-bounds to set the no-warning bit on
>>>> the enclosing expression to keep -Wstringop-overflow from issuing
>>>> another warning for the same problem.
>>>>
>>>> Testing the compute_objsize enhancement to bring it up to par
>>>> with -Warray-bounds in turn exposed a weakness in the latter
>>>> warning for flexible array members.  Rather than snowballing
>>>> additional improvements into this one I decided to put that
>>>> off until later, so the new -Warray-bounds test has a bunch
>>>> of XFAILs.  I'll see if I can find the time to deal with those
>>>> either still in stage 1 or in stage 3 (one of them is actually
>>>> an ancient regression).
>>>
>>> +static tree
>>> +get_initializer_for (tree init, tree decl)
>>> +{
>>>
>>> can't you use fold_ctor_reference here?
>>
>> Yes, but only with an additional enhancement.  Char initializers
>> for flexible array members aren't transformed to STRING_CSTs yet,
>> so without the size of the initializer specified, the function
>> returns the initializer for the smallest subobject, or char in
>> this case.  I've enhanced the function to handle them.
> 
> So at the moment it returns an empty array constructor, correct?
> Isn't that the more canonical representation?  The STRING_CST
> index type doesn't really handle "empty" strings and I expect code
> more confused about that than about an empty CTOR?
> 
>>>
>>> +/* Determine the size of the flexible array FLD from the initializer
>>> +   expression for the struct object DECL in which the meber is declared
>>> +   (possibly recursively).  Return the size or zero constant if it isn't
>>> +   initialized.  */
>>> +
>>> +static tree
>>> +get_flexarray_size (tree decl, tree fld)
>>> +{
>>> +  if (tree init = DECL_INITIAL (decl))
>>> +    {
>>> +      init = get_initializer_for (init, fld);
>>> +      if (init)
>>> +       return TYPE_SIZE_UNIT (TREE_TYPE (init));
>>> +    }
>>> +
>>> +  return integer_zero_node;
>>>
>>> so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
>>> returns has a complete type but the initialized object didn't get it
>>> completed.  Isnt that wishful thinking?
>>
>> I don't know what you mean.  When might a CONSTRUCTOR not have
>> a complete type, and if/when it doesn't, why would that be
>> a problem here?  TYPE_SIZE_UNIT will evaluate to null meaning
>> "don't know" and that's fine.  Could you try to be more specific
>> about the problem you're pointing out?
>>
>>> And why return integer_zero_node
>>> rather than NULL_TREE here?
>>
>> Because the size of a flexible array member with no initializer
>> is zero.
>>
>>>
>>> +  if (TREE_CODE (dest) == COMPONENT_REF)
>>> +    {
>>> +      *pdecl = TREE_OPERAND (dest, 1);
>>> +
>>> +      /* If the member has a size return it.  Otherwise it's a flexible
>>> +        array member.  */
>>> +      if (tree size = DECL_SIZE_UNIT (*pdecl))
>>> +       return size;
>>>
>>> because here you do.
>>
>> Not sure what you mean here either.  (This code was also a bit
> 
> You return NULL_TREE.
> 
>> out of date WRT to the patch I had tested.  Not sure how that
>> happened.  The attached patch is up to date.)
>>
>>>
>>> Also once you have an underlying VAR_DECL you can compute
>>> the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
>>> Isn't that way cheaper than walking the initializer (possibly many
>>> times?)
>>
>> It would be nice if it were this easy.  Is the value of DECL_SIZE
>> (var) supposed to include the size of the flexible array member?
> 
> Yes, DECL_SIZE of the VAR_DECL is the size we use for assembling.
> It is usually only the types that remain incomplete (or too small) since
> the FE doesn't create many variants of a struct S { int n; char x[]; }
> when "instantiating" it via struct S s = { 5, "abcde" };  that then holds
> true for the DECL_SIZE of the FIELD_DECL of x as well.
> 
>> I don't see it mentioned in the comments in tree.h and in my tests
>> it only does in C but not in C++.  Is that a bug that in C++ it
>> doesn't?
> 
> tree.h dcoumentation isn't complete.  And for me the C++ FE for
> the above simple example has
> 
>  <var_decl 0x7ffff7fedb40 s
>     type <record_type 0x7ffff6d94f18 S cxx-odr-p type_5 type_6 BLK
>         size <integer_cst 0x7ffff6c5e0a8 constant 32>
>         unit-size <integer_cst 0x7ffff6c5e0c0 constant 4>
>         align:32 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x7ffff6d94f18
>         fields <function_decl 0x7ffff6d98b00 __dt  type <method_type
> 0x7ffff6da8150>
>             public abstract external autoinline decl_3 QI t.ii:1:8
> align:16 warn_if_not_align:0 context <record_type 0x7ffff6d94f18 S>
>             full-name "S::~S() noexcept (<uninstantiated>)"
>             not-really-extern chain <function_decl 0x7ffff6d98d00
> __dt_base >> context <translation_unit_decl 0x7ffff6c4a168 t.ii>
>         full-name "struct S"
>         X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
>         pointer_to_this <pointer_type 0x7ffff6da85e8>
> reference_to_this <reference_type 0x7ffff6da8a80> chain <type_decl
> 0x7ffff6c6b850 S>>
>     public static tree_1 tree_2 tree_3 BLK t.ii:3:3 size <integer_cst
> 0x7ffff6c5e0a8 32> unit-size <integer_cst 0x7ffff6c5e0c0 4>
>     align:32 warn_if_not_align:0 context <translation_unit_decl
> 0x7ffff6c4a168 t.ii> initial <constructor 0x7ffff6da52a0> chain
> <type_decl 0x7ffff6c6b850 S>>
> 
> and we assemble it like
> 
>         .globl  s
>         .data
>         .align 4
>         .type   s, @object
>         .size   s, 4
> s:
>         .long   5
>         .string "abcde"
> 
> note how we have .size s, 4 here...
> 
> of course in the end nobody cares about a symbols size ...(?)
The linker may care.  I've certainly run across those which would
warn/error when the size of an object with global scope differed across
TUs.   I wouldn't expect to be running into that kind of issue in the
modern world though.

Jeff

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-22 11:03         ` Richard Biener
@ 2019-08-22 19:59           ` Jason Merrill
  0 siblings, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2019-08-22 19:59 UTC (permalink / raw)
  To: Richard Biener; +Cc: Martin Sebor, gcc-patches

On Thu, Aug 22, 2019 at 3:43 AM Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Aug 21, 2019 at 10:10 PM Martin Sebor <msebor@gmail.com> wrote:
> > On 8/20/19 1:26 AM, Richard Biener wrote:
> > > On Tue, Aug 20, 2019 at 4:32 AM Martin Sebor <msebor@gmail.com> wrote:
> > >> On 8/19/19 8:10 AM, Richard Biener wrote:
> > >>> On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
> > >>>>
> > >>>> With the recent enhancement to the strlen handling of multibyte
> > >>>> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
> > >>>> started failing on hppa (and probably elsewhere as well).  This
> > >>>> is partly the result of the added detection of past-the-end
> > >>>> writes into the strlen pass which detects more instances of
> > >>>> the problem than -Warray-bounds.  Since the IL each warning
> > >>>> works with varies between targets, the same invalid code can
> > >>>> be diagnosed by one warning one target and different warning
> > >>>> on another.
> > >>>>
> > >>>> The attached patch does three things:
> > >>>>
> > >>>> 1) It enhances compute_objsize to also determine the size of
> > >>>> a flexible array member (and its various variants), including
> > >>>> from its initializer if necessary.  (This resolves 91457 but
> > >>>> introduces another warning where was previously just one.)
> > >>>> 2) It guards the new instance of -Wstringop-overflow with
> > >>>> the no-warning bit on the assignment to avoid warning on code
> > >>>> that's already been diagnosed.
> > >>>> 3) It arranges for -Warray-bounds to set the no-warning bit on
> > >>>> the enclosing expression to keep -Wstringop-overflow from issuing
> > >>>> another warning for the same problem.
> > >>>>
> > >>>> Testing the compute_objsize enhancement to bring it up to par
> > >>>> with -Warray-bounds in turn exposed a weakness in the latter
> > >>>> warning for flexible array members.  Rather than snowballing
> > >>>> additional improvements into this one I decided to put that
> > >>>> off until later, so the new -Warray-bounds test has a bunch
> > >>>> of XFAILs.  I'll see if I can find the time to deal with those
> > >>>> either still in stage 1 or in stage 3 (one of them is actually
> > >>>> an ancient regression).
> > >>>
> > >>> +static tree
> > >>> +get_initializer_for (tree init, tree decl)
> > >>> +{
> > >>>
> > >>> can't you use fold_ctor_reference here?
> > >>
> > >> Yes, but only with an additional enhancement.  Char initializers
> > >> for flexible array members aren't transformed to STRING_CSTs yet,
> > >> so without the size of the initializer specified, the function
> > >> returns the initializer for the smallest subobject, or char in
> > >> this case.  I've enhanced the function to handle them.
> > >
> > > So at the moment it returns an empty array constructor, correct?
> > > Isn't that the more canonical representation?  The STRING_CST
> > > index type doesn't really handle "empty" strings and I expect code
> > > more confused about that than about an empty CTOR?
> >
> > Yes.  Returning an empty CTOR is more general than an empty
> > string and enables more optimizations.  It requires changing
> > the caller(s) that look for a string but I think that's fine.
> > Thanks for the hint!
> >
> > >>> +/* Determine the size of the flexible array FLD from the initializer
> > >>> +   expression for the struct object DECL in which the meber is declared
> > >>> +   (possibly recursively).  Return the size or zero constant if it isn't
> > >>> +   initialized.  */
> > >>> +
> > >>> +static tree
> > >>> +get_flexarray_size (tree decl, tree fld)
> > >>> +{
> > >>> +  if (tree init = DECL_INITIAL (decl))
> > >>> +    {
> > >>> +      init = get_initializer_for (init, fld);
> > >>> +      if (init)
> > >>> +       return TYPE_SIZE_UNIT (TREE_TYPE (init));
> > >>> +    }
> > >>> +
> > >>> +  return integer_zero_node;
> > >>>
> > >>> so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
> > >>> returns has a complete type but the initialized object didn't get it
> > >>> completed.  Isnt that wishful thinking?
> > >>
> > >> I don't know what you mean.  When might a CONSTRUCTOR not have
> > >> a complete type, and if/when it doesn't, why would that be
> > >> a problem here?  TYPE_SIZE_UNIT will evaluate to null meaning
> > >> "don't know" and that's fine.  Could you try to be more specific
> > >> about the problem you're pointing out?
> > >>
> > >>> And why return integer_zero_node
> > >>> rather than NULL_TREE here?
> > >>
> > >> Because the size of a flexible array member with no initializer
> > >> is zero.
> > >>
> > >>>
> > >>> +  if (TREE_CODE (dest) == COMPONENT_REF)
> > >>> +    {
> > >>> +      *pdecl = TREE_OPERAND (dest, 1);
> > >>> +
> > >>> +      /* If the member has a size return it.  Otherwise it's a flexible
> > >>> +        array member.  */
> > >>> +      if (tree size = DECL_SIZE_UNIT (*pdecl))
> > >>> +       return size;
> > >>>
> > >>> because here you do.
> > >>
> > >> Not sure what you mean here either.  (This code was also a bit
> > >
> > > You return NULL_TREE.
> > >
> > >> out of date WRT to the patch I had tested.  Not sure how that
> > >> happened.  The attached patch is up to date.)
> > >>
> > >>>
> > >>> Also once you have an underlying VAR_DECL you can compute
> > >>> the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
> > >>> Isn't that way cheaper than walking the initializer (possibly many
> > >>> times?)
> > >>
> > >> It would be nice if it were this easy.  Is the value of DECL_SIZE
> > >> (var) supposed to include the size of the flexible array member?
> > >
> > > Yes, DECL_SIZE of the VAR_DECL is the size we use for assembling.
> > > It is usually only the types that remain incomplete (or too small) since
> > > the FE doesn't create many variants of a struct S { int n; char x[]; }
> > > when "instantiating" it via struct S s = { 5, "abcde" };  that then holds
> > > true for the DECL_SIZE of the FIELD_DECL of x as well.
> > >
> > >> I don't see it mentioned in the comments in tree.h and in my tests
> > >> it only does in C but not in C++.  Is that a bug that in C++ it
> > >> doesn't?
> > >
> > > tree.h dcoumentation isn't complete.  And for me the C++ FE for
> > > the above simple example has
> > >
> > >   <var_decl 0x7ffff7fedb40 s
> > >      type <record_type 0x7ffff6d94f18 S cxx-odr-p type_5 type_6 BLK
> > >          size <integer_cst 0x7ffff6c5e0a8 constant 32>
> > >          unit-size <integer_cst 0x7ffff6c5e0c0 constant 4>
> > >          align:32 warn_if_not_align:0 symtab:0 alias-set -1
> > > canonical-type 0x7ffff6d94f18
> > >          fields <function_decl 0x7ffff6d98b00 __dt  type <method_type
> > > 0x7ffff6da8150>
> > >              public abstract external autoinline decl_3 QI t.ii:1:8
> > > align:16 warn_if_not_align:0 context <record_type 0x7ffff6d94f18 S>
> > >              full-name "S::~S() noexcept (<uninstantiated>)"
> > >              not-really-extern chain <function_decl 0x7ffff6d98d00
> > > __dt_base >> context <translation_unit_decl 0x7ffff6c4a168 t.ii>
> > >          full-name "struct S"
> > >          X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown
> > >          pointer_to_this <pointer_type 0x7ffff6da85e8>
> > > reference_to_this <reference_type 0x7ffff6da8a80> chain <type_decl
> > > 0x7ffff6c6b850 S>>
> > >      public static tree_1 tree_2 tree_3 BLK t.ii:3:3 size <integer_cst
> > > 0x7ffff6c5e0a8 32> unit-size <integer_cst 0x7ffff6c5e0c0 4>
> > >      align:32 warn_if_not_align:0 context <translation_unit_decl
> > > 0x7ffff6c4a168 t.ii> initial <constructor 0x7ffff6da52a0> chain
> > > <type_decl 0x7ffff6c6b850 S>>
> > >
> > > and we assemble it like
> > >
> > >          .globl  s
> > >          .data
> > >          .align 4
> > >          .type   s, @object
> > >          .size   s, 4
> > > s:
> > >          .long   5
> > >          .string "abcde"
> > >
> > > note how we have .size s, 4 here...
> > >
> > > of course in the end nobody cares about a symbols size ...(?)
> > >
> > > I expected layout_decl to fixup DECL_SIZE according to the iniitalizer
> > > but it looks like the initializer isn't present when it is layouted.
> > >
> > > I think this is a middle-end flaw we should try to fix that either in
> > > the FEs or in the middle-end.  The C FE calls add_flexible_array_elts_to_size
> > > and complete_flexible_array_elts in its finish_decl routine (layouting happens
> > > at build_decl time, too early, but if we want to move this functionality then
> > > re-layouting might be posible).  I'm not sure about other FEs but I guess
> > > the easiest point we could do sanity checking is when assemble_variable
> > > outputs the constructor.
> >
> > I don't know if I followed all this but it sounds like you're
> > saying this is a separate issue.
>
> Yeah, it looks like a separate issue to me but one that would be
> nice to have fixed so we can compute the length of the trailing
> array without needing to parse the initializer.  I'd even say we should
> delay that effort if the above is confirmed as a bug and a fix is
> possible.  Jason, is the above intended behavior?  I realize
> "trailing" arrays is a GNU extension in C++ but as seen above
> it doesn't match GNU C behavior here.

Having DECL_SIZE different from GNU C for the same example is
definitely not intended.

Jason

> As said, I'm not sure if a wrong symbol size has an impact on
> any target binary format we support.
>
> > >
> > >> Attached is an updated patch that uses fold_ctor_reference as you
> > >> suggested.  I've also made a few other minor changes to diagnose
> > >> a few more invalid strlen calls with out-of-bounds offsets.
> > >> (More still remain.)
> > >
> > > Thanks, but the patch was already large ... it would be nice to strip it
> > > down to the bare bones again and do followup adjustments instead.
> >
> > I'm not sure what you view as the bare bones bits but I took
> > a guess and split off the changes for PR 91490 (missing folding
> > of constant flexible array members) into a separate patch and
> > posted that.  Once that's approved I'll update this one and post
> > what's left of it.
>
> Thanks,
> Richard.
>
> > Martin
> >
> > >
> > > Richard.
> > >
> > >> Martin
> > >>
> > >>>
> > >>> Richard.
> > >>>
> > >>>>
> > >>>> Martin
> > >>>>
> > >>>> PS I imagine the new get_flexarray_size function will probably
> > >>>> need to move somewhere more appropriate so that other warnings
> > >>>> (like -Warray-bounds to remove the XFAILs) and optimizations
> > >>>> can make use of it.
> > >>
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-21 21:18       ` Martin Sebor
  2019-08-22 11:03         ` Richard Biener
@ 2019-08-28 18:33         ` Martin Sebor
  2019-08-28 18:43           ` Jeff Law
  1 sibling, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2019-08-28 18:33 UTC (permalink / raw)
  To: Richard Biener, Jeff Law; +Cc: gcc-patches

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

With the PR 91490 subset committed, attached is what's left
of the original fix.  It seems simple enough that I decided it's
not worth chopping up any further.  The meat of the change is in
builtins.c.  Everything else just adjusts warnings either by
making use of it (tree-ssa-strlen.c) or by setting the no-warning
bit (tree-vrp.c).

Retested on x86_64-linux.

Martin

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

PR tree-optimization/91457 - inconsistent warning for writing past the end of an array member

gcc/ChangeLog:

	PR tree-optimization/91457
	* builtins.c (component_size): New function.
	(compute_objsize): Add argument. Handle ARRAY_REF and COMPONENT_REF.
	* builtins.h (compute_objsize): Add argument.
	* tree-ssa-strlen.c (handle_store): Handle no-warning bit.
	* tree-vrp.c (vrp_prop::check_array_ref): Return warning result.
	(vrp_prop::check_mem_ref): Same.
	(vrp_prop::search_for_addr_array): Set no-warning bit.
	(check_array_bounds): Same.

gcc/testsuite/ChangeLog:

	PR tree-optimization/91457
	* c-c++-common/Wstringop-overflow-2.c: New test.
	* g++.dg/warn/Warray-bounds-8.C: New test.
	* g++.dg/warn/Wstringop-overflow-3.C: New test.
	* gcc.dg/Wstringop-overflow-15.c: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 0b25adc17a0..f8063c138a3 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -72,6 +72,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "file-prefix-map.h" /* remap_macro_filename()  */
 #include "gomp-constants.h"
 #include "omp-general.h"
+#include "tree-dfa.h"
 
 struct target_builtins default_target_builtins;
 #if SWITCHABLE_TARGET
@@ -3561,6 +3562,54 @@ check_access (tree exp, tree, tree, tree dstwrite,
   return true;
 }
 
+/* Determines the size of the member referenced by the COMPONENT_REF
+   REF, using its initializer expression if necessary in order to
+   determine the size of an initialized flexible array member.
+   Returns the size (which might be zero for an object with
+   an uninitialized flexible array member) or null if the size
+   cannot be determined.  */
+
+static tree
+component_size (tree ref)
+{
+  gcc_assert (TREE_CODE (ref) == COMPONENT_REF);
+
+  tree member = TREE_OPERAND (ref, 1);
+
+  /* If the member is not last or has a size greater than one, return
+     it.  Otherwise it's either a flexible array member or a zero-length
+     array member, or an array of length one treated as such.  */
+  tree size = DECL_SIZE_UNIT (member);
+  if (size
+      && (!array_at_struct_end_p (ref)
+	  || (!integer_zerop (size)
+	      && !integer_onep (size))))
+    return size;
+
+  /* If the reference is to a declared object and the member a true
+     flexible array, try to determine its size from its initializer.  */
+  poly_int64 off = 0;
+  tree base = get_addr_base_and_unit_offset (ref, &off);
+  if (!base || !VAR_P (base))
+    return NULL_TREE;
+
+  /* The size of any member of a declared object other than a flexible
+     array member is that obtained above.  */
+  if (size)
+    return size;
+
+  if (tree init = DECL_INITIAL (base))
+    if (TREE_CODE (init) == CONSTRUCTOR)
+      {
+	off <<= LOG2_BITS_PER_UNIT;
+	init = fold_ctor_reference (NULL_TREE, init, off, 0, base);
+	if (init)
+	  return TYPE_SIZE_UNIT (TREE_TYPE (init));
+      }
+
+  return DECL_EXTERNAL (base) ? NULL_TREE : integer_zero_node;
+}
+
 /* Helper to compute the size of the object referenced by the DEST
    expression which must have pointer type, using Object Size type
    OSTYPE (only the least significant 2 bits are used).  Return
@@ -3568,12 +3617,18 @@ check_access (tree exp, tree, tree, tree dstwrite,
    the size cannot be determined.  When the referenced object involves
    a non-constant offset in some range the returned value represents
    the largest size given the smallest non-negative offset in the
-   range.  The function is intended for diagnostics and should not
-   be used to influence code generation or optimization.  */
+   range.  If nonnull, set *PDECL to the decl of the referenced
+   subobject if it can be determined, or to null otherwise.
+   The function is intended for diagnostics and should not be used
+   to influence code generation or optimization.  */
 
 tree
-compute_objsize (tree dest, int ostype)
+compute_objsize (tree dest, int ostype, tree *pdecl /* = NULL */)
 {
+  tree dummy = NULL_TREE;
+  if (!pdecl)
+    pdecl = &dummy;
+
   unsigned HOST_WIDE_INT size;
 
   /* Only the two least significant bits are meaningful.  */
@@ -3600,7 +3655,7 @@ compute_objsize (tree dest, int ostype)
 	  tree off = gimple_assign_rhs2 (stmt);
 	  if (TREE_CODE (off) == INTEGER_CST)
 	    {
-	      if (tree size = compute_objsize (dest, ostype))
+	      if (tree size = compute_objsize (dest, ostype, pdecl))
 		{
 		  wide_int wioff = wi::to_wide (off);
 		  wide_int wisiz = wi::to_wide (size);
@@ -3625,7 +3680,7 @@ compute_objsize (tree dest, int ostype)
 
 	      if (rng == VR_RANGE)
 		{
-		  if (tree size = compute_objsize (dest, ostype))
+		  if (tree size = compute_objsize (dest, ostype, pdecl))
 		    {
 		      wide_int wisiz = wi::to_wide (size);
 
@@ -3653,12 +3708,31 @@ compute_objsize (tree dest, int ostype)
   if (!ostype)
     return NULL_TREE;
 
-  if (TREE_CODE (dest) == MEM_REF)
+  if (TREE_CODE (dest) == ARRAY_REF
+      || TREE_CODE (dest) == MEM_REF)
     {
       tree ref = TREE_OPERAND (dest, 0);
       tree off = TREE_OPERAND (dest, 1);
-      if (tree size = compute_objsize (ref, ostype))
+      if (tree size = compute_objsize (ref, ostype, pdecl))
 	{
+	  /* If the declaration of the destination object is known
+	     to have zero size, return zero.  */
+	  if (integer_zerop (size))
+	    return integer_zero_node;
+
+	  if (TREE_CODE (off) != INTEGER_CST
+	      || TREE_CODE (size) != INTEGER_CST)
+	    return NULL_TREE;
+
+	  if (TREE_CODE (dest) == ARRAY_REF)
+	    {
+	      tree eltype = TREE_TYPE (dest);
+	      if (tree tpsize = TYPE_SIZE_UNIT (eltype))
+		off = fold_build2 (MULT_EXPR, size_type_node, off, tpsize);
+	      else
+		return NULL_TREE;
+	    }
+
 	  if (tree_int_cst_lt (off, size))
 	    return fold_build2 (MINUS_EXPR, size_type_node, size, off);
 	  return integer_zero_node;
@@ -3667,9 +3741,22 @@ compute_objsize (tree dest, int ostype)
       return NULL_TREE;
     }
 
+  if (TREE_CODE (dest) == COMPONENT_REF)
+    {
+      *pdecl = TREE_OPERAND (dest, 1);
+      return component_size (dest);
+    }
+
   if (TREE_CODE (dest) != ADDR_EXPR)
     return NULL_TREE;
 
+  tree ref = TREE_OPERAND (dest, 0);
+  if (DECL_P (ref))
+    {
+      *pdecl = ref;
+      return DECL_SIZE_UNIT (ref);
+    }
+
   tree type = TREE_TYPE (dest);
   if (TREE_CODE (type) == POINTER_TYPE)
     type = TREE_TYPE (type);
@@ -3677,14 +3764,10 @@ compute_objsize (tree dest, int ostype)
   type = TYPE_MAIN_VARIANT (type);
 
   if (TREE_CODE (type) == ARRAY_TYPE
-      && !array_at_struct_end_p (TREE_OPERAND (dest, 0)))
-    {
-      /* Return the constant size unless it's zero (that's a zero-length
-	 array likely at the end of a struct).  */
-      tree size = TYPE_SIZE_UNIT (type);
-      if (size && TREE_CODE (size) == INTEGER_CST
-	  && !integer_zerop (size))
-	return size;
+      && !array_at_struct_end_p (ref))
+    {
+      if (tree size = TYPE_SIZE_UNIT (type))
+	return TREE_CODE (size) == INTEGER_CST ? size : NULL_TREE;
     }
 
   return NULL_TREE;
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 66c9295ff4a..1ad82e86963 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -134,7 +134,7 @@ extern tree fold_call_stmt (gcall *, bool);
 extern void set_builtin_user_assembler_name (tree decl, const char *asmspec);
 extern bool is_simple_builtin (tree);
 extern bool is_inexpensive_builtin (tree);
-extern tree compute_objsize (tree, int);
+extern tree compute_objsize (tree, int, tree * = NULL);
 
 extern bool readonly_data_expr (tree exp);
 extern bool init_target_chars (void);
diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
new file mode 100644
index 00000000000..d1aab4805e9
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
@@ -0,0 +1,348 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-array-bounds" } */
+
+void sink (void*);
+
+// Exercise flexible array members.
+
+struct Ax
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+struct Ax ax_;
+
+void gax_ (void)
+{
+  ax_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[1] = 1;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+struct Ax ax0 = { 0 };
+
+void gax0 (void)
+{
+  ax0.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[1] = 1;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+struct Ax ax0_ = { 0, { } };
+
+void gax0_ (void)
+{
+  ax0_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[1] = 1;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[2] = 2;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for out-of-bounds accesses to a definition with
+// an initializer.
+struct Ax ax1 = { 1, { 0 } };
+
+void gax1 (void)
+{
+  ax1.a[0] = 0;
+  ax1.a[1] = 1;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax1.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+struct Ax ax2 = { 2, { 1, 0 } };
+
+void gax2 (void)
+{
+  ax2.a[0] = 0;
+  ax2.a[1] = 1;
+  ax2.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void gaxp (struct Ax *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 3;
+  p->a[9] = 9;
+}
+
+
+// Verify no warning for an extern struct object whose array may be
+// initialized to any number of elements.
+extern struct Ax axx;
+
+void gaxx (void)
+{
+  axx.a[0] = 0;
+  axx.a[3] = 3;
+  axx.a[9] = 9;
+}
+
+// Exercise zero-length array members.
+
+struct A0
+{
+  char n;
+  char a[0];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+struct A0 a0_;
+
+void ga0_ (void)
+{
+  a0_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[1] = 1;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+struct A0 a00 = { 0 };
+
+void ga00 (void)
+{
+  a00.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[1] = 1;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+struct A0 a00_ = { 0, { } };
+
+void ga00_ (void)
+{
+  a00_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[1] = 1;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[2] = 2;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// The following are rejected with
+//   error: too many initializers for 'char [0]'
+// A0 a01 = { 1, { 0 } };
+// A0 a02 = { 2, { 1, 0 } };
+
+
+// Verify no warning for an unknown struct object.
+void ga0p (struct A0 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 3;
+  p->a[9] = 9;
+}
+
+
+// Verify warning for an extern struct object which (unlike a true
+// flexible array member) may not be initialized.
+extern struct A0 a0x;
+
+void ga0x (void)
+{
+  a0x.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Exercise trailing one-element array members.
+
+struct A1
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+struct A1 a1_;
+
+void ga1_ (void)
+{
+  a1_.a[0] = 0;
+  a1_.a[1] = 1;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a;
+  a.a[0] = 0;
+  a.a[1] = 1;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+struct A1 a1__ = { 0 };
+
+void ga1__ (void)
+{
+  a1__.a[0] = 0;
+  a1__.a[1] = 1;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1__.a[2] = 2;                 // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 1 };
+  a.a[0] = 0;
+  a.a[1] = 1;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 2;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+struct A1 a1_0 = { 0, { } };
+
+void ga1_0_ (void)
+{
+  a1_0.a[0] = 0;
+  a1_0.a[1] = 1;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_0.a[2] = 2;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 1, { } };
+  a.a[0] = 0;
+  a.a[1] = 1;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+struct A1 a1_1 = { 0, { 1 } };
+
+void ga1_1 (void)
+{
+  a1_1.a[0] = 0;
+  a1_1.a[1] = 1;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_1.a[2] = 2;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 0, { 1 } };
+  a.a[0] = 0;
+  a.a[1] = 1;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1p (struct A1 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 3;
+  p->a[9] = 9;
+}
+
+
+// Verify warning for an extern struct object.  Similar to the zero-length
+// array case, a one-element trailing array can be initialized to at most
+// a single element.
+extern struct A1 a1x;
+
+void ga1x (void)
+{
+  a1x.a[0] = 0;
+  a1x.a[3] = 3;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1x.a[9] = 9;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Exercise interior one-element array members (verify they're not
+// treated as trailing.
+
+struct A1i
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+  char x;
+};
+
+// Verify warning for a definition with no initializer.
+struct A1i a1i_;
+
+void ga1i_ (void)
+{
+  a1i_.a[0] = 0;
+  a1i_.a[1] = 1;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_.a[2] = 2;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1i a;
+  a.a[0] = 1;
+  a.a[1] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 3;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+struct A1i a1i__ = { 0 };
+
+void ga1i__ (void)
+{
+  a1i__.a[0] = 0;
+  a1i__.a[1] = 1;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i__.a[2] = 2;                // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1i a = { 0 };
+  a.a[0] = 0;
+  a.a[1] = 1;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 2;                    // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+struct A1 a1i_0 = { 0, { } };
+
+void ga1i_0_ (void)
+{
+  a1i_0.a[0] = 0;
+  a1i_0.a[1] = 1;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_0.a[2] = 2;               // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 0, { } };
+  a.a[0] = 0;
+  a.a[1] = 1;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+struct A1 a1i_1 = { 0, { 1 } };
+
+void ga1i_1 (void)
+{
+  a1i_1.a[0] = 0;
+  a1i_1.a[1] = 1;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_1.a[2] = 2;               // { dg-warning "\\\[-Wstringop-overflow" }
+
+  struct A1 a = { 0, { 1 } };
+  a.a[0] = 1;
+  a.a[1] = 2;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  a.a[2] = 3;                   // { dg-warning "\\\[-Wstringop-overflow" }
+  sink (&a);
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1ip (struct A1i *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 3;                  // { dg-warning "\\\[-Wstringop-overflow" }
+  p->a[9] = 9;                  // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an extern struct object.
+extern struct A1i a1ix;
+
+void ga1ix (void)
+{
+  a1ix.a[0] = 0;
+  a1ix.a[3] = 3;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1ix.a[9] = 9;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-8.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-8.C
new file mode 100644
index 00000000000..850414ede4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-8.C
@@ -0,0 +1,388 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   See Wstringop-overflow-3.C for the same test that exercises the other
+   warning.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-stringop-overflow" } */
+
+void sink (void*);
+
+// Exercise flexible array members.
+
+struct Ax
+{
+  char n;
+  char a[];                     // { dg-message "while referencing .Ax::a." "pr91463" { xfail *-*-* } }
+};
+
+// Verify warning for a definition with no initializer.
+Ax ax_;
+
+void gax_ ()
+{
+  ax_.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax_.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax_.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+Ax ax0 = { 0 };
+
+void gax0 ()
+{
+  ax0.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+Ax ax0_ = { 0, { } };
+
+void gax0_ ()
+{
+  ax0_.a[0] = 0;                // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0_.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax0_.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+// Verify warning for out-of-bounds accesses to a definition with
+// an initializer.
+Ax ax1 = { 1, { 0 } };
+
+void gax1 ()
+{
+  ax1.a[0] = 0;
+  ax1.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+  ax1.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+Ax ax2 = { 2, { 1, 0 } };
+
+void gax2 ()
+{
+  ax2.a[0] = 0;
+  ax2.a[1] = 0;
+  ax2.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+}
+
+
+// Verify no warning for an unknown struct object.
+void gaxp (Ax *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify no warning for an extern struct object whose array may be
+// initialized to any number of elements.
+extern Ax axx;
+
+void gaxx ()
+{
+  axx.a[0] = 0;
+  axx.a[3] = 0;
+  axx.a[9] = 0;
+}
+
+// Exercise zero-length array members.
+
+struct A0
+{
+  char n;
+  char a[0];                    // { dg-message "while referencing .A0::a." }
+};
+
+// Verify warning for a definition with no initializer.
+A0 a0_;
+
+void ga0_ ()
+{
+  a0_.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0_.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0_.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+A0 a00 = { 0 };
+
+void ga00 ()
+{
+  a00.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a00.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a00.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+A0 a00_ = { 0, { } };
+
+void ga00_ ()
+{
+  a00_.a[0] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a00_.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a00_.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// The following are rejected with
+//   error: too many initializers for 'char [0]'
+// A0 a01 = { 1, { 0 } };
+// A0 a02 = { 2, { 1, 0 } };
+
+
+// Verify no warning for an unknown struct object.
+void ga0p (A0 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object which (unlike a true
+// flexible array member) may not be initialized.
+extern A0 a0x;
+
+void ga0x ()
+{
+  a0x.a[0] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0x.a[3] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a0x.a[9] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Exercise trailing one-element array members.
+
+struct A1
+{
+  char n;
+  char a[1];                    // { dg-message "while referencing .A1::a." }
+};
+
+// Verify warning for a definition with no initializer.
+A1 a1_;
+
+void ga1_ ()
+{
+  a1_.a[0] = 0;
+  a1_.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1_.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1 a1__ = { 0 };
+
+void ga1__ ()
+{
+  a1__.a[0] = 0;
+  a1__.a[1] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1__.a[2] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1_0 = { 0, { } };
+
+void ga1_0_ ()
+{
+  a1_0.a[0] = 0;
+  a1_0.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1_0.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1_1 = { 0, { 1 } };
+
+void ga1_1 ()
+{
+  a1_1.a[0] = 0;
+  a1_1.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1_1.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1p (A1 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object.  Similar to the zero-length
+// array case, a one-element trailing array can be initialized to at most
+// a single element.
+extern A1 a1x;
+
+void ga1x ()
+{
+  a1x.a[0] = 0;
+  a1x.a[3] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1x.a[9] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Exercise interior one-element array members (verify they're not
+// treated as trailing.
+
+struct A1i
+{
+  char n;
+  char a[1];                    // { dg-message "while referencing .A1i::a." }
+  char x;
+};
+
+// Verify warning for a definition with no initializer.
+A1i a1i_;
+
+void ga1i_ ()
+{
+  a1i_.a[0] = 0;
+  a1i_.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1i_.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1i a1i__ = { 0 };
+
+void ga1i__ ()
+{
+  a1i__.a[0] = 0;
+  a1i__.a[1] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+  a1i__.a[2] = 0;                // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1i_0 = { 0, { } };
+
+void ga1i_0_ ()
+{
+  a1i_0.a[0] = 0;
+  a1i_0.a[1] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+  a1i_0.a[2] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1i_1 = { 0, { 1 } };
+
+void ga1i_1 ()
+{
+  a1i_1.a[0] = 0;
+  a1i_1.a[1] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+  a1i_1.a[2] = 0;               // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1ip (A1i *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;                  // { dg-warning "\\\[-Warray-bounds" }
+  p->a[9] = 0;                  // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify no warning for an extern struct object.
+extern A1i a1ix;
+
+void ga1ix ()
+{
+  a1ix.a[0] = 0;
+  a1ix.a[3] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+  a1ix.a[9] = 0;                 // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+// Verify non-POD classes with flexible array members.
+
+struct Bx
+{
+  char n;
+  char a[];                     // { dg-message "while referencing .Bx::a." "pr91463" { xfail *-*-* } }
+
+  // Verify the warning for a constant.
+  Bx () { a[0] = 0; }           // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+
+  // And also for a non-constant.  Regardless of the subscript, the array
+  // of the object in function gxi() below has a zero size.
+  Bx (int i) { a[i] = 0; }      // { dg-warning "\\\[-Warray-bounds" "pr91463" { xfail *-*-* } }
+};
+
+void gbx (void)
+{
+  struct Bx bx;
+  sink (&bx);
+}
+
+void gbxi (int i)
+{
+  struct Bx bxi (i);
+  sink (&bxi);
+}
+
+struct B0
+{
+  char n;
+  char a[0];                    // { dg-message "while referencing .B0::a." }
+
+  B0 () { a[0] = 0; }           // { dg-warning "\\\[-Warray-bounds" }
+};
+
+
+void gb0 (void)
+{
+  struct B0 b0;
+  sink (&b0);
+}
+
+
+struct B1
+{
+  char n;
+  char a[1];                    // { dg-message "while referencing .B1::a." }
+
+  B1 () { a[1] = 0; }           // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void gb1 (void)
+{
+  struct B1 b1;
+  sink (&b1);
+}
+
+
+struct B123
+{
+  char a[123];                  // { dg-message "while referencing .B123::a." }
+
+  B123 () { a[123] = 0; }       // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void gb123 (void)
+{
+  struct B123 b123;
+  sink (&b123);
+}
+
+
+struct B234
+{
+  char a[234];                  // { dg-message "while referencing .B234::a." }
+
+  B234 (int i) { a[i] = 0; }    // { dg-warning "\\\[-Warray-bounds" }
+};
+
+void g234 (void)
+{
+  struct B234 b234 (234);
+  sink (&b234);
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C
new file mode 100644
index 00000000000..99ce427c1b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C
@@ -0,0 +1,386 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-array-bounds" } */
+
+void sink (void*);
+
+// Exercise flexible array members.
+
+struct Ax
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+Ax ax_;
+
+void gax_ ()
+{
+  ax_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+Ax ax0 = { 0 };
+
+void gax0 ()
+{
+  ax0.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+Ax ax0_ = { 0, { } };
+
+void gax0_ ()
+{
+  ax0_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  ax0_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for out-of-bounds accesses to a definition with
+// an initializer.
+Ax ax1 = { 1, { 0 } };
+
+void gax1 ()
+{
+  ax1.a[0] = 0;
+  ax1.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  ax1.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+Ax ax2 = { 2, { 1, 0 } };
+
+void gax2 ()
+{
+  ax2.a[0] = 0;
+  ax2.a[1] = 0;
+  ax2.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void gaxp (Ax *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify no warning for an extern struct object whose array may be
+// initialized to any number of elements.
+extern Ax axx;
+
+void gaxx ()
+{
+  axx.a[0] = 0;
+  axx.a[3] = 0;
+  axx.a[9] = 0;
+}
+
+// Exercise zero-length array members.
+
+struct A0
+{
+  char n;
+  char a[0];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+A0 a0_;
+
+void ga0_ ()
+{
+  a0_.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the flexible array member.
+A0 a00 = { 0 };
+
+void ga00 ()
+{
+  a00.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a00.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the flexible array member to empty.
+A0 a00_ = { 0, { } };
+
+void ga00_ ()
+{
+  a00_.a[0] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a00_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// The following are rejected with
+//   error: too many initializers for 'char [0]'
+// A0 a01 = { 1, { 0 } };
+// A0 a02 = { 2, { 1, 0 } };
+
+
+// Verify no warning for an unknown struct object.
+void ga0p (A0 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object which (unlike a true
+// flexible array member) may not be initialized.
+extern A0 a0x;
+
+void ga0x ()
+{
+  a0x.a[0] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a0x.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Exercise trailing one-element array members.
+
+struct A1
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+};
+
+// Verify warning for a definition with no initializer.
+A1 a1_;
+
+void ga1_ ()
+{
+  a1_.a[0] = 0;
+  a1_.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1 a1__ = { 0 };
+
+void ga1__ ()
+{
+  a1__.a[0] = 0;
+  a1__.a[1] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1__.a[2] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1_0 = { 0, { } };
+
+void ga1_0_ ()
+{
+  a1_0.a[0] = 0;
+  a1_0.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_0.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1_1 = { 0, { 1 } };
+
+void ga1_1 ()
+{
+  a1_1.a[0] = 0;
+  a1_1.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1_1.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1p (A1 *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;
+  p->a[9] = 0;
+}
+
+
+// Verify warning for an extern struct object.  Similar to the zero-length
+// array case, a one-element trailing array can be initialized to at most
+// a single element.
+extern A1 a1x;
+
+void ga1x ()
+{
+  a1x.a[0] = 0;
+  a1x.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1x.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Exercise interior one-element array members (verify they're not
+// treated as trailing.
+
+struct A1i
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+  char x;
+};
+
+// Verify warning for a definition with no initializer.
+A1i a1i_;
+
+void ga1i_ ()
+{
+  a1i_.a[0] = 0;
+  a1i_.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that doesn't
+// initialize the one-element array member.
+A1i a1i__ = { 0 };
+
+void ga1i__ ()
+{
+  a1i__.a[0] = 0;
+  a1i__.a[1] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i__.a[2] = 0;                // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member to empty.
+A1 a1i_0 = { 0, { } };
+
+void ga1i_0_ ()
+{
+  a1i_0.a[0] = 0;
+  a1i_0.a[1] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_0.a[2] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+// Verify warning for access to a definition with an initializer that
+// initializes the one-element array member.
+A1 a1i_1 = { 0, { 1 } };
+
+void ga1i_1 ()
+{
+  a1i_1.a[0] = 0;
+  a1i_1.a[1] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+  a1i_1.a[2] = 0;               // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an unknown struct object.
+void ga1ip (A1i *p)
+{
+  p->a[0] = 0;
+  p->a[3] = 0;                  // { dg-warning "\\\[-Wstringop-overflow" }
+  p->a[9] = 0;                  // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify no warning for an extern struct object.
+extern A1i a1ix;
+
+void ga1ix ()
+{
+  a1ix.a[0] = 0;
+  a1ix.a[3] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+  a1ix.a[9] = 0;                 // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify non-POD classes with flexible array members.
+
+struct Bx
+{
+  char n;
+  char a[];                     // { dg-message "destination object declared here" }
+
+  // Verify the warning for a constant.
+  Bx () { a[0] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+
+  // And also for a non-constant.  Regardless of the subscript, the array
+  // of the object in function gxi() below has a zero size.
+  Bx (int i) { a[i] = 0; }      // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void gbx (void)
+{
+  struct Bx bx;
+  sink (&bx);
+}
+
+void gbxi (int i)
+{
+  struct Bx bxi (i);
+  sink (&bxi);
+}
+
+struct B0
+{
+  char n;
+  char a[0];                    // { dg-message "destination object declared here" }
+
+  B0 () { a[0] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+
+void gb0 (void)
+{
+  struct B0 b0;
+  sink (&b0);
+}
+
+
+struct B1
+{
+  char n;
+  char a[1];                    // { dg-message "destination object declared here" }
+
+  B1 () { a[1] = 0; }           // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void gb1 (void)
+{
+  struct B1 b1;
+  sink (&b1);
+}
+
+
+struct B123
+{
+  char a[123];                  // { dg-message "destination object declared here" }
+
+  B123 () { a[123] = 0; }       // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void gb123 (void)
+{
+  struct B123 b123;
+  sink (&b123);
+}
+
+
+struct B234
+{
+  char a[234];                  // { dg-message "destination object declared here" }
+
+  B234 (int i) { a[i] = 0; }    // { dg-warning "\\\[-Wstringop-overflow" }
+};
+
+void g234 (void)
+{
+  struct B234 b234 (234);
+  sink (&b234);
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
new file mode 100644
index 00000000000..12f8f9d353b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
@@ -0,0 +1,62 @@
+/* PR middle-end/91458 - inconsistent warning for writing past the end
+   of an array member
+   Verify that the -Wstringop-overflow detection doesn't cause an ICE
+   for either kind of VLAs (member and non-member).
+   Diagnosing the accesses is the subject of pr82608.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-array-bounds" } */
+
+void sink (void*);
+
+void vla_unbounded (int n)
+{
+  char a[n];
+
+  a[0] = 0;
+  a[1] = 1;
+  a[n] = n;         // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&a);
+}
+
+void vla_bounded (int n)
+{
+  if (n > 32)
+    n = 32;
+
+  char a[n];
+
+  a[0] = 0;
+  a[1] = 1;
+  a[n] = n;         // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+  a[69] = n;        // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&a);
+}
+
+
+void member_vla_unbounded (int n)
+{
+  struct S { char i, a[n]; } s;
+
+  s.a[0] = 0;
+  s.a[1] = 1;
+  s.a[n] = n;       // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&s);
+}
+
+void member_vla_bounded (int n)
+{
+  if (n > 32)
+    n = 32;
+
+  struct S { char i, a[n]; } s;
+
+  s.a[0] = 0;
+  s.a[1] = 1;
+  s.a[n] = n;       // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+  s.a[69] = n;      // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+
+  sink (&s);
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index d38352a0c4c..7bb5f52c1e5 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -4026,16 +4026,30 @@ handle_store (gimple_stmt_iterator *gsi)
       rhs_minlen = lenrange[0];
       storing_nonzero_p = lenrange[1] > 0;
 
-      if (tree dstsize = compute_objsize (lhs, 1))
-	if (compare_tree_int (dstsize, lenrange[2]) < 0)
-	  {
-	    location_t loc = gimple_nonartificial_location (stmt);
-	    warning_n (loc, OPT_Wstringop_overflow_,
-		       lenrange[2],
-		       "%Gwriting %u byte into a region of size %E",
-		       "%Gwriting %u bytes into a region of size %E",
-		       stmt, lenrange[2], dstsize);
-	  }
+      /* Avoid issuing multiple warnings for the same LHS or statement.
+	 For example, -Warray-bounds may have already been issued for
+	 an out-of-bounds subscript.  */
+      if (!TREE_NO_WARNING (lhs) && !gimple_no_warning_p (stmt))
+	{
+	  /* Set to the declaration referenced by LHS (if known).  */
+	  tree decl = NULL_TREE;
+	  if (tree dstsize = compute_objsize (lhs, 1, &decl))
+	    if (compare_tree_int (dstsize, lenrange[2]) < 0)
+	      {
+		location_t loc = gimple_nonartificial_location (stmt);
+		if (warning_n (loc, OPT_Wstringop_overflow_,
+			       lenrange[2],
+			       "%Gwriting %u byte into a region of size %E",
+			       "%Gwriting %u bytes into a region of size %E",
+			       stmt, lenrange[2], dstsize))
+		  {
+		    if (decl)
+		      inform (DECL_SOURCE_LOCATION (decl),
+			      "destination object declared here");
+		    gimple_set_no_warning (stmt, true);
+		  }
+	      }
+	}
     }
   else
     {
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 5ec4d17f23b..c95b5ade781 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -4388,8 +4388,8 @@ class vrp_prop : public ssa_propagation_engine
   void vrp_initialize (void);
   void vrp_finalize (bool);
   void check_all_array_refs (void);
-  void check_array_ref (location_t, tree, bool);
-  void check_mem_ref (location_t, tree, bool);
+  bool check_array_ref (location_t, tree, bool);
+  bool check_mem_ref (location_t, tree, bool);
   void search_for_addr_array (tree, location_t);
 
   class vr_values vr_values;
@@ -4415,9 +4415,10 @@ class vrp_prop : public ssa_propagation_engine
    array subscript is a constant, check if it is outside valid
    range. If the array subscript is a RANGE, warn if it is
    non-overlapping with valid range.
-   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
+   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.
+   Returns true if a warning has been issued.  */
 
-void
+bool
 vrp_prop::check_array_ref (location_t location, tree ref,
 			   bool ignore_off_by_one)
 {
@@ -4426,7 +4427,7 @@ vrp_prop::check_array_ref (location_t location, tree ref,
   tree low_bound, up_bound, up_bound_p1;
 
   if (TREE_NO_WARNING (ref))
-    return;
+    return false;
 
   low_sub = up_sub = TREE_OPERAND (ref, 1);
   up_bound = array_ref_up_bound (ref);
@@ -4541,12 +4542,16 @@ vrp_prop::check_array_ref (location_t location, tree ref,
   if (warned)
     {
       ref = TREE_OPERAND (ref, 0);
+      if (TREE_CODE (ref) == COMPONENT_REF)
+	ref = TREE_OPERAND (ref, 1);
 
       if (DECL_P (ref))
 	inform (DECL_SOURCE_LOCATION (ref), "while referencing %qD", ref);
 
       TREE_NO_WARNING (ref) = 1;
     }
+
+  return warned;
 }
 
 /* Checks one MEM_REF in REF, located at LOCATION, for out-of-bounds
@@ -4556,14 +4561,15 @@ vrp_prop::check_array_ref (location_t location, tree ref,
    with valid range.
    IGNORE_OFF_BY_ONE is true if the MEM_REF is inside an ADDR_EXPR
    (used to allow one-past-the-end indices for code that takes
-   the address of the just-past-the-end element of an array).  */
+   the address of the just-past-the-end element of an array).
+   Returns true if a warning has been issued.  */
 
-void
+bool
 vrp_prop::check_mem_ref (location_t location, tree ref,
 			 bool ignore_off_by_one)
 {
   if (TREE_NO_WARNING (ref))
-    return;
+    return false;
 
   tree arg = TREE_OPERAND (ref, 0);
   /* The constant and variable offset of the reference.  */
@@ -4615,7 +4621,7 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
 	  continue;
 	}
       else
-	return;
+	return false;
 
       /* VAROFF should always be a SSA_NAME here (and not even
 	 INTEGER_CST) but there's no point in taking chances.  */
@@ -4677,10 +4683,10 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
       arg = TREE_OPERAND (arg, 0);
       if (TREE_CODE (arg) != STRING_CST
 	  && TREE_CODE (arg) != VAR_DECL)
-	return;
+	return false;
     }
   else
-    return;
+    return false;
 
   /* The type of the object being referred to.  It can be an array,
      string literal, or a non-array type when the MEM_REF represents
@@ -4695,7 +4701,7 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
       || !COMPLETE_TYPE_P (reftype)
       || TREE_CODE (TYPE_SIZE_UNIT (reftype)) != INTEGER_CST
       || RECORD_OR_UNION_TYPE_P (reftype))
-    return;
+    return false;
 
   offset_int eltsize;
   if (TREE_CODE (reftype) == ARRAY_TYPE)
@@ -4797,11 +4803,11 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
 
       if (warned)
 	TREE_NO_WARNING (ref) = 1;
-      return;
+      return warned;
     }
 
   if (warn_array_bounds < 2)
-    return;
+    return false;
 
   /* At level 2 check also intermediate offsets.  */
   int i = 0;
@@ -4812,8 +4818,13 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
       if (warning_at (location, OPT_Warray_bounds,
 		      "intermediate array offset %wi is outside array bounds "
 		      "of %qT", tmpidx, reftype))
-	TREE_NO_WARNING (ref) = 1;
+	{
+	  TREE_NO_WARNING (ref) = 1;
+	  return true;
+	}
     }
+
+  return false;
 }
 
 /* Searches if the expr T, located at LOCATION computes
@@ -4825,10 +4836,14 @@ vrp_prop::search_for_addr_array (tree t, location_t location)
   /* Check each ARRAY_REF and MEM_REF in the reference chain. */
   do
     {
+      bool warned = false;
       if (TREE_CODE (t) == ARRAY_REF)
-	check_array_ref (location, t, true /*ignore_off_by_one*/);
+	warned = check_array_ref (location, t, true /*ignore_off_by_one*/);
       else if (TREE_CODE (t) == MEM_REF)
-	check_mem_ref (location, t, true /*ignore_off_by_one*/);
+	warned = check_mem_ref (location, t, true /*ignore_off_by_one*/);
+
+      if (warned)
+	TREE_NO_WARNING (t) = true;
 
       t = TREE_OPERAND (t, 0);
     }
@@ -4920,16 +4935,20 @@ check_array_bounds (tree *tp, int *walk_subtree, void *data)
 
   *walk_subtree = TRUE;
 
+  bool warned = false;
   vrp_prop *vrp_prop = (class vrp_prop *)wi->info;
   if (TREE_CODE (t) == ARRAY_REF)
-    vrp_prop->check_array_ref (location, t, false /*ignore_off_by_one*/);
+    warned = vrp_prop->check_array_ref (location, t, false/*ignore_off_by_one*/);
   else if (TREE_CODE (t) == MEM_REF)
-    vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/);
+    warned = vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/);
   else if (TREE_CODE (t) == ADDR_EXPR)
     {
       vrp_prop->search_for_addr_array (t, location);
       *walk_subtree = FALSE;
     }
+  /* Propagate the no-warning bit to the outer expression.  */
+  if (warned)
+    TREE_NO_WARNING (t) = true;
 
   return NULL_TREE;
 }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] issue consistent warning for past-the-end array stores (PR 91457)
  2019-08-28 18:33         ` Martin Sebor
@ 2019-08-28 18:43           ` Jeff Law
  0 siblings, 0 replies; 10+ messages in thread
From: Jeff Law @ 2019-08-28 18:43 UTC (permalink / raw)
  To: Martin Sebor, Richard Biener; +Cc: gcc-patches

On 8/28/19 9:44 AM, Martin Sebor wrote:
> With the PR 91490 subset committed, attached is what's left
> of the original fix.  It seems simple enough that I decided it's
> not worth chopping up any further.  The meat of the change is in
> builtins.c.  Everything else just adjusts warnings either by
> making use of it (tree-ssa-strlen.c) or by setting the no-warning
> bit (tree-vrp.c).
> 
> Retested on x86_64-linux.
> 
> Martin
> 
> gcc-91457.diff
> 
> PR tree-optimization/91457 - inconsistent warning for writing past the end of an array member
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/91457
> 	* builtins.c (component_size): New function.
> 	(compute_objsize): Add argument. Handle ARRAY_REF and COMPONENT_REF.
> 	* builtins.h (compute_objsize): Add argument.
> 	* tree-ssa-strlen.c (handle_store): Handle no-warning bit.
> 	* tree-vrp.c (vrp_prop::check_array_ref): Return warning result.
> 	(vrp_prop::check_mem_ref): Same.
> 	(vrp_prop::search_for_addr_array): Set no-warning bit.
> 	(check_array_bounds): Same.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/91457
> 	* c-c++-common/Wstringop-overflow-2.c: New test.
> 	* g++.dg/warn/Warray-bounds-8.C: New test.
> 	* g++.dg/warn/Wstringop-overflow-3.C: New test.
> 	* gcc.dg/Wstringop-overflow-15.c: New test.
OK
jeff

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-08-28 16:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-16 23:36 [PATCH] issue consistent warning for past-the-end array stores (PR 91457) Martin Sebor
2019-08-19 15:08 ` Richard Biener
2019-08-20  8:31   ` Martin Sebor
2019-08-20  8:47     ` Richard Biener
2019-08-21 21:18       ` Martin Sebor
2019-08-22 11:03         ` Richard Biener
2019-08-22 19:59           ` Jason Merrill
2019-08-28 18:33         ` Martin Sebor
2019-08-28 18:43           ` Jeff Law
2019-08-22 18:50       ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).