[PATCH] Fix PR 100944 - Array boundary misscalculation

Giuliano Belinassi giuliano.belinassi@usp.br
Sun Jun 13 17:00:07 GMT 2021


This patch proposes a fix to PR 100944 by improving the array boundary
computation when the flexible array has no clear constructor: if no
constructor were found in the input code, we compute the size of the
array as:

  offset(array begin) - offset(next element in RECORD_TYPE)

If no next element is found in the RECORD, simply fall back to:

  offset(array begin) - offset(RECORD_TYPE ends)

We avoid doing this calculation if the RECORD_TYPE is actually an
UNION_TYPE, once things get very complicated in this case.

gcc/ChangeLog:
2021-13-08  Giuliano Belinassi  <giuliano.belinassi@usp.br>

	PR middle-end/100944
	* tree-dfa.c (get_ref_base_and_unit_offset): Add option to compute size
	of next field.
	* (get_ref_base_and_unit_offset_1): Same as above.
	* tree-dfa.h (get_ref_base_and_unit_offset): Same as above.
	(get_ref_base_and_unit_offset_1): Same as above.
	* tree.c (least_common_record_1): New.
	(least_common_record): New.
	(component_ref_size): Improve array size calculation.

gcc/testsuite/ChangeLog:
2021-13-08  Giuliano Belinassi  <giuliano.belinassi@usp.br>

	PR middle-end/100944
	* gcc.dg/Wzero-length-array-bounds.c: Update diagnostic.
	* gcc.dg/Warray-bounds-71.c: New test.
---
 gcc/testsuite/gcc.dg/Warray-bounds-71.c       |  42 +++++++
 .../gcc.dg/Wzero-length-array-bounds.c        |  18 +--
 gcc/tree-dfa.c                                |  31 ++++-
 gcc/tree-dfa.h                                |   6 +-
 gcc/tree.c                                    | 115 ++++++++++++++----
 5 files changed, 172 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Warray-bounds-71.c

diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-71.c b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
new file mode 100644
index 00000000000..cc5b083bc77
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
@@ -0,0 +1,42 @@
+/* PR middle-end/100944 - missing -Warray-bounds accessing a flexible array
+   member of a nested struct
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+struct A0
+{
+  int i, a[0];
+};
+
+struct B0
+{
+  struct A0 a;
+  long x;
+} b0;
+
+void f0 (int i)
+{
+  long t = b0.x;
+  b0.a.a[i] = 0;    // { dg-warning "\\\[-Warray-bounds" }
+  if (t != b0.x)    // folded to false
+    __builtin_abort ();
+}
+
+struct Ax
+{
+  int i, a[];
+};
+
+struct Bx
+{
+  struct Ax a;
+  long x;
+} bx;
+
+void fx (int i)
+{
+  long t = bx.x;
+  bx.a.a[i] = 0;    // { dg-warning "\\\[-Warray-bounds" }
+  if (t != bx.x)    // folded to false
+    __builtin_abort ();
+}
diff --git a/gcc/testsuite/gcc.dg/Wzero-length-array-bounds.c b/gcc/testsuite/gcc.dg/Wzero-length-array-bounds.c
index 8e880d92dea..117b30ff294 100644
--- a/gcc/testsuite/gcc.dg/Wzero-length-array-bounds.c
+++ b/gcc/testsuite/gcc.dg/Wzero-length-array-bounds.c
@@ -68,21 +68,21 @@ extern struct Y y;
 
 void access_to_member (int i)
 {
-  y.a[0].a[0] = 0;      // { dg-warning "\\\[-Wzero-length-bounds" }
-  y.a[0].a[1] = 0;      // { dg-warning "\\\[-Wzero-length-bounds" }
-  y.a[0].a[2] = 0;      // { dg-warning "\\\[-Wzero-length-bounds" }
+  y.a[0].a[0] = 0;      // { dg-warning "\\\[-Warray-bounds" }
+  y.a[0].a[1] = 0;      // { dg-warning "\\\[-Warray-bounds" }
+  y.a[0].a[2] = 0;      // { dg-warning "\\\[-Warray-bounds" }
   sink (a);
 
-  y.a[1].a[0] = 0;      // { dg-warning "\\\[-Wzero-length-bounds" }
-  y.a[1].a[1] = 0;      // { dg-warning "\\\[-Wzero-length-bounds" }
+  y.a[1].a[0] = 0;      // { dg-warning "\\\[-Warray-bounds" }
+  y.a[1].a[1] = 0;      // { dg-warning "\\\[-Warray-bounds" }
   /* Similar to the array case above, accesses to a subsequent member
      of the "parent" struct seem like a more severe problem than those
      to the next member of the same struct.  */
-  y.a[1].a[2] = 0;      // { dg-warning "\\\[-Wzero-length-bounds" }
+  y.a[1].a[2] = 0;      // { dg-warning "\\\[-Warray-bounds" }
   sink (a);
 
-  y.b.a[0] = 0;         // { dg-warning "\\\[-Wzero-length-bounds" }
-  y.b.a[1] = 0;         // { dg-warning "\\\[-Wzero-length-bounds" }
-  y.b.a[2] = 0;         // { dg-warning "\\\[-Wzero-length-bounds" }
+  y.b.a[0] = 0;         // { dg-warning "\\\[-Warray-bounds" }
+  y.b.a[1] = 0;         // { dg-warning "\\\[-Warray-bounds" }
+  y.b.a[2] = 0;         // { dg-warning "\\\[-Warray-bounds" }
   y.b.a[3] = 0;         // { dg-warning "\\\[-Warray-bounds" }
 }
diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c
index 1d20de0c400..dc3c15f11d5 100644
--- a/gcc/tree-dfa.c
+++ b/gcc/tree-dfa.c
@@ -772,9 +772,11 @@ get_ref_base_and_extent_hwi (tree exp, HOST_WIDE_INT *poffset,
 
 tree
 get_addr_base_and_unit_offset_1 (tree exp, poly_int64_pod *poffset,
-				 tree (*valueize) (tree))
+				 tree (*valueize) (tree),
+				 bool of_next_component /* = false.  */)
 {
   poly_int64 byte_offset = 0;
+  tree next_field = NULL_TREE;
 
   /* Compute cumulative byte-offset for nested component-refs and array-refs,
      and find the ultimate containing object.  */
@@ -797,11 +799,27 @@ get_addr_base_and_unit_offset_1 (tree exp, poly_int64_pod *poffset,
 	case COMPONENT_REF:
 	  {
 	    tree field = TREE_OPERAND (exp, 1);
-	    tree this_offset = component_ref_field_offset (exp);
+	    tree this_offset;
+
 	    poly_int64 hthis_offset;
 
+	    if (of_next_component && !next_field)
+	      {
+		/* We are looking for the next component of the record.  */
+		next_field = TREE_CHAIN (field);
+		if (!next_field)
+		  break;
+
+		/* We found a next component.  Flag that we found it and
+		   update the target field.  */
+		field = next_field;
+	      }
+
+	    this_offset = component_ref_field_offset (exp);
+
 	    if (!this_offset
 		|| !poly_int_tree_p (this_offset, &hthis_offset)
+		|| TREE_CODE (field) != FIELD_DECL
 		|| (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
 		    % BITS_PER_UNIT))
 	      return NULL_TREE;
@@ -904,6 +922,9 @@ get_addr_base_and_unit_offset_1 (tree exp, poly_int64_pod *poffset,
 done:
 
   *poffset = byte_offset;
+
+  if (of_next_component)
+    return next_field;
   return exp;
 }
 
@@ -913,9 +934,11 @@ done:
    is not BITS_PER_UNIT-aligned.  */
 
 tree
-get_addr_base_and_unit_offset (tree exp, poly_int64_pod *poffset)
+get_addr_base_and_unit_offset (tree exp, poly_int64_pod *poffset, bool
+			       of_next_component /* = false.  */)
 {
-  return get_addr_base_and_unit_offset_1 (exp, poffset, NULL);
+  return get_addr_base_and_unit_offset_1 (exp, poffset, NULL,
+					  of_next_component);
 }
 
 /* Returns true if STMT references an SSA_NAME that has
diff --git a/gcc/tree-dfa.h b/gcc/tree-dfa.h
index b1457ab065c..94e44d9c3f6 100644
--- a/gcc/tree-dfa.h
+++ b/gcc/tree-dfa.h
@@ -34,8 +34,10 @@ extern tree get_ref_base_and_extent (tree, poly_int64_pod *, poly_int64_pod *,
 extern tree get_ref_base_and_extent_hwi (tree, HOST_WIDE_INT *,
 					 HOST_WIDE_INT *, bool *);
 extern tree get_addr_base_and_unit_offset_1 (tree, poly_int64_pod *,
-					     tree (*) (tree));
-extern tree get_addr_base_and_unit_offset (tree, poly_int64_pod *);
+					     tree (*) (tree),
+					     bool of_next_component = false);
+extern tree get_addr_base_and_unit_offset (tree, poly_int64_pod *,
+					   bool = false);
 extern bool stmt_references_abnormal_ssa_name (gimple *);
 extern void replace_abnormal_ssa_names (gimple *);
 extern void dump_enumerated_decls (FILE *, dump_flags_t);
diff --git a/gcc/tree.c b/gcc/tree.c
index 1aa6e557a04..45d7fa2ae92 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -12649,6 +12649,47 @@ get_initializer_for (tree init, tree decl)
   return NULL_TREE;
 }
 
+static int
+least_common_record_1 (tree basetype, tree field1, tree field2,
+		       tree *least_basetype)
+{
+  int ret = 0;
+
+  for (tree fld = TYPE_FIELDS (basetype); fld; fld = TREE_CHAIN (fld))
+    {
+      if (fld == field1 || fld == field2)
+	ret++;
+
+      if (TREE_CODE (TREE_TYPE (fld)) == UNION_TYPE
+	  || TREE_CODE (TREE_TYPE (fld)) == RECORD_TYPE)
+	ret += least_common_record_1 (TREE_TYPE (fld), field1, field2,
+				      least_basetype);
+    }
+
+  if (ret == 2)
+    {
+      *least_basetype = basetype;
+      ret++; /* Avoid getting in this block again if a common basetype were
+		found.  */
+    }
+
+  return ret;
+}
+
+/* Find the least common RECORD type common to two FIELDS from base.  */
+static tree
+least_common_record (tree basetype, tree field1, tree field2)
+{
+  if (!(TREE_CODE (basetype) == RECORD_TYPE
+	|| TREE_CODE (basetype) == UNION_TYPE))
+    return NULL_TREE;
+
+  tree least_basetype = NULL_TREE;
+  least_common_record_1 (basetype, field1, field2, &least_basetype);
+
+  return least_basetype;
+}
+
 /* 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.
@@ -12768,28 +12809,30 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
   memsize = NULL_TREE;
 
   if (typematch)
-    /* MEMBER is a true flexible array member.  Compute its size from
-       the initializer of the BASE object if it has one.  */
-    if (tree init = DECL_P (base) ? DECL_INITIAL (base) : NULL_TREE)
-      if (init != error_mark_node)
-	{
-	  init = get_initializer_for (init, member);
-	  if (init)
-	    {
-	      memsize = TYPE_SIZE_UNIT (TREE_TYPE (init));
-	      if (tree refsize = TYPE_SIZE_UNIT (argtype))
-		{
-		  /* Use the larger of the initializer size and the tail
-		     padding in the enclosing struct.  */
-		  poly_int64 rsz = tree_to_poly_int64 (refsize);
-		  rsz -= baseoff;
-		  if (known_lt (tree_to_poly_int64 (memsize), rsz))
-		    memsize = wide_int_to_tree (TREE_TYPE (memsize), rsz);
-		}
-
-	      baseoff = 0;
-	    }
-	}
+    {
+      /* MEMBER is a true flexible array member.  Compute its size from
+	 the initializer of the BASE object if it has one.  */
+      if (tree init = DECL_P (base) ? DECL_INITIAL (base) : NULL_TREE)
+	if (init != error_mark_node)
+	  {
+	    init = get_initializer_for (init, member);
+	    if (init)
+	      {
+		memsize = TYPE_SIZE_UNIT (TREE_TYPE (init));
+		if (tree refsize = TYPE_SIZE_UNIT (argtype))
+		  {
+		    /* Use the larger of the initializer size and the tail
+		       padding in the enclosing struct.  */
+		    poly_int64 rsz = tree_to_poly_int64 (refsize);
+		    rsz -= baseoff;
+		    if (known_lt (tree_to_poly_int64 (memsize), rsz))
+		      memsize = wide_int_to_tree (TREE_TYPE (memsize), rsz);
+		  }
+
+		baseoff = 0;
+	      }
+	  }
+    }
 
   if (!memsize)
     {
@@ -12810,9 +12853,31 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
 	  memsize = TYPE_SIZE_UNIT (bt);
 	}
       else if (DECL_P (base))
-	/* Use the size of the BASE object (possibly an array of some
-	   other type such as char used to store the struct).  */
-	memsize = DECL_SIZE_UNIT (base);
+	{
+	  /* It is possible that the flexible array has no constructor at all.
+	     In such cases, GCC will allocate the array in some weird way.
+	     Assume that the array size is the difference between the start
+	     address of the array and the next component, if it exists.  */
+
+	  tree lcr;
+
+	  poly_int64 baseoff2 = 0;
+	  tree next_field = get_addr_base_and_unit_offset (ref, &baseoff2,
+						      /* next_elmnt =  */ true);
+	  if (next_field
+	      && (lcr = least_common_record (basetype, member, next_field))
+	      && TREE_CODE (lcr) == RECORD_TYPE
+	      && known_ge (baseoff2, baseoff))
+	    {
+		poly_int64 size =  baseoff2 - baseoff;
+		memsize = wide_int_to_tree (TREE_TYPE (DECL_SIZE_UNIT (base)),
+					    size);
+	    }
+	  else
+	    /* Use the size of the BASE object (possibly an array of some
+	       other type such as char used to store the struct).  */
+	    memsize = DECL_SIZE_UNIT (base);
+	}
       else
 	return NULL_TREE;
     }
-- 
2.32.0



More information about the Gcc-patches mailing list