ubsan: Honor -fstrict-flex-arrays= in -fsanitize=bounds [PR108894]

While this isn't really a regression, the -fstrict-flex-arrays*
option is new in GCC 13 and so I think we should make -fsanitize=bounds
play with it well from the beginning.

The current behavior is that -fsanitize=bounds considers all trailing
arrays as flexible member-like arrays and both -fsanitize=bounds and
-fsanitize=bounds-strict because of a bug don't even instrument
[0] arrays at all, not as trailing nor when followed by other members.

I think -fstrict-flex-arrays* options can be considered as language
mode changing options, by default flexible member-like arrays are
handled like flexible arrays, but that option can change the set of
the arrays which are treated like that.  So, -fsanitize=bounds should
change with that on what is considered acceptable and what isn't.
While -fsanitize=bounds-strict should reject them all always to
continue previous behavior.

The following patch implements that.  To support [0] array instrumentation,
I had to change the meaning of the bounds argument to .UBSAN_BOUNDS,
previously it was the TYPE_MAX_VALUE of the domain unless ignore_off_by_one
(used for taking address of the array element rather than accessing it;
in that case 1 is added to the bound argument) and the later lowered checks
were if (index > bound) report_failure ().
The problem with that is that for [0] arrays where (at least for C++)
the max value is all ones, for accesses that condition will be never true;
for addresses of elements it was working (in C++) correctly before.
This patch changes it to add 1 + ignore_off_by_one, so -1 becomes 0 or
1 for &array_ref and changing the lowering to be if (index >= bound)
report_failure ().  Furthermore, as C represents the [0] arrays with
NULL TYPE_MAX_VALUE, I treated those like the C++ ones.

2023-02-28  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/108894
gcc/
	* ubsan.cc (ubsan_expand_bounds_ifn): Emit index >= bound
	comparison rather than index > bound.
	* gimple-fold.cc (gimple_fold_call): Use tree_int_cst_lt
	rather than tree_int_cst_le for IFN_UBSAN_BOUND comparison.
	* doc/invoke.texi (-fsanitize=bounds): Document that whether
	flexible array member-like arrays are instrumented or not depends
	on -fstrict-flex-arrays* options of strict_flex_array attributes.
	(-fsanitize=bounds-strict): Document that flexible array members
	are not instrumented.
gcc/c-family/
	* c-common.h (c_strict_flex_array_level_of): Declare.
	* c-common.cc (c_strict_flex_array_level_of): New function,
	moved and renamed from c-decl.cc's strict_flex_array_level_of.
	* c-ubsan.cc (ubsan_instrument_bounds): Fix comment typo.  For
	C check c_strict_flex_array_level_of whether a trailing array
	should be treated as flexible member like.  Handle C [0] arrays.
	Add 1 + index_off_by_one rather than index_off_by_one to bounds
	and use tree_int_cst_lt rather than tree_int_cst_le for idx vs.
	bounds comparison.
gcc/c/
	* c-decl.cc (strict_flex_array_level_of): Move to c-common.cc
	and rename to c_strict_flex_array_level_of.
	(is_flexible_array_member_p): Adjust caller.
gcc/testsuite/
	* gcc.dg/ubsan/bounds-4.c: New test.
	* gcc.dg/ubsan/bounds-4a.c: New test.
	* gcc.dg/ubsan/bounds-4b.c: New test.
	* gcc.dg/ubsan/bounds-4c.c: New test.
	* gcc.dg/ubsan/bounds-4d.c: New test.
	* g++.dg/ubsan/bounds-1.C: New test.

1 files changed, 1 insertions, 30 deletions

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 08078ea..9159965 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9050,35 +9050,6 @@ finish_incomplete_vars (tree incomplete_vars, bool toplevel)
     }
 }
 
-/* Get the LEVEL of the strict_flex_array for the ARRAY_FIELD based on the
-   values of attribute strict_flex_array and the flag_strict_flex_arrays.  */
-static unsigned int
-strict_flex_array_level_of (tree array_field)
-{
-  gcc_assert (TREE_CODE (array_field) == FIELD_DECL);
-  unsigned int strict_flex_array_level = flag_strict_flex_arrays;
-
-  tree attr_strict_flex_array
-    = lookup_attribute ("strict_flex_array", DECL_ATTRIBUTES (array_field));
-  /* If there is a strict_flex_array attribute attached to the field,
-     override the flag_strict_flex_arrays.  */
-  if (attr_strict_flex_array)
-    {
-      /* Get the value of the level first from the attribute.  */
-      unsigned HOST_WIDE_INT attr_strict_flex_array_level = 0;
-      gcc_assert (TREE_VALUE (attr_strict_flex_array) != NULL_TREE);
-      attr_strict_flex_array = TREE_VALUE (attr_strict_flex_array);
-      gcc_assert (TREE_VALUE (attr_strict_flex_array) != NULL_TREE);
-      attr_strict_flex_array = TREE_VALUE (attr_strict_flex_array);
-      gcc_assert (tree_fits_uhwi_p (attr_strict_flex_array));
-      attr_strict_flex_array_level = tree_to_uhwi (attr_strict_flex_array);
-
-      /* The attribute has higher priority than flag_struct_flex_array.  */
-      strict_flex_array_level = attr_strict_flex_array_level;
-    }
-  return strict_flex_array_level;
-}
-
 /* Determine whether the FIELD_DECL X is a flexible array member according to
    the following info:
   A. whether the FIELD_DECL X is the last field of the DECL_CONTEXT;
@@ -9105,7 +9076,7 @@ is_flexible_array_member_p (bool is_last_field,
   bool is_one_element_array = one_element_array_type_p (TREE_TYPE (x));
   bool is_flexible_array = flexible_array_member_type_p (TREE_TYPE (x));
 
-  unsigned int strict_flex_array_level = strict_flex_array_level_of (x);
+  unsigned int strict_flex_array_level = c_strict_flex_array_level_of (x);
 
   switch (strict_flex_array_level)
     {