public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
@ 2024-03-29 16:06 Qing Zhao
  2024-03-29 16:06 ` [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896) Qing Zhao
                   ` (6 more replies)
  0 siblings, 7 replies; 34+ messages in thread
From: Qing Zhao @ 2024-03-29 16:06 UTC (permalink / raw)
  To: josmyers, richard.guenther, siddhesh, uecker
  Cc: keescook, isanbard, gcc-patches, Qing Zhao

Hi,

This is the 8th version of the patch.

compare with the 7th version, the difference are:

updates per Joseph's comments:

1. Wording changes in diagnostics;
   "non flexible" to "non-flexible";
   Diagnostics starts with a lowercase letter;
2. Documentation changes:
   "named ``@var{count}'' to ``@var{count}'';
   use present tense in the documentation;
3. Checking "INTEGRAL_TYPE_P" instead of just INTEGER_TYPE for integer types.
   Add testcases for _Bool/enum/_BitInt count fields. 
4. Add handling for multiple counted_by attributes on the same field:
   Allow duplicates if they name the same field;
   Error when they name different fields.
   Add testcase for this.
5. Updates for comments style.


The 7th version is at:
https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648087.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648088.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648089.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648090.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648091.html

It based on the following original proposal:

https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635884.html
Represent the missing dependence for the "counted_by" attribute and its consumers

******The summary of the proposal is:

* Add a new internal function ".ACCESS_WITH_SIZE" to carry the size information for every reference to a FAM field;
* In C FE, Replace every reference to a FAM field whose TYPE has the "counted_by" attribute with the new internal function ".ACCESS_WITH_SIZE";
* In every consumer of the size information, for example, BDOS or array bound sanitizer, query the size information or ACCESS_MODE information from the new internal function;
* When expansing to RTL, replace the internal function with the actual reference to the FAM field;
* Some adjustment to ipa alias analysis, and other SSA passes to mitigate the impact to the optimizer and code generation.


******The new internal function

  .ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, TYPE_OF_SIZE, ACCESS_MODE, TYPE_OF_REF)

INTERNAL_FN (ACCESS_WITH_SIZE, ECF_LEAF | ECF_NOTHROW, NULL)

which returns the "REF_TO_OBJ" same as the 1st argument;

Both the return type and the type of the first argument of this function have been converted from the incomplete array type to the corresponding pointer type.

The call to .ACCESS_WITH_SIZE is wrapped with an INDIRECT_REF, whose type is the original imcomplete array type.

Please see the following link for why:
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638793.html
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639605.html

1st argument "REF_TO_OBJ": The reference to the object;
2nd argument "REF_TO_SIZE": The reference to the size of the object,
3rd argument "CLASS_OF_SIZE": The size referenced by the REF_TO_SIZE represents
   0: the number of bytes;
   1: the number of the elements of the object type;
4th argument "TYPE_OF_SIZE": A constant 0 with the TYPE of the object
  refed by REF_TO_SIZE
5th argument "ACCESS_MODE":
  -1: Unknown access semantics
   0: none
   1: read_only
   2: write_only
   3: read_write
6th argument "TYPE_OF_REF": A constant 0 with the pointer TYPE to
  to the original flexible array type.

****** The Patch sets included:

1. Provide counted_by attribute to flexible array member field;
      which includes:
      * "counted_by" attribute documentation;
      * C FE handling of the new attribute;
        syntax checking, error reporting;
      * testing cases;

2. Convert "counted_by" attribute to/from .ACCESS_WITH_SIZE.
      which includes:
      * The definition of the new internal function .ACCESS_WITH_SIZE in internal-fn.def.
      * C FE converts every reference to a FAM with "counted_by" attribute to a call to the internal function .ACCESS_WITH_SIZE.
        (build_component_ref in c_typeck.cc)
        This includes the case when the object is statically allocated and initialized.
        In order to make this working, we should update initializer_constant_valid_p_1 and output_constant in varasm.cc to include calls to .ACCESS_WITH_SIZE.

        However, for the reference inside "offsetof", ignore the "counted_by" attribute since it's not useful at all. (c_parser_postfix_expression in c/c-parser.cc)
	In addtion to "offsetof", for the reference inside operator "typeof" and
  "alignof", we ignore counted_by attribute too.
  	When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
  replace the call with its first argument.

      * Convert every call to .ACCESS_WITH_SIZE to its first argument.
        (expand_ACCESS_WITH_SIZE in internal-fn.cc)
      * adjust alias analysis to exclude the new internal from clobbering anything.
        (ref_maybe_used_by_call_p_1 and call_may_clobber_ref_p_1 in tree-ssa-alias.cc)
      * adjust dead code elimination to eliminate the call to .ACCESS_WITH_SIZE when
        it's LHS is eliminated as dead code.
        (eliminate_unnecessary_stmts in tree-ssa-dce.cc)
      * Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
        get the reference from the call to .ACCESS_WITH_SIZE.
        (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)
      * testing cases. (for offsetof, static initialization, generation of calls to
        .ACCESS_WITH_SIZE, code runs correctly after calls to .ACCESS_WITH_SIZE are
        converted back)

3. Use the .ACCESS_WITH_SIZE in builtin object size (sub-object only)
      which includes:
      * use the size info of the .ACCESS_WITH_SIZE for sub-object.
      * when the size is a negative integer, treat it as zero.
      * testing cases. 

4 Use the .ACCESS_WITH_SIZE in bound sanitizer
      which includes:
      * Instrument array_ref with a call to .ACCESS_WITH_SIZE for bound sanitizer.
      * when the size is a negative integer, treat it as zero.
      * testing cases. 

5. Add the 6th argument to .ACCESS_WITH_SIZE to carry the TYPE of the flexible array.
      which includes:
      * Add the 6th argument to .ACCESS_WITH_SIZE.
      * use the type of the 6th argument of the routine in tree-object-size.cc
      * testing case.

******Remaining works: 

6  Improve __bdos to use the counted_by info in whole-object size for the structure with FAM.
7  Emit warnings when the user breaks the requirments for the new counted_by attribute
   compilation time: -Wcounted-by
   run time: -fsanitizer=counted-by
      * The initialization to the size field should be done before the first reference to the FAM field.
      * the array has at least # of elements specified by the size field all the time during the program.

I have bootstrapped and regression tested on both x86 and aarch64, no issue.

Let me know your comments.

thanks.

Qing

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

* [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
@ 2024-03-29 16:06 ` Qing Zhao
  2024-04-10 17:35   ` Joseph Myers
  2024-03-29 16:07 ` [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE Qing Zhao
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 34+ messages in thread
From: Qing Zhao @ 2024-03-29 16:06 UTC (permalink / raw)
  To: josmyers, richard.guenther, siddhesh, uecker
  Cc: keescook, isanbard, gcc-patches, Qing Zhao

'counted_by (COUNT)'
     The 'counted_by' attribute may be attached to the C99 flexible
     array member of a structure.  It indicates that the number of the
     elements of the array is given by the field "COUNT" in the
     same structure as the flexible array member.
     GCC may use this information to improve detection of object size information
     for such structures and provide better results in compile-time diagnostics
     and runtime features like the array bound sanitizer and
     the '__builtin_dynamic_object_size'.

     For instance, the following code:

          struct P {
            size_t count;
            char other;
            char array[] __attribute__ ((counted_by (count)));
          } *p;

     specifies that the 'array' is a flexible array member whose number
     of elements is given by the field 'count' in the same structure.

     The field that represents the number of the elements should have an
     integer type.  Otherwise, the compiler reports an error and
     ignores the attribute.

     When the field that represents the number of the elements is assigned a
     negative integer value, the compiler treats the value as zero.

     An explicit 'counted_by' annotation defines a relationship between
     two objects, 'p->array' and 'p->count', and there are the following
     requirementthat on the relationship between this pair:

        * 'p->count' must be initialized before the first reference to
          'p->array';

        * 'p->array' has _at least_ 'p->count' number of elements
          available all the time.  This relationship must hold even
          after any of these related objects are updated during the
          program.

     It's the user's responsibility to make sure the above requirements
     to be kept all the time.  Otherwise the compiler reports
     warnings, at the same time, the results of the array bound
     sanitizer and the '__builtin_dynamic_object_size' is undefined.

     One important feature of the attribute is, a reference to the
     flexible array member field uses the latest value assigned to
     the field that represents the number of the elements before that
     reference.  For example,

            p->count = val1;
            p->array[20] = 0;  // ref1 to p->array
            p->count = val2;
            p->array[30] = 0;  // ref2 to p->array

     in the above, 'ref1' uses 'val1' as the number of the elements
     in 'p->array', and 'ref2' uses 'val2' as the number of elements
     in 'p->array'.

gcc/c-family/ChangeLog:

	PR C/108896
	* c-attribs.cc (handle_counted_by_attribute): New function.
	(attribute_takes_identifier_p): Add counted_by attribute to the list.
	* c-common.cc (c_flexible_array_member_type_p): ...To this.
	* c-common.h (c_flexible_array_member_type_p): New prototype.

gcc/c/ChangeLog:

	PR C/108896
	* c-decl.cc (flexible_array_member_type_p): Renamed and moved to...
	(add_flexible_array_elts_to_size): Use renamed function.
	(is_flexible_array_member_p): Use renamed function.
	(verify_counted_by_attribute): New function.
	(finish_struct): Use renamed function and verify counted_by
	attribute.
	* c-tree.h (lookup_field): New prototype.
	* c-typeck.cc (lookup_field): Expose as extern function.

gcc/ChangeLog:

	PR C/108896
	* doc/extend.texi: Document attribute counted_by.

gcc/testsuite/ChangeLog:

	PR C/108896
	* gcc.dg/flex-array-counted-by.c: New test.
	* gcc.dg/flex-array-counted-by-7.c: New test.
---
 gcc/c-family/c-attribs.cc                     | 68 +++++++++++++++-
 gcc/c-family/c-common.cc                      | 13 ++++
 gcc/c-family/c-common.h                       |  1 +
 gcc/c/c-decl.cc                               | 78 +++++++++++++++----
 gcc/c/c-tree.h                                |  1 +
 gcc/c/c-typeck.cc                             |  3 +-
 gcc/doc/extend.texi                           | 68 ++++++++++++++++
 .../gcc.dg/flex-array-counted-by-7.c          |  8 ++
 gcc/testsuite/gcc.dg/flex-array-counted-by.c  | 62 +++++++++++++++
 9 files changed, 282 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-7.c
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by.c

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 40a0cf90295d..39e5824ee7a5 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -105,6 +105,8 @@ static tree handle_warn_if_not_aligned_attribute (tree *, tree, tree,
 						  int, bool *);
 static tree handle_strict_flex_array_attribute (tree *, tree, tree,
 						 int, bool *);
+static tree handle_counted_by_attribute (tree *, tree, tree,
+					   int, bool *);
 static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, bool *);
@@ -412,6 +414,8 @@ const struct attribute_spec c_common_gnu_attributes[] =
 			      handle_warn_if_not_aligned_attribute, NULL },
   { "strict_flex_array",      1, 1, true, false, false, false,
 			      handle_strict_flex_array_attribute, NULL },
+  { "counted_by",	      1, 1, true, false, false, false,
+			      handle_counted_by_attribute, NULL },
   { "weak",                   0, 0, true,  false, false, false,
 			      handle_weak_attribute, NULL },
   { "noplt",                   0, 0, true,  false, false, false,
@@ -659,7 +663,8 @@ attribute_takes_identifier_p (const_tree attr_id)
   else if (!strcmp ("mode", spec->name)
 	   || !strcmp ("format", spec->name)
 	   || !strcmp ("cleanup", spec->name)
-	   || !strcmp ("access", spec->name))
+	   || !strcmp ("access", spec->name)
+	   || !strcmp ("counted_by", spec->name))
     return true;
   else
     return targetm.attribute_takes_identifier_p (attr_id);
@@ -2806,6 +2811,67 @@ handle_strict_flex_array_attribute (tree *node, tree name,
   return NULL_TREE;
 }
 
+/* Handle a "counted_by" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_counted_by_attribute (tree *node, tree name,
+			     tree args, int ARG_UNUSED (flags),
+			     bool *no_add_attrs)
+{
+  tree decl = *node;
+  tree argval = TREE_VALUE (args);
+  tree old_counted_by = lookup_attribute ("counted_by", DECL_ATTRIBUTES (decl));
+
+  /* This attribute only applies to field decls of a structure.  */
+  if (TREE_CODE (decl) != FIELD_DECL)
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"%qE attribute is not allowed for a non-field"
+		" declaration %q+D", name, decl);
+      *no_add_attrs = true;
+    }
+  /* This attribute only applies to field with array type.  */
+  else if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE)
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"%qE attribute is not allowed for a non-array field",
+		name);
+      *no_add_attrs = true;
+    }
+  /* This attribute only applies to a C99 flexible array member type.  */
+  else if (! c_flexible_array_member_type_p (TREE_TYPE (decl)))
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"%qE attribute is not allowed for a non-flexible"
+		" array member field", name);
+      *no_add_attrs = true;
+    }
+  /* The argument should be an identifier.  */
+  else if (TREE_CODE (argval) != IDENTIFIER_NODE)
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"%<counted_by%> argument is not an identifier");
+      *no_add_attrs = true;
+    }
+  /* Issue error when there is a counted_by attribute with a different
+     field as the argument for the same flexible array member field.  */
+  else if (old_counted_by != NULL_TREE)
+    {
+      tree old_fieldname = TREE_VALUE (TREE_VALUE (old_counted_by));
+      if (strcmp (IDENTIFIER_POINTER (old_fieldname),
+		  IDENTIFIER_POINTER (argval)) != 0)
+	{
+	  error_at (DECL_SOURCE_LOCATION (decl),
+		    "%<counted_by%> argument %qE conflicts with"
+		    " previous declaration %qE", argval, old_fieldname);
+	  *no_add_attrs = true;
+	}
+    }
+
+  return NULL_TREE;
+}
+
 /* Handle a "weak" attribute; arguments as in
    struct attribute_spec.handler.  */
 
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index e15eff698dfd..bc53a5292f37 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -9909,6 +9909,19 @@ c_common_finalize_early_debug (void)
       (*debug_hooks->early_global_decl) (cnode->decl);
 }
 
+/* Determine whether TYPE is an ISO C99 flexible array member type "[]".  */
+bool
+c_flexible_array_member_type_p (const_tree type)
+{
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && TYPE_SIZE (type) == NULL_TREE
+      && TYPE_DOMAIN (type) != NULL_TREE
+      && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE)
+    return true;
+
+  return false;
+}
+
 /* 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.  */
 unsigned int
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 2d5f53998855..3e0eed0548b0 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -904,6 +904,7 @@ extern tree fold_for_warn (tree);
 extern tree c_common_get_narrower (tree, int *);
 extern bool get_attribute_operand (tree, unsigned HOST_WIDE_INT *);
 extern void c_common_finalize_early_debug (void);
+extern bool c_flexible_array_member_type_p (const_tree);
 extern unsigned int c_strict_flex_array_level_of (tree);
 extern bool c_option_is_from_cpp_diagnostics (int);
 extern tree c_hardbool_type_attr_1 (tree, tree *, tree *);
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index fe20bc21c926..3dc21e5ee9ce 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5301,19 +5301,6 @@ set_array_declarator_inner (struct c_declarator *decl,
   return decl;
 }
 
-/* Determine whether TYPE is a ISO C99 flexible array memeber type "[]".  */
-static bool
-flexible_array_member_type_p (const_tree type)
-{
-  if (TREE_CODE (type) == ARRAY_TYPE
-      && TYPE_SIZE (type) == NULL_TREE
-      && TYPE_DOMAIN (type) != NULL_TREE
-      && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE)
-    return true;
-
-  return false;
-}
-
 /* Determine whether TYPE is a one-element array type "[1]".  */
 static bool
 one_element_array_type_p (const_tree type)
@@ -5350,7 +5337,7 @@ add_flexible_array_elts_to_size (tree decl, tree init)
 
   elt = CONSTRUCTOR_ELTS (init)->last ().value;
   type = TREE_TYPE (elt);
-  if (flexible_array_member_type_p (type))
+  if (c_flexible_array_member_type_p (type))
     {
       complete_array_type (&type, elt, false);
       DECL_SIZE (decl)
@@ -9317,7 +9304,7 @@ is_flexible_array_member_p (bool is_last_field,
 
   bool is_zero_length_array = zero_length_array_type_p (TREE_TYPE (x));
   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));
+  bool is_flexible_array = c_flexible_array_member_type_p (TREE_TYPE (x));
 
   unsigned int strict_flex_array_level = c_strict_flex_array_level_of (x);
 
@@ -9347,6 +9334,53 @@ is_flexible_array_member_p (bool is_last_field,
   return false;
 }
 
+/* Verify the argument of the counted_by attribute of the flexible array
+   member FIELD_DECL is a valid field of the containing structure,
+   STRUCT_TYPE, Report error and remove this attribute when it's not.  */
+static void
+verify_counted_by_attribute (tree struct_type, tree field_decl)
+{
+  tree attr_counted_by = lookup_attribute ("counted_by",
+					   DECL_ATTRIBUTES (field_decl));
+
+  if (!attr_counted_by)
+    return;
+
+  /* If there is an counted_by attribute attached to the field,
+     verify it.  */
+
+  tree fieldname = TREE_VALUE (TREE_VALUE (attr_counted_by));
+
+  /* Verify the argument of the attrbute is a valid field of the
+     containing structure.  */
+
+  tree counted_by_field = lookup_field (struct_type, fieldname);
+
+  /* Error when the field is not found in the containing structure.  */
+  if (!counted_by_field)
+    error_at (DECL_SOURCE_LOCATION (field_decl),
+	      "argument %qE to the %qE attribute is not a field declaration"
+	      " in the same structure as %qD", fieldname,
+	      (get_attribute_name (attr_counted_by)),
+	      field_decl);
+
+  else
+  /* Error when the field is not with an integer type.  */
+    {
+      while (TREE_CHAIN (counted_by_field))
+	counted_by_field = TREE_CHAIN (counted_by_field);
+      tree real_field = TREE_VALUE (counted_by_field);
+
+      if (!INTEGRAL_TYPE_P (TREE_TYPE (real_field)))
+	error_at (DECL_SOURCE_LOCATION (field_decl),
+		  "argument %qE to the %qE attribute is not a field declaration"
+		  " with an integer type", fieldname,
+		  (get_attribute_name (attr_counted_by)));
+
+    }
+
+  return;
+}
 
 /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T.
    LOC is the location of the RECORD_TYPE or UNION_TYPE's definition.
@@ -9408,6 +9442,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
      until now.)  */
 
   bool saw_named_field = false;
+  tree counted_by_fam_field = NULL_TREE;
   for (x = fieldlist; x; x = DECL_CHAIN (x))
     {
       /* Whether this field is the last field of the structure or union.
@@ -9468,7 +9503,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 	DECL_PACKED (x) = 1;
 
       /* Detect flexible array member in an invalid context.  */
-      if (flexible_array_member_type_p (TREE_TYPE (x)))
+      if (c_flexible_array_member_type_p (TREE_TYPE (x)))
 	{
 	  if (TREE_CODE (t) == UNION_TYPE)
 	    {
@@ -9489,6 +9524,12 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 			"members");
 	      TREE_TYPE (x) = error_mark_node;
 	    }
+
+	  /* If there is a counted_by attribute attached to this field,
+	     record it here and do more verification later after the
+	     whole structure is complete.  */
+	  if (lookup_attribute ("counted_by", DECL_ATTRIBUTES (x)))
+	    counted_by_fam_field = x;
 	}
 
       if (pedantic && TREE_CODE (t) == RECORD_TYPE
@@ -9503,7 +9544,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 	 when x is an array and is the last field.  */
       if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
 	TYPE_INCLUDES_FLEXARRAY (t)
-	  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
+	  = is_last_field && c_flexible_array_member_type_p (TREE_TYPE (x));
       /* Recursively set TYPE_INCLUDES_FLEXARRAY for the context of x, t
 	 when x is an union or record and is the last field.  */
       else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
@@ -9758,6 +9799,9 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 	struct_parse_info->struct_types.safe_push (t);
      }
 
+  if (counted_by_fam_field)
+    verify_counted_by_attribute (t, counted_by_fam_field);
+
   return t;
 }
 
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 1fba9c8dae76..c7c23edc4840 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -776,6 +776,7 @@ extern struct c_expr convert_lvalue_to_rvalue (location_t, struct c_expr,
 extern tree decl_constant_value_1 (tree, bool);
 extern void mark_exp_read (tree);
 extern tree composite_type (tree, tree);
+extern tree lookup_field (tree, tree);
 extern tree build_component_ref (location_t, tree, tree, location_t,
 				 location_t);
 extern tree build_array_ref (location_t, tree, tree);
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index ddeab1e2a8a1..cead0a055068 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -101,7 +101,6 @@ static bool function_types_compatible_p (const_tree, const_tree,
 					 struct comptypes_data *);
 static bool type_lists_compatible_p (const_tree, const_tree,
 				     struct comptypes_data *);
-static tree lookup_field (tree, tree);
 static int convert_arguments (location_t, vec<location_t>, tree,
 			      vec<tree, va_gc> *, vec<tree, va_gc> *, tree,
 			      tree);
@@ -2375,7 +2374,7 @@ default_conversion (tree exp)
    the component is embedded within (nested) anonymous structures or
    unions, the list steps down the chain to the component.  */
 
-static tree
+tree
 lookup_field (tree type, tree component)
 {
   tree field;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 2b8ba1949bf1..2def553961ce 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7753,6 +7753,74 @@ align them on any target.
 The @code{aligned} attribute can also be used for functions
 (@pxref{Common Function Attributes}.)
 
+@cindex @code{counted_by} variable attribute
+@item counted_by (@var{count})
+The @code{counted_by} attribute may be attached to the C99 flexible array
+member of a structure.  It indicates that the number of the elements of the
+array is given by the field "@var{count}" in the same structure as the
+flexible array member.
+GCC may use this information to improve detection of object size information
+for such structures and provide better results in compile-time diagnostics
+and runtime features like the array bound sanitizer and
+the @code{__builtin_dynamic_object_size}.
+
+For instance, the following code:
+
+@smallexample
+struct P @{
+  size_t count;
+  char other;
+  char array[] __attribute__ ((counted_by (count)));
+@} *p;
+@end smallexample
+
+@noindent
+specifies that the @code{array} is a flexible array member whose number of
+elements is given by the field @code{count} in the same structure.
+
+The field that represents the number of the elements should have an
+integer type.  Otherwise, the compiler reports an error and ignores
+the attribute.
+
+When the field that represents the number of the elements is assigned a
+negative integer value, the compiler treats the value as zero.
+
+An explicit @code{counted_by} annotation defines a relationship between
+two objects, @code{p->array} and @code{p->count}, and there are the
+following requirementthat on the relationship between this pair:
+
+@itemize @bullet
+@item
+@code{p->count} must be initialized before the first reference to
+@code{p->array};
+
+@item
+@code{p->array} has @emph{at least} @code{p->count} number of elements
+available all the time.  This relationship must hold even after any of
+these related objects are updated during the program.
+@end itemize
+
+It's the user's responsibility to make sure the above requirements to
+be kept all the time.  Otherwise the compiler reports warnings,
+at the same time, the results of the array bound sanitizer and the
+@code{__builtin_dynamic_object_size} is undefined.
+
+One important feature of the attribute is, a reference to the flexible
+array member field uses the latest value assigned to the field that
+represents the number of the elements before that reference.  For example,
+
+@smallexample
+  p->count = val1;
+  p->array[20] = 0;  // ref1 to p->array
+  p->count = val2;
+  p->array[30] = 0;  // ref2 to p->array
+@end smallexample
+
+@noindent
+in the above, @code{ref1} uses @code{val1} as the number of the elements in
+@code{p->array}, and @code{ref2} uses @code{val2} as the number of elements
+in @code{p->array}.
+
 @cindex @code{alloc_size} variable attribute
 @item alloc_size (@var{position})
 @itemx alloc_size (@var{position-1}, @var{position-2})
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-7.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-7.c
new file mode 100644
index 000000000000..fcb6f1b79690
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-7.c
@@ -0,0 +1,8 @@
+/* Testing the correct usage of attribute counted_by: _BitInt  */   
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-O2 -std=c23" } */
+
+struct trailing_array {
+  _BitInt(24) count; 
+  int array[] __attribute ((counted_by (count)));
+};
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by.c b/gcc/testsuite/gcc.dg/flex-array-counted-by.c
new file mode 100644
index 000000000000..e8b54c2de1c0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by.c
@@ -0,0 +1,62 @@
+/* Testing the correct usage of attribute counted_by.  */   
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#include <wchar.h>
+
+int size;
+int x __attribute ((counted_by (size))); /* { dg-error "attribute is not allowed for a non-field declaration" } */
+
+struct trailing {
+  int count;
+  int field __attribute ((counted_by (count))); /* { dg-error "attribute is not allowed for a non-array field" } */
+};
+
+struct trailing_1 {
+  int count;
+  int array_1[0] __attribute ((counted_by (count))); /* { dg-error "attribute is not allowed for a non-flexible array member field" } */
+};
+
+int count;
+struct trailing_array_2 {
+  int count;
+  int array_2[] __attribute ((counted_by ("count"))); /* { dg-error "argument is not an identifier" } */
+};
+
+struct trailing_array_3 {
+  int other;
+  int array_3[] __attribute ((counted_by (L"count"))); /* { dg-error "argument is not an identifier" } */
+};
+
+struct trailing_array_4 {
+  int other;
+  int array_4[] __attribute ((counted_by (count))); /* { dg-error "attribute is not a field declaration in the same structure as" } */
+};
+
+int count;
+struct trailing_array_5 {
+  float count;
+  int array_5[] __attribute ((counted_by (count))); /* { dg-error "attribute is not a field declaration with an integer type" } */
+}; 
+
+struct trailing_array_6 {
+  int count;
+  int array_6[] __attribute ((counted_by (count))) __attribute ((counted_by (count)));
+}; 
+
+struct trailing_array_7 {
+  int count1;
+  int count2;
+  int array_7[] __attribute ((counted_by (count1))) __attribute ((counted_by (count2))); /* { dg-error "conflicts with previous declaration" } */
+}; 
+
+struct trailing_array_8 {
+  _Bool count;
+  int array_8[] __attribute ((counted_by (count)));
+}; 
+
+enum week {Mon, Tue, Wed};
+struct trailing_array_9 {
+  enum week days;
+  int array_9[] __attribute ((counted_by (days)));
+}; 
-- 
2.31.1


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

* [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE.
  2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
  2024-03-29 16:06 ` [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896) Qing Zhao
@ 2024-03-29 16:07 ` Qing Zhao
  2024-04-10 18:36   ` Joseph Myers
  2024-03-29 16:07 ` [PATCH v8 3/5] Use the .ACCESS_WITH_SIZE in builtin object size Qing Zhao
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 34+ messages in thread
From: Qing Zhao @ 2024-03-29 16:07 UTC (permalink / raw)
  To: josmyers, richard.guenther, siddhesh, uecker
  Cc: keescook, isanbard, gcc-patches, Qing Zhao

Including the following changes:
* The definition of the new internal function .ACCESS_WITH_SIZE
  in internal-fn.def.
* C FE converts every reference to a FAM with a "counted_by" attribute
  to a call to the internal function .ACCESS_WITH_SIZE.
  (build_component_ref in c_typeck.cc)

  This includes the case when the object is statically allocated and
  initialized.
  In order to make this working, the routines initializer_constant_valid_p_1
  and output_constant in varasm.cc are updated to handle calls to
  .ACCESS_WITH_SIZE.
  (initializer_constant_valid_p_1 and output_constant in varasm.c)

  However, for the reference inside "offsetof", the "counted_by" attribute is
  ignored since it's not useful at all.
  (c_parser_postfix_expression in c/c-parser.cc)

  In addtion to "offsetof", for the reference inside operator "typeof" and
  "alignof", we ignore counted_by attribute too.

  When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
  replace the call with its first argument.

* Convert every call to .ACCESS_WITH_SIZE to its first argument.
  (expand_ACCESS_WITH_SIZE in internal-fn.cc)
* Adjust alias analysis to exclude the new internal from clobbering anything.
  (ref_maybe_used_by_call_p_1 and call_may_clobber_ref_p_1 in tree-ssa-alias.cc)
* Adjust dead code elimination to eliminate the call to .ACCESS_WITH_SIZE when
  it's LHS is eliminated as dead code.
  (eliminate_unnecessary_stmts in tree-ssa-dce.cc)
* Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
  get the reference from the call to .ACCESS_WITH_SIZE.
  (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)

gcc/c/ChangeLog:

	* c-parser.cc (c_parser_postfix_expression): Ignore the counted-by
	attribute when build_component_ref inside offsetof operator.
	* c-tree.h (build_component_ref): Add one more parameter.
	* c-typeck.cc (build_counted_by_ref): New function.
	(build_access_with_size_for_counted_by): New function.
	(build_component_ref): Check the counted-by attribute and build
	call to .ACCESS_WITH_SIZE.
	(build_unary_op): When building ADDR_EXPR for
        .ACCESS_WITH_SIZE, use its first argument.
        (lvalue_p): Accept call to .ACCESS_WITH_SIZE.

gcc/ChangeLog:

	* internal-fn.cc (expand_ACCESS_WITH_SIZE): New function.
	* internal-fn.def (ACCESS_WITH_SIZE): New internal function.
	* tree-ssa-alias.cc (ref_maybe_used_by_call_p_1): Special case
	IFN_ACCESS_WITH_SIZE.
	(call_may_clobber_ref_p_1): Special case IFN_ACCESS_WITH_SIZE.
	* tree-ssa-dce.cc (eliminate_unnecessary_stmts): Eliminate the call
	to .ACCESS_WITH_SIZE when its LHS is dead.
	* tree.cc (process_call_operands): Adjust side effect for function
	.ACCESS_WITH_SIZE.
	(is_access_with_size_p): New function.
	(get_ref_from_access_with_size): New function.
	* tree.h (is_access_with_size_p): New prototype.
	(get_ref_from_access_with_size): New prototype.
	* varasm.cc (initializer_constant_valid_p_1): Handle call to
	.ACCESS_WITH_SIZE.
	(output_constant): Handle call to .ACCESS_WITH_SIZE.

gcc/testsuite/ChangeLog:

	* gcc.dg/flex-array-counted-by-2.c: New test.
---
 gcc/c/c-parser.cc                             |  10 +-
 gcc/c/c-tree.h                                |   2 +-
 gcc/c/c-typeck.cc                             | 128 +++++++++++++++++-
 gcc/internal-fn.cc                            |  35 +++++
 gcc/internal-fn.def                           |   4 +
 .../gcc.dg/flex-array-counted-by-2.c          | 112 +++++++++++++++
 gcc/tree-ssa-alias.cc                         |   2 +
 gcc/tree-ssa-dce.cc                           |   5 +-
 gcc/tree.cc                                   |  25 +++-
 gcc/tree.h                                    |   8 ++
 gcc/varasm.cc                                 |  10 ++
 11 files changed, 331 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-2.c

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index c31349dae2ff..a6ed5ac43bb1 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -10850,9 +10850,12 @@ c_parser_postfix_expression (c_parser *parser)
 	    if (c_parser_next_token_is (parser, CPP_NAME))
 	      {
 		c_token *comp_tok = c_parser_peek_token (parser);
+		/* Ignore the counted_by attribute for reference inside
+		   offsetof since the information is not useful at all.  */
 		offsetof_ref
 		  = build_component_ref (loc, offsetof_ref, comp_tok->value,
-					 comp_tok->location, UNKNOWN_LOCATION);
+					 comp_tok->location, UNKNOWN_LOCATION,
+					 false);
 		c_parser_consume_token (parser);
 		while (c_parser_next_token_is (parser, CPP_DOT)
 		       || c_parser_next_token_is (parser,
@@ -10879,11 +10882,14 @@ c_parser_postfix_expression (c_parser *parser)
 			    break;
 			  }
 			c_token *comp_tok = c_parser_peek_token (parser);
+			/* Ignore the counted_by attribute for reference inside
+			   offsetof since the information is not useful.  */
 			offsetof_ref
 			  = build_component_ref (loc, offsetof_ref,
 						 comp_tok->value,
 						 comp_tok->location,
-						 UNKNOWN_LOCATION);
+						 UNKNOWN_LOCATION,
+						 false);
 			c_parser_consume_token (parser);
 		      }
 		    else
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index c7c23edc4840..59b69ca54f0f 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -778,7 +778,7 @@ extern void mark_exp_read (tree);
 extern tree composite_type (tree, tree);
 extern tree lookup_field (tree, tree);
 extern tree build_component_ref (location_t, tree, tree, location_t,
-				 location_t);
+				 location_t, bool = true);
 extern tree build_array_ref (location_t, tree, tree);
 extern tree build_omp_array_section (location_t, tree, tree, tree);
 extern tree build_external_ref (location_t, tree, bool, tree *);
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index cead0a055068..f7b0e08459b0 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -2546,15 +2546,116 @@ should_suggest_deref_p (tree datum_type)
     return false;
 }
 
+/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
+   the object that represents its counted_by per the attribute counted_by
+   attached to this field if it's a flexible array member field, otherwise
+   return NULL_TREE.
+   set COUNTED_BY_TYPE to the TYPE of the counted_by field.
+   For example, if:
+
+    struct P {
+      int k;
+      int x[] __attribute__ ((counted_by (k)));
+    } *p;
+
+    for:
+    p->x
+
+    the ref to the object that represents its element count will be:
+
+    &(p->k)
+
+*/
+static tree
+build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
+{
+  tree type = TREE_TYPE (datum);
+  if (!(c_flexible_array_member_type_p (TREE_TYPE (subdatum))))
+    return NULL_TREE;
+
+  tree attr_counted_by = lookup_attribute ("counted_by",
+					   DECL_ATTRIBUTES (subdatum));
+  tree counted_by_ref = NULL_TREE;
+  *counted_by_type = NULL_TREE;
+  if (attr_counted_by)
+    {
+      tree field_id = TREE_VALUE (TREE_VALUE (attr_counted_by));
+      counted_by_ref
+	= build_component_ref (UNKNOWN_LOCATION,
+			       datum, field_id,
+			       UNKNOWN_LOCATION, UNKNOWN_LOCATION);
+      counted_by_ref = build_fold_addr_expr (counted_by_ref);
+
+      /* Get the TYPE of the counted_by field.  */
+      tree counted_by_field = lookup_field (type, field_id);
+      gcc_assert (counted_by_field);
+
+      do
+	{
+	  *counted_by_type = TREE_TYPE (TREE_VALUE (counted_by_field));
+	  counted_by_field = TREE_CHAIN (counted_by_field);
+	}
+      while (counted_by_field);
+    }
+  return counted_by_ref;
+}
+
+/* Given a COMPONENT_REF REF with the location LOC, the corresponding
+   COUNTED_BY_REF, and the COUNTED_BY_TYPE, generate an INDIRECT_REF
+   to a call to the internal function .ACCESS_WITH_SIZE.
+
+   REF
+
+   to:
+
+   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
+
+   NOTE: The return type of this function is the POINTER type pointing
+   to the original flexible array type.
+   Then the type of the INDIRECT_REF is the original flexible array type.
+
+   The type of the first argument of this function is a POINTER type
+   to the original flexible array type.
+
+   The 4th argument of the call is a constant 0 with the TYPE of the
+   object pointed by COUNTED_BY_REF.
+
+  */
+static tree
+build_access_with_size_for_counted_by (location_t loc, tree ref,
+				       tree counted_by_ref,
+				       tree counted_by_type)
+{
+  gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
+  /* The result type of the call is a pointer to the flexible array type.  */
+  tree result_type = build_pointer_type (TREE_TYPE (ref));
+
+  tree call
+    = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
+				    result_type, 5,
+				    array_to_pointer_conversion (loc, ref),
+				    counted_by_ref,
+				    build_int_cst (integer_type_node, 1),
+				    build_int_cst (counted_by_type, 0),
+				    build_int_cst (integer_type_node, -1));
+  /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
+  call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
+  SET_EXPR_LOCATION (call, loc);
+  return call;
+}
+
 /* Make an expression to refer to the COMPONENT field of structure or
    union value DATUM.  COMPONENT is an IDENTIFIER_NODE.  LOC is the
    location of the COMPONENT_REF.  COMPONENT_LOC is the location
    of COMPONENT.  ARROW_LOC is the location of the first -> operand if
-   it is from -> operator.  */
+   it is from -> operator.
+   If HANDLE_COUNTED_BY is true, check the counted_by attribute and generate
+   a call to .ACCESS_WITH_SIZE.  Otherwise, ignore the attribute.  */
 
 tree
 build_component_ref (location_t loc, tree datum, tree component,
-		     location_t component_loc, location_t arrow_loc)
+		     location_t component_loc, location_t arrow_loc,
+		     bool handle_counted_by)
 {
   tree type = TREE_TYPE (datum);
   enum tree_code code = TREE_CODE (type);
@@ -2626,7 +2727,13 @@ build_component_ref (location_t loc, tree datum, tree component,
 	  int quals;
 	  tree subtype;
 	  bool use_datum_quals;
-
+	  tree counted_by_type = NULL_TREE;
+	  /* Do not handle counted_by when in typeof and alignof operator.  */
+	  handle_counted_by = handle_counted_by && !in_typeof && !in_alignof;
+	  tree counted_by_ref = handle_counted_by
+				? build_counted_by_ref (datum, subdatum,
+							&counted_by_type)
+				: NULL_TREE;
 	  if (TREE_TYPE (subdatum) == error_mark_node)
 	    return error_mark_node;
 
@@ -2645,6 +2752,12 @@ build_component_ref (location_t loc, tree datum, tree component,
 	  ref = build3 (COMPONENT_REF, subtype, datum, subdatum,
 			NULL_TREE);
 	  SET_EXPR_LOCATION (ref, loc);
+
+	  if (counted_by_ref)
+	    ref = build_access_with_size_for_counted_by (loc, ref,
+							 counted_by_ref,
+							 counted_by_type);
+
 	  if (TREE_READONLY (subdatum)
 	      || (use_datum_quals && TREE_READONLY (datum)))
 	    TREE_READONLY (ref) = 1;
@@ -5048,7 +5161,11 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
 	  goto return_build_unary_op;
 	}
 
-      /* Ordinary case; arg is a COMPONENT_REF or a decl.  */
+      /* Ordinary case; arg is a COMPONENT_REF or a decl, or a call to
+	 .ACCESS_WITH_SIZE.  */
+      if (is_access_with_size_p (arg))
+	arg = TREE_OPERAND (TREE_OPERAND (CALL_EXPR_ARG (arg, 0), 0), 0);
+
       argtype = TREE_TYPE (arg);
 
       /* If the lvalue is const or volatile, merge that into the type
@@ -5195,6 +5312,9 @@ lvalue_p (const_tree ref)
     case BIND_EXPR:
       return TREE_CODE (TREE_TYPE (ref)) == ARRAY_TYPE;
 
+    case CALL_EXPR:
+      return is_access_with_size_p (ref);
+
     default:
       return false;
     }
diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
index a07f25f3aee3..e744080ee670 100644
--- a/gcc/internal-fn.cc
+++ b/gcc/internal-fn.cc
@@ -3393,6 +3393,41 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
     }
 }
 
+/* Expand the IFN_ACCESS_WITH_SIZE function:
+   ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE,
+		     TYPE_OF_SIZE, ACCESS_MODE)
+   which returns the REF_TO_OBJ same as the 1st argument;
+
+   1st argument REF_TO_OBJ: The reference to the object;
+   2nd argument REF_TO_SIZE: The reference to the size of the object,
+   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
+     0: the number of bytes.
+     1: the number of the elements of the object type;
+   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
+    of the object referenced by REF_TO_SIZE
+   5th argument ACCESS_MODE:
+    -1: Unknown access semantics
+     0: none
+     1: read_only
+     2: write_only
+     3: read_write
+
+   Both the return type and the type of the first argument of this
+   function have been converted from the incomplete array type to
+   the corresponding pointer type.
+
+   For each call to a .ACCESS_WITH_SIZE, replace it with its 1st argument.  */
+static void
+expand_ACCESS_WITH_SIZE (internal_fn, gcall *stmt)
+{
+  tree lhs = gimple_call_lhs (stmt);
+  tree ref_to_obj = gimple_call_arg (stmt, 0);
+  if (lhs)
+    expand_assignment (lhs, ref_to_obj, false);
+  else
+    emit_insn (expand_normal (ref_to_obj));
+}
+
 /* The size of an OpenACC compute dimension.  */
 
 static void
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index c14d30365c14..0801c8bfe61d 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -510,6 +510,10 @@ DEF_INTERNAL_FN (PHI, 0, NULL)
    automatic variable.  */
 DEF_INTERNAL_FN (DEFERRED_INIT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 
+/* A function to associate the access size and access mode information
+   with the corresponding reference to an object.  */
+DEF_INTERNAL_FN (ACCESS_WITH_SIZE, ECF_LEAF | ECF_NOTHROW, NULL)
+
 /* DIM_SIZE and DIM_POS return the size of a particular compute
    dimension and the executing thread's position within that
    dimension.  DIM_POS is pure (and not const) so that it isn't
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
new file mode 100644
index 000000000000..d4899a63af3c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
@@ -0,0 +1,112 @@
+/* Test the code generation for the new attribute counted_by.
+   And also the offsetof operator on such array.  */
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-original" } */
+
+#include <stdlib.h>
+
+struct annotated {
+  int b;
+  char c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+static struct annotated static_annotated = { sizeof "hello", "hello" };
+static char *y = static_annotated.c;
+
+struct flex {
+  int b;
+  char c[]; 
+}; 
+
+struct nested_annotated {
+  struct {
+    union {
+      int b;
+      float f;	
+    };
+    int n;
+  };
+  char c[] __attribute__ ((counted_by (b)));
+} *array_nested_annotated;
+
+static struct nested_annotated nested_static_annotated
+				 = { sizeof "hello1", 0, "hello1" };
+static char *nested_y = nested_static_annotated.c;
+
+struct nested_flex {
+  struct {
+    union {
+      int b;
+      float f;	
+    };
+    int n;
+  };
+  char c[];
+};
+
+void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
+{
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated)
+				  + attr_count *  sizeof (char));
+  array_annotated->b = attr_count;
+
+  array_nested_annotated
+    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
+					 + attr_count *  sizeof (char));
+  array_nested_annotated->b = attr_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test (char a, char b)
+{
+  if (__builtin_offsetof (struct annotated, c[0])
+      != __builtin_offsetof (struct flex, c[0]))
+    abort ();
+  if (__builtin_offsetof (struct annotated, c[1])
+      != __builtin_offsetof (struct flex, c[1]))
+    abort ();
+  if (__builtin_offsetof (struct nested_annotated, c[0]) 
+      != __builtin_offsetof (struct nested_flex, c[0])) 
+    abort ();
+  if (__builtin_offsetof (struct nested_annotated, c[1]) 
+      != __builtin_offsetof (struct nested_flex, c[1])) 
+    abort ();
+
+  if (__builtin_types_compatible_p (typeof (array_annotated->c),
+				    typeof (&(array_annotated->c)[0])))
+    abort ();
+  if (__builtin_types_compatible_p (typeof (array_nested_annotated->c),
+				    typeof (&(array_nested_annotated->c)[0])))
+    abort ();
+
+  if (__alignof (array_annotated->c) != __alignof (char))
+    abort ();
+  if (__alignof (array_nested_annotated->c) != __alignof (char))
+    abort ();
+
+  if ((unsigned long) array_annotated->c != (unsigned long) &array_annotated->c)
+    abort ();
+  if ((unsigned long) array_nested_annotated->c
+       != (unsigned long) &array_nested_annotated->c)
+    abort ();
+
+  array_annotated->c[2] = a;
+  array_nested_annotated->c[3] = b;
+
+  if (y[2] != 'l') abort ();
+  if (nested_y[4] !='o') abort ();
+
+}
+
+int main(int argc, char *argv[])
+{
+  setup (10,10);   
+  test ('A', 'B');
+  if (array_annotated->c[2] != 'A') abort ();
+  if (array_nested_annotated->c[3] != 'B') abort ();
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "ACCESS_WITH_SIZE" 8 "original" } } */
diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
index e7c1c1aa6243..8c070e173bdd 100644
--- a/gcc/tree-ssa-alias.cc
+++ b/gcc/tree-ssa-alias.cc
@@ -2823,6 +2823,7 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *ref, bool tbaa_p)
 	return false;
       case IFN_MASK_STORE_LANES:
       case IFN_MASK_LEN_STORE_LANES:
+      case IFN_ACCESS_WITH_SIZE:
 	goto process_args;
       case IFN_MASK_LOAD:
       case IFN_LEN_LOAD:
@@ -3073,6 +3074,7 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref, bool tbaa_p)
       case IFN_UBSAN_OBJECT_SIZE:
       case IFN_UBSAN_PTR:
       case IFN_ASAN_CHECK:
+      case IFN_ACCESS_WITH_SIZE:
 	return false;
       case IFN_MASK_STORE:
       case IFN_LEN_STORE:
diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
index 636c471d4c89..a54fb1b754dd 100644
--- a/gcc/tree-ssa-dce.cc
+++ b/gcc/tree-ssa-dce.cc
@@ -1459,8 +1459,8 @@ eliminate_unnecessary_stmts (bool aggressive)
 		  update_stmt (stmt);
 		  release_ssa_name (name);
 
-		  /* GOMP_SIMD_LANE (unless three argument) or ASAN_POISON
-		     without lhs is not needed.  */
+		  /* GOMP_SIMD_LANE (unless three argument), ASAN_POISON
+		     or .ACCESS_WITH_SIZE without lhs is not needed.  */
 		  if (gimple_call_internal_p (stmt))
 		    switch (gimple_call_internal_fn (stmt))
 		      {
@@ -1470,6 +1470,7 @@ eliminate_unnecessary_stmts (bool aggressive)
 			  break;
 			/* FALLTHRU */
 		      case IFN_ASAN_POISON:
+		      case IFN_ACCESS_WITH_SIZE:
 			remove_dead_stmt (&gsi, bb, to_remove_edges);
 			break;
 		      default:
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 3dff8c510832..5fdb425f612a 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -4068,7 +4068,8 @@ process_call_operands (tree t)
   int i = call_expr_flags (t);
 
   /* Calls have side-effects, except those to const or pure functions.  */
-  if ((i & ECF_LOOPING_CONST_OR_PURE) || !(i & (ECF_CONST | ECF_PURE)))
+  if ((i & ECF_LOOPING_CONST_OR_PURE)
+      || (!(i & (ECF_CONST | ECF_PURE)) && !is_access_with_size_p (t)))
     side_effects = true;
   /* Propagate TREE_READONLY of arguments for const functions.  */
   if (i & ECF_CONST)
@@ -13362,6 +13363,28 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
 	  ? NULL_TREE : size_zero_node);
 }
 
+/* Return true if the given node CALL is a call to a .ACCESS_WITH_SIZE
+   function.  */
+bool
+is_access_with_size_p (const_tree call)
+{
+  if (TREE_CODE (call) != CALL_EXPR)
+    return false;
+  if (CALL_EXPR_IFN (call) == IFN_ACCESS_WITH_SIZE)
+    return true;
+  return false;
+}
+
+/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE.
+ * i.e the first argument of this call.  Return NULL_TREE otherwise.  */
+tree
+get_ref_from_access_with_size (tree call)
+{
+  if (is_access_with_size_p (call))
+    return  CALL_EXPR_ARG (call, 0);
+  return NULL_TREE;
+}
+
 /* Return the machine mode of T.  For vectors, returns the mode of the
    inner type.  The main use case is to feed the result to HONOR_NANS,
    avoiding the BLKmode that a direct TYPE_MODE (T) might return.  */
diff --git a/gcc/tree.h b/gcc/tree.h
index 972a067a1f7a..fbaef3e5fb5c 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5760,6 +5760,14 @@ extern special_array_member component_ref_sam_type (tree);
    cannot be determined.  */
 extern tree component_ref_size (tree, special_array_member * = NULL);
 
+/* Return true if the given node is a call to a .ACCESS_WITH_SIZE
+   function.  */
+extern bool is_access_with_size_p (const_tree);
+
+/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE,
+ * i.e. the first argument of this call.  Return NULL_TREE otherwise.  */
+extern tree get_ref_from_access_with_size (tree);
+
 extern int tree_map_base_eq (const void *, const void *);
 extern unsigned int tree_map_base_hash (const void *);
 extern bool tree_map_base_marked_p (const void *);
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index fa17eff551e8..d75b23668925 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -5082,6 +5082,11 @@ initializer_constant_valid_p_1 (tree value, tree endtype, tree *cache)
 	}
       return ret;
 
+    case CALL_EXPR:
+      /* For a call to .ACCESS_WITH_SIZE, check the first argument.  */
+      if (tree ref = get_ref_from_access_with_size (value))
+	return initializer_constant_valid_p_1 (ref, endtype, cache);
+      /* FALLTHROUGH.  */
     default:
       break;
     }
@@ -5276,6 +5281,11 @@ output_constant (tree exp, unsigned HOST_WIDE_INT size, unsigned int align,
 	exp = TREE_OPERAND (exp, 0);
     }
 
+  /* For a call to .ACCESS_WITH_SIZE, check the first argument.  */
+  if (TREE_CODE (exp) == CALL_EXPR)
+    if (tree ref = get_ref_from_access_with_size (exp))
+      exp = ref;
+
   code = TREE_CODE (TREE_TYPE (exp));
   thissize = int_size_in_bytes (TREE_TYPE (exp));
 
-- 
2.31.1


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

* [PATCH v8 3/5] Use the .ACCESS_WITH_SIZE in builtin object size.
  2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
  2024-03-29 16:06 ` [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896) Qing Zhao
  2024-03-29 16:07 ` [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE Qing Zhao
@ 2024-03-29 16:07 ` Qing Zhao
  2024-04-10 21:45   ` Siddhesh Poyarekar
  2024-03-29 16:07 ` [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer Qing Zhao
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 34+ messages in thread
From: Qing Zhao @ 2024-03-29 16:07 UTC (permalink / raw)
  To: josmyers, richard.guenther, siddhesh, uecker
  Cc: keescook, isanbard, gcc-patches, Qing Zhao

gcc/ChangeLog:

	* tree-object-size.cc (access_with_size_object_size): New function.
	(call_object_size): Call the new function.

gcc/testsuite/ChangeLog:

	* gcc.dg/builtin-object-size-common.h: Add a new macro EXPECT.
	* gcc.dg/flex-array-counted-by-3.c: New test.
	* gcc.dg/flex-array-counted-by-4.c: New test.
	* gcc.dg/flex-array-counted-by-5.c: New test.
---
 .../gcc.dg/builtin-object-size-common.h       |  11 ++
 .../gcc.dg/flex-array-counted-by-3.c          |  63 +++++++
 .../gcc.dg/flex-array-counted-by-4.c          | 178 ++++++++++++++++++
 .../gcc.dg/flex-array-counted-by-5.c          |  48 +++++
 gcc/tree-object-size.cc                       |  60 ++++++
 5 files changed, 360 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-5.c

diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-common.h b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
index 66ff7cdd953a..b677067c6e6b 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-common.h
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
@@ -30,3 +30,14 @@ unsigned nfails = 0;
       __builtin_abort ();						      \
     return 0;								      \
   } while (0)
+
+#define EXPECT(p, _v) do {						      \
+  size_t v = _v;							      \
+  if (p == v)								      \
+    __builtin_printf ("ok:  %s == %zd\n", #p, p);			      \
+  else									      \
+    {									      \
+      __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v);	      \
+      FAIL ();								      \
+    }									      \
+} while (0);
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
new file mode 100644
index 000000000000..78f50230e891
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
@@ -0,0 +1,63 @@
+/* Test the attribute counted_by and its usage in
+ * __builtin_dynamic_object_size.  */ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+struct flex {
+  int b;
+  int c[];
+} *array_flex;
+
+struct annotated {
+  int b;
+  int c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+struct nested_annotated {
+  struct {
+    union {
+      int b;
+      float f;	
+    };
+    int n;
+  };
+  int c[] __attribute__ ((counted_by (b)));
+} *array_nested_annotated;
+
+void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
+{
+  array_flex
+    = (struct flex *)malloc (sizeof (struct flex)
+			     + normal_count *  sizeof (int));
+  array_flex->b = normal_count;
+
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated)
+				  + attr_count *  sizeof (int));
+  array_annotated->b = attr_count;
+
+  array_nested_annotated
+    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
+					 + attr_count *  sizeof (int));
+  array_nested_annotated->b = attr_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test ()
+{
+    EXPECT(__builtin_dynamic_object_size(array_flex->c, 1), -1);
+    EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1),
+	   array_annotated->b * sizeof (int));
+    EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1),
+	   array_nested_annotated->b * sizeof (int));
+}
+
+int main(int argc, char *argv[])
+{
+  setup (10,10);   
+  test ();
+  DONE ();
+}
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
new file mode 100644
index 000000000000..20103d58ef51
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
@@ -0,0 +1,178 @@
+/* Test the attribute counted_by and its usage in
+__builtin_dynamic_object_size: what's the correct behavior when the
+allocation size mismatched with the value of counted_by attribute?
+We should always use the latest value that is hold by the counted_by
+field.  */
+/* { dg-do run } */
+/* { dg-options "-O -fstrict-flex-arrays=3" } */
+
+#include "builtin-object-size-common.h"
+
+struct annotated {
+  size_t foo;
+  char others;
+  char array[] __attribute__((counted_by (foo)));
+};
+
+#define noinline __attribute__((__noinline__))
+#define SIZE_BUMP 10 
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+
+/* In general, Due to type casting, the type for the pointee of a pointer
+   does not say anything about the object it points to,
+   So, __builtin_object_size can not directly use the type of the pointee
+   to decide the size of the object the pointer points to.
+
+   There are only two reliable ways:
+   A. observed allocations  (call to the allocation functions in the routine)
+   B. observed accesses     (read or write access to the location of the
+                             pointer points to)
+
+   That provide information about the type/existence of an object at
+   the corresponding address.
+
+   For A, we use the "alloc_size" attribute for the corresponding allocation
+   functions to determine the object size;
+   (We treat counted_by attribute the same as the "alloc_size" attribute)
+
+   For B, we use the SIZE info of the TYPE attached to the corresponding access.
+
+   The only other way in C which ensures that a pointer actually points
+   to an object of the correct type is 'static':
+
+   void foo(struct P *p[static 1]);
+
+   See https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624814.html
+   for more details.  */
+
+/* In the following function, malloc allocated more space than the value
+   of counted_by attribute.  Then what's the correct behavior we expect 
+   the __builtin_dynamic_object_size should have for each of the cases?  */
+
+static struct annotated * noinline alloc_buf_more (size_t index)
+{
+  struct annotated *p;
+  size_t allocated_size
+    = MAX (sizeof (struct annotated),
+	   (__builtin_offsetof (struct annotated, array[0])
+	    + (index + SIZE_BUMP) * sizeof (char)));
+  p = (struct annotated *) malloc (allocated_size);
+
+  p->foo = index;
+
+  /* When checking the observed access p->array, we have info on both
+    observered allocation and observed access,
+    A.1 from observed allocation: 
+    	allocated_size - offsetof (struct annotated, array[0])
+
+    A.2 from the counted-by attribute:
+    	p->foo * sizeof (char)
+
+    We always use the latest value that is hold by the counted-by field.
+   */
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 0),
+	 (p->foo) * sizeof(char));
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 1),
+	 (p->foo) * sizeof(char));
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 2),
+	 (p->foo) * sizeof(char));
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 3),
+	 (p->foo) * sizeof(char));
+
+  /* When checking the pointer p, we only have info on the observed allocation.
+    So, the object size info can only been obtained from the call to malloc.
+    For both MAXIMUM and MINIMUM: A = (index + SIZE_BUMP) * sizeof (char)  */
+  EXPECT(__builtin_dynamic_object_size(p, 0), allocated_size);
+  EXPECT(__builtin_dynamic_object_size(p, 1), allocated_size);
+  EXPECT(__builtin_dynamic_object_size(p, 2), allocated_size);
+  EXPECT(__builtin_dynamic_object_size(p, 3), allocated_size);
+  return p;
+}
+
+/* In the following function, malloc allocated less space than the value
+   of counted_by attribute.  Then what's the correct behavior we expect 
+   the __builtin_dynamic_object_size should have for each of the cases?
+   NOTE: this is an user error, GCC should issue warnings for such case.
+   This is a seperate issue we should address later.  */
+
+static struct annotated * noinline alloc_buf_less (size_t index)
+{
+  struct annotated *p;
+  size_t allocated_size
+    = MAX (sizeof (struct annotated),
+	   (__builtin_offsetof (struct annotated, array[0])
+	    + (index) * sizeof (char)));
+  p = (struct annotated *) malloc (allocated_size);
+
+  p->foo = index + SIZE_BUMP;
+
+  /* When checking the observed access p->array, we have info on both
+    observered allocation and observed access,
+    A.1 from observed allocation:
+    	allocated_size - offsetof (struct annotated, array[0])
+    A.2 from the counted-by attribute:
+    	p->foo * sizeof (char)
+
+    We always use the latest value that is hold by the counted-by field.
+   */
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 0),
+	 (p->foo) * sizeof(char));
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 1),
+	 (p->foo) * sizeof(char));
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 2),
+	 (p->foo) * sizeof(char));
+
+  EXPECT(__builtin_dynamic_object_size(p->array, 3),
+	 (p->foo) * sizeof(char));
+
+  /* When checking the pointer p, we only have info on the observed
+    allocation. So, the object size info can only been obtained from
+    the call to malloc.  */
+  EXPECT(__builtin_dynamic_object_size(p, 0), allocated_size);
+  EXPECT(__builtin_dynamic_object_size(p, 1), allocated_size);
+  EXPECT(__builtin_dynamic_object_size(p, 2), allocated_size);
+  EXPECT(__builtin_dynamic_object_size(p, 3), allocated_size);
+  return p;
+}
+
+int main ()
+{
+  struct annotated *p, *q;
+  p = alloc_buf_more (10);
+  q = alloc_buf_less (10);
+
+  /* When checking the access p->array, we only have info on the counted-by
+    value.  */ 
+  EXPECT(__builtin_dynamic_object_size(p->array, 0), p->foo * sizeof(char));
+  EXPECT(__builtin_dynamic_object_size(p->array, 1), p->foo * sizeof(char));
+  EXPECT(__builtin_dynamic_object_size(p->array, 2), p->foo * sizeof(char));
+  EXPECT(__builtin_dynamic_object_size(p->array, 3), p->foo * sizeof(char));
+  /* When checking the pointer p, we have no observed allocation nor observed
+    access, therefore, we cannot determine the size info here.  */
+  EXPECT(__builtin_dynamic_object_size(p, 0), -1);
+  EXPECT(__builtin_dynamic_object_size(p, 1), -1);
+  EXPECT(__builtin_dynamic_object_size(p, 2), 0);
+  EXPECT(__builtin_dynamic_object_size(p, 3), 0);
+
+  /* When checking the access p->array, we only have info on the counted-by
+    value.  */ 
+  EXPECT(__builtin_dynamic_object_size(q->array, 0), q->foo * sizeof(char));
+  EXPECT(__builtin_dynamic_object_size(q->array, 1), q->foo * sizeof(char));
+  EXPECT(__builtin_dynamic_object_size(q->array, 2), q->foo * sizeof(char));
+  EXPECT(__builtin_dynamic_object_size(q->array, 3), q->foo * sizeof(char));
+  /* When checking the pointer p, we have no observed allocation nor observed
+    access, therefore, we cannot determine the size info here.  */
+  EXPECT(__builtin_dynamic_object_size(q, 0), -1);
+  EXPECT(__builtin_dynamic_object_size(q, 1), -1);
+  EXPECT(__builtin_dynamic_object_size(q, 2), 0);
+  EXPECT(__builtin_dynamic_object_size(q, 3), 0);
+
+  DONE ();
+}
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
new file mode 100644
index 000000000000..68f9b0f7c8d2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
@@ -0,0 +1,48 @@
+/* Test the attribute counted_by and its usage in
+ * __builtin_dynamic_object_size: when the counted_by field is negative.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+struct annotated {
+  int b;
+  int c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+struct nested_annotated {
+  struct {
+    union {
+      int b;
+      float f;	
+    };
+    int n;
+  };
+  int c[] __attribute__ ((counted_by (b)));
+} *array_nested_annotated;
+
+void __attribute__((__noinline__)) setup (int attr_count)
+{
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated));
+  array_annotated->b = attr_count;
+
+  array_nested_annotated
+    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated));
+  array_nested_annotated->b = attr_count -1;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test ()
+{
+    EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1), 0);
+    EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1), 0);
+}
+
+int main(int argc, char *argv[])
+{
+  setup (-10);   
+  test ();
+  DONE ();
+}
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 018fbc30cbb6..8de264d1dee2 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "attribs.h"
 #include "builtins.h"
 #include "gimplify-me.h"
+#include "gimplify.h"
 
 struct object_size_info
 {
@@ -60,6 +61,7 @@ static tree compute_object_offset (tree, const_tree);
 static bool addr_object_size (struct object_size_info *,
 			      const_tree, int, tree *, tree *t = NULL);
 static tree alloc_object_size (const gcall *, int);
+static tree access_with_size_object_size (const gcall *, int);
 static tree pass_through_call (const gcall *);
 static void collect_object_sizes_for (struct object_size_info *, tree);
 static void expr_object_size (struct object_size_info *, tree, tree);
@@ -749,6 +751,60 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
   return false;
 }
 
+/* Compute __builtin_object_size for a CALL to .ACCESS_WITH_SIZE,
+   OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
+   The 2nd, 3rd, and the 4th parameters of the call determine the size of
+   the CALL:
+
+   2nd argument REF_TO_SIZE: The reference to the size of the object,
+   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
+     0: the number of bytes;
+     1: the number of the elements of the object type;
+   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
+    of the object referenced by REF_TO_SIZE
+
+   The size of the element can be retrived from the result type of the call,
+   which is the pointer to the array type.  */
+static tree
+access_with_size_object_size (const gcall *call, int object_size_type)
+{
+  /* If not for dynamic object size, return.  */
+  if ((object_size_type & OST_DYNAMIC) == 0)
+    return size_unknown (object_size_type);
+
+  gcc_assert (gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE));
+  /* Result type is a pointer type to the original flexible array type.  */
+  tree result_type = gimple_call_return_type (call);
+  gcc_assert (POINTER_TYPE_P (result_type));
+  tree element_size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (result_type)));
+  tree ref_to_size = gimple_call_arg (call, 1);
+  unsigned int class_of_size = TREE_INT_CST_LOW (gimple_call_arg (call, 2));
+  tree type = TREE_TYPE (gimple_call_arg (call, 3));
+
+  tree size = fold_build2 (MEM_REF, type, ref_to_size,
+			   build_int_cst (ptr_type_node, 0));
+
+  /* If size is negative value, treat it as zero.  */
+  if (!TYPE_UNSIGNED (type))
+  {
+    tree cond_expr = fold_build2 (LT_EXPR, boolean_type_node,
+				  unshare_expr (size), build_zero_cst (type));
+    size = fold_build3 (COND_EXPR, integer_type_node, cond_expr,
+			build_zero_cst (type), size);
+  }
+
+  if (class_of_size == 1)
+    size = size_binop (MULT_EXPR,
+		       fold_convert (sizetype, size),
+		       fold_convert (sizetype, element_size));
+  else
+    size = fold_convert (sizetype, size);
+
+  if (!todo)
+    todo = TODO_update_ssa_only_virtuals;
+
+  return size;
+}
 
 /* Compute __builtin_object_size for CALL, which is a GIMPLE_CALL.
    Handles calls to functions declared with attribute alloc_size.
@@ -1350,8 +1406,12 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
 
   bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
   bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
+  bool is_access_with_size
+	 = gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE);
   if (is_strdup || is_strndup)
     bytes = strdup_object_size (call, object_size_type, is_strndup);
+  else if (is_access_with_size)
+    bytes = access_with_size_object_size (call, object_size_type);
   else
     bytes = alloc_object_size (call, object_size_type);
 
-- 
2.31.1


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

* [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer.
  2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
                   ` (2 preceding siblings ...)
  2024-03-29 16:07 ` [PATCH v8 3/5] Use the .ACCESS_WITH_SIZE in builtin object size Qing Zhao
@ 2024-03-29 16:07 ` Qing Zhao
  2024-04-10 18:37   ` Joseph Myers
  2024-04-10 21:46   ` Siddhesh Poyarekar
  2024-03-29 16:07 ` [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE Qing Zhao
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 34+ messages in thread
From: Qing Zhao @ 2024-03-29 16:07 UTC (permalink / raw)
  To: josmyers, richard.guenther, siddhesh, uecker
  Cc: keescook, isanbard, gcc-patches, Qing Zhao

gcc/c-family/ChangeLog:

	* c-ubsan.cc (get_bound_from_access_with_size): New function.
	(ubsan_instrument_bounds): Handle call to .ACCESS_WITH_SIZE.

gcc/testsuite/ChangeLog:

	* gcc.dg/ubsan/flex-array-counted-by-bounds-2.c: New test.
	* gcc.dg/ubsan/flex-array-counted-by-bounds-3.c: New test.
	* gcc.dg/ubsan/flex-array-counted-by-bounds-4.c: New test.
	* gcc.dg/ubsan/flex-array-counted-by-bounds.c: New test.
---
 gcc/c-family/c-ubsan.cc                       | 42 +++++++++++++++++
 .../ubsan/flex-array-counted-by-bounds-2.c    | 45 ++++++++++++++++++
 .../ubsan/flex-array-counted-by-bounds-3.c    | 34 ++++++++++++++
 .../ubsan/flex-array-counted-by-bounds-4.c    | 34 ++++++++++++++
 .../ubsan/flex-array-counted-by-bounds.c      | 46 +++++++++++++++++++
 5 files changed, 201 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c

diff --git a/gcc/c-family/c-ubsan.cc b/gcc/c-family/c-ubsan.cc
index 940982819ddf..7cd3c6aa5b88 100644
--- a/gcc/c-family/c-ubsan.cc
+++ b/gcc/c-family/c-ubsan.cc
@@ -376,6 +376,40 @@ ubsan_instrument_return (location_t loc)
   return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data));
 }
 
+/* Get the tree that represented the number of counted_by, i.e, the maximum
+   number of the elements of the object that the call to .ACCESS_WITH_SIZE
+   points to, this number will be the bound of the corresponding array.  */
+static tree
+get_bound_from_access_with_size (tree call)
+{
+  if (!is_access_with_size_p (call))
+    return NULL_TREE;
+
+  tree ref_to_size = CALL_EXPR_ARG (call, 1);
+  unsigned int class_of_size = TREE_INT_CST_LOW (CALL_EXPR_ARG (call, 2));
+  tree type = TREE_TYPE (CALL_EXPR_ARG (call, 3));
+  tree size = fold_build2 (MEM_REF, type, unshare_expr (ref_to_size),
+			   build_int_cst (ptr_type_node, 0));
+  /* If size is negative value, treat it as zero.  */
+  if (!TYPE_UNSIGNED (type))
+  {
+    tree cond = fold_build2 (LT_EXPR, boolean_type_node,
+			     unshare_expr (size), build_zero_cst (type));
+    size = fold_build3 (COND_EXPR, type, cond,
+			build_zero_cst (type), size);
+  }
+
+  /* Only when class_of_size is 1, i.e, the number of the elements of
+     the object type, return the size.  */
+  if (class_of_size != 1)
+    return NULL_TREE;
+  else
+    size = fold_convert (sizetype, size);
+
+  return size;
+}
+
+
 /* Instrument array bounds for ARRAY_REFs.  We create special builtin,
    that gets expanded in the sanopt pass, and make an array dimension
    of it.  ARRAY is the array, *INDEX is an index to the array.
@@ -401,6 +435,14 @@ ubsan_instrument_bounds (location_t loc, tree array, tree *index,
 	  && COMPLETE_TYPE_P (type)
 	  && integer_zerop (TYPE_SIZE (type)))
 	bound = build_int_cst (TREE_TYPE (TYPE_MIN_VALUE (domain)), -1);
+      else if (INDIRECT_REF_P (array)
+	       && is_access_with_size_p ((TREE_OPERAND (array, 0))))
+	{
+	  bound = get_bound_from_access_with_size ((TREE_OPERAND (array, 0)));
+	  bound = fold_build2 (MINUS_EXPR, TREE_TYPE (bound),
+			       bound,
+			       build_int_cst (TREE_TYPE (bound), 1));
+	}
       else
 	return NULL_TREE;
     }
diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
new file mode 100644
index 000000000000..b503320628d2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
@@ -0,0 +1,45 @@
+/* Test the attribute counted_by and its usage in
+   bounds sanitizer combined with VLA.  */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=bounds" } */
+/* { dg-output "index 11 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*index 20 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*index 11 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*index 10 out of bounds for type 'int \\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+
+
+#include <stdlib.h>
+
+void __attribute__((__noinline__)) setup_and_test_vla (int n, int m)
+{
+   struct foo {
+       int n;
+       int p[][n] __attribute__((counted_by(n)));
+   } *f;
+
+   f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n]));
+   f->n = m;
+   f->p[m][n-1]=1;
+   return;
+}
+
+void __attribute__((__noinline__)) setup_and_test_vla_1 (int n1, int n2, int m)
+{
+  struct foo {
+    int n;
+    int p[][n2][n1] __attribute__((counted_by(n)));
+  } *f;
+
+  f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n2][n1]));
+  f->n = m;
+  f->p[m][n2][n1]=1;
+  return;
+}
+
+int main(int argc, char *argv[])
+{
+  setup_and_test_vla (10, 11);
+  setup_and_test_vla_1 (10, 11, 20);
+  return 0;
+}
+
diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
new file mode 100644
index 000000000000..9da25644af3e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
@@ -0,0 +1,34 @@
+/* Test the attribute counted_by and its usage in bounds
+   sanitizer. when counted_by field is negative value.  */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=bounds" } */
+
+#include <stdlib.h>
+
+struct annotated {
+  int b;
+  int c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+void __attribute__((__noinline__)) setup (int annotated_count)
+{
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated));
+  array_annotated->b = annotated_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test (int annotated_index)
+{
+  array_annotated->c[annotated_index] = 2;
+}
+
+int main(int argc, char *argv[])
+{
+  setup (-3);   
+  test (2);
+  return 0;
+}
+
+/* { dg-output "24:21: runtime error: index 2 out of bounds for type" } */
diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
new file mode 100644
index 000000000000..bd7e144274fc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
@@ -0,0 +1,34 @@
+/* Test the attribute counted_by and its usage in bounds
+   sanitizer. when counted_by field is zero value.  */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=bounds" } */
+
+#include <stdlib.h>
+
+struct annotated {
+  int b;
+  int c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+void __attribute__((__noinline__)) setup (int annotated_count)
+{
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated));
+  array_annotated->b = annotated_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test (int annotated_index)
+{
+  array_annotated->c[annotated_index] = 2;
+}
+
+int main(int argc, char *argv[])
+{
+  setup (0);   
+  test (1);
+  return 0;
+}
+
+/* { dg-output "24:21: runtime error: index 1 out of bounds for type" } */
diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
new file mode 100644
index 000000000000..e2b911dde626
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
@@ -0,0 +1,46 @@
+/* Test the attribute counted_by and its usage in
+   bounds sanitizer.  */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=bounds" } */
+
+#include <stdlib.h>
+
+struct flex {
+  int b;
+  int c[];
+} *array_flex;
+
+struct annotated {
+  int b;
+  int c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+void __attribute__((__noinline__)) setup (int normal_count, int annotated_count)
+{
+  array_flex
+    = (struct flex *)malloc (sizeof (struct flex)
+			     + normal_count *  sizeof (int));
+  array_flex->b = normal_count;
+
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated)
+				  + annotated_count *  sizeof (int));
+  array_annotated->b = annotated_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test (int normal_index, int annotated_index)
+{
+  array_flex->c[normal_index] = 1;
+  array_annotated->c[annotated_index] = 2;
+}
+
+int main(int argc, char *argv[])
+{
+  setup (10, 10);   
+  test (10, 10);
+  return 0;
+}
+
+/* { dg-output "36:21: runtime error: index 10 out of bounds for type" } */
-- 
2.31.1


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

* [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE
  2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
                   ` (3 preceding siblings ...)
  2024-03-29 16:07 ` [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer Qing Zhao
@ 2024-03-29 16:07 ` Qing Zhao
  2024-04-10 18:38   ` Joseph Myers
  2024-04-10 21:48   ` Siddhesh Poyarekar
  2024-03-29 18:09 ` [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Tom Tromey
  2024-03-30 13:57 ` Kees Cook
  6 siblings, 2 replies; 34+ messages in thread
From: Qing Zhao @ 2024-03-29 16:07 UTC (permalink / raw)
  To: josmyers, richard.guenther, siddhesh, uecker
  Cc: keescook, isanbard, gcc-patches, Qing Zhao

to carry the TYPE of the flexible array.

Such information is needed during tree-object-size.cc.

We cannot use the result type or the type of the 1st argument
of the routine .ACCESS_WITH_SIZE to decide the element type
of the original array due to possible type casting in the
source code.

gcc/c/ChangeLog:

	* c-typeck.cc (build_access_with_size_for_counted_by): Add the 6th
	argument to .ACCESS_WITH_SIZE.

gcc/ChangeLog:

	* tree-object-size.cc (access_with_size_object_size): Use the type
	of the 6th argument for the type of the element.

gcc/testsuite/ChangeLog:

	* gcc.dg/flex-array-counted-by-6.c: New test.
---
 gcc/c/c-typeck.cc                             | 11 +++--
 gcc/internal-fn.cc                            |  2 +
 .../gcc.dg/flex-array-counted-by-6.c          | 46 +++++++++++++++++++
 gcc/tree-object-size.cc                       | 16 ++++---
 4 files changed, 66 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-6.c

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index f7b0e08459b0..05948f76039e 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -2608,7 +2608,8 @@ build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
 
    to:
 
-   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
+   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1,
+			(TYPE_OF_ARRAY *)0))
 
    NOTE: The return type of this function is the POINTER type pointing
    to the original flexible array type.
@@ -2620,6 +2621,9 @@ build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
    The 4th argument of the call is a constant 0 with the TYPE of the
    object pointed by COUNTED_BY_REF.
 
+   The 6th argument of the call is a constant 0 with the pointer TYPE
+   to the original flexible array type.
+
   */
 static tree
 build_access_with_size_for_counted_by (location_t loc, tree ref,
@@ -2632,12 +2636,13 @@ build_access_with_size_for_counted_by (location_t loc, tree ref,
 
   tree call
     = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
-				    result_type, 5,
+				    result_type, 6,
 				    array_to_pointer_conversion (loc, ref),
 				    counted_by_ref,
 				    build_int_cst (integer_type_node, 1),
 				    build_int_cst (counted_by_type, 0),
-				    build_int_cst (integer_type_node, -1));
+				    build_int_cst (integer_type_node, -1),
+				    build_int_cst (result_type, 0));
   /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
   call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
   SET_EXPR_LOCATION (call, loc);
diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
index e744080ee670..34e4a4aea534 100644
--- a/gcc/internal-fn.cc
+++ b/gcc/internal-fn.cc
@@ -3411,6 +3411,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
      1: read_only
      2: write_only
      3: read_write
+   6th argument: A constant 0 with the pointer TYPE to the original flexible
+     array type.
 
    Both the return type and the type of the first argument of this
    function have been converted from the incomplete array type to
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
new file mode 100644
index 000000000000..65fa01443d95
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
@@ -0,0 +1,46 @@
+/* Test the attribute counted_by and its usage in
+ * __builtin_dynamic_object_size: when the type of the flexible array member
+ * is casting to another type.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+typedef unsigned short u16;
+
+struct info {
+       u16 data_len;
+       char data[] __attribute__((counted_by(data_len)));
+};
+
+struct foo {
+       int a;
+       int b;
+};
+
+static __attribute__((__noinline__))
+struct info *setup ()
+{
+ struct info *p;
+ size_t bytes = 3 * sizeof(struct foo);
+
+ p = (struct info *)malloc (sizeof (struct info) + bytes);
+ p->data_len = bytes;
+
+ return p;
+}
+
+static void
+__attribute__((__noinline__)) report (struct info *p)
+{
+ struct foo *bar = (struct foo *)p->data;
+ EXPECT(__builtin_dynamic_object_size((char *)(bar + 1), 1), 16);
+ EXPECT(__builtin_dynamic_object_size((char *)(bar + 2), 1), 8);
+}
+
+int main(int argc, char *argv[])
+{
+ struct info *p = setup();
+ report(p);
+ return 0;
+}
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 8de264d1dee2..4c1fa9b555fa 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -762,9 +762,11 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
      1: the number of the elements of the object type;
    4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
     of the object referenced by REF_TO_SIZE
+   6th argument: A constant 0 with the pointer TYPE to the original flexible
+     array type.
 
-   The size of the element can be retrived from the result type of the call,
-   which is the pointer to the array type.  */
+   The size of the element can be retrived from the TYPE of the 6th argument
+   of the call, which is the pointer to the array type.  */
 static tree
 access_with_size_object_size (const gcall *call, int object_size_type)
 {
@@ -773,10 +775,12 @@ access_with_size_object_size (const gcall *call, int object_size_type)
     return size_unknown (object_size_type);
 
   gcc_assert (gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE));
-  /* Result type is a pointer type to the original flexible array type.  */
-  tree result_type = gimple_call_return_type (call);
-  gcc_assert (POINTER_TYPE_P (result_type));
-  tree element_size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (result_type)));
+  /* The type of the 6th argument type is the pointer TYPE to the original
+     flexible array type.  */
+  tree pointer_to_array_type = TREE_TYPE (gimple_call_arg (call, 5));
+  gcc_assert (POINTER_TYPE_P (pointer_to_array_type));
+  tree element_type = TREE_TYPE (TREE_TYPE (pointer_to_array_type));
+  tree element_size = TYPE_SIZE_UNIT (element_type);
   tree ref_to_size = gimple_call_arg (call, 1);
   unsigned int class_of_size = TREE_INT_CST_LOW (gimple_call_arg (call, 2));
   tree type = TREE_TYPE (gimple_call_arg (call, 3));
-- 
2.31.1


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

* Re: [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
  2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
                   ` (4 preceding siblings ...)
  2024-03-29 16:07 ` [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE Qing Zhao
@ 2024-03-29 18:09 ` Tom Tromey
  2024-03-29 19:16   ` Kees Cook
  2024-03-30 13:57 ` Kees Cook
  6 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2024-03-29 18:09 UTC (permalink / raw)
  To: Qing Zhao
  Cc: josmyers, richard.guenther, siddhesh, uecker, keescook, isanbard,
	gcc-patches

>>>>> Qing Zhao <qing.zhao@oracle.com> writes:

> This is the 8th version of the patch.

> compare with the 7th version, the difference are:

[...]

Hi.  I was curious to know if the information supplied by this attribute
shows up in the DWARF.  It would be good if it did, because that would
let gdb correctly print these arrays without user intervention.

Tom

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

* Re: [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
  2024-03-29 18:09 ` [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Tom Tromey
@ 2024-03-29 19:16   ` Kees Cook
  2024-03-29 19:58     ` Qing Zhao
  2024-03-30  0:15     ` Tom Tromey
  0 siblings, 2 replies; 34+ messages in thread
From: Kees Cook @ 2024-03-29 19:16 UTC (permalink / raw)
  To: Tom Tromey
  Cc: Qing Zhao, josmyers, richard.guenther, siddhesh, uecker,
	isanbard, gcc-patches

On Fri, Mar 29, 2024 at 12:09:15PM -0600, Tom Tromey wrote:
> >>>>> Qing Zhao <qing.zhao@oracle.com> writes:
> 
> > This is the 8th version of the patch.
> 
> > compare with the 7th version, the difference are:
> 
> [...]
> 
> Hi.  I was curious to know if the information supplied by this attribute
> shows up in the DWARF.  It would be good if it did, because that would
> let gdb correctly print these arrays without user intervention.

Does DWARF have such an annotation? Regardless, I think this could be a
future patch to not hold up landing the initial feature.

-- 
Kees Cook

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

* Re: [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
  2024-03-29 19:16   ` Kees Cook
@ 2024-03-29 19:58     ` Qing Zhao
  2024-03-30  0:16       ` Tom Tromey
  2024-03-30  0:15     ` Tom Tromey
  1 sibling, 1 reply; 34+ messages in thread
From: Qing Zhao @ 2024-03-29 19:58 UTC (permalink / raw)
  To: Kees Cook, tom
  Cc: Tom Tromey, Joseph Myers, richard.guenther, Siddhesh Poyarekar,
	uecker, isanbard, gcc-patches

Hi,  Tom,

Thanks a lot for the comments. 

It’s good to hear that this new attribute might be able to be used to help gdb. 

We might spend some time to study to use this information in other consumers, for example, gdb, in the future, if necessary and possible.  If you have good examples to show the importance of using such information in gdb, please let me know. I’m glad to study a little more. 

At this time, I agree with Kees, it’s better for the initial patches of the “counted-by” support to focus on the the attribute itself and the immediate security consumers, such as array bound sanitizer and dynamic object size, etc. 

So, let’s delay the possible support to gdb in a later patch. 

Does this sound reasonable to you?

Qing



> On Mar 29, 2024, at 15:16, Kees Cook <keescook@chromium.org> wrote:
> 
> On Fri, Mar 29, 2024 at 12:09:15PM -0600, Tom Tromey wrote:
>>>>>>> Qing Zhao <qing.zhao@oracle.com> writes:
>> 
>>> This is the 8th version of the patch.
>> 
>>> compare with the 7th version, the difference are:
>> 
>> [...]
>> 
>> Hi.  I was curious to know if the information supplied by this attribute
>> shows up in the DWARF.  It would be good if it did, because that would
>> let gdb correctly print these arrays without user intervention.
> 
> Does DWARF have such an annotation? Regardless, I think this could be a
> future patch to not hold up landing the initial feature.
> 
> -- 
> Kees Cook


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

* Re: [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
  2024-03-29 19:16   ` Kees Cook
  2024-03-29 19:58     ` Qing Zhao
@ 2024-03-30  0:15     ` Tom Tromey
  1 sibling, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2024-03-30  0:15 UTC (permalink / raw)
  To: Kees Cook
  Cc: Tom Tromey, Qing Zhao, josmyers, richard.guenther, siddhesh,
	uecker, isanbard, gcc-patches

Kees> Does DWARF have such an annotation? Regardless, I think this could be a
Kees> future patch to not hold up landing the initial feature.

Sure, the compiler can emit the array length (and structure size) as a
DWARF expression using the length.

Tom

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

* Re: [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
  2024-03-29 19:58     ` Qing Zhao
@ 2024-03-30  0:16       ` Tom Tromey
  0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2024-03-30  0:16 UTC (permalink / raw)
  To: Qing Zhao
  Cc: Kees Cook, tom, Joseph Myers, richard.guenther,
	Siddhesh Poyarekar, uecker, isanbard, gcc-patches

> So, let’s delay the possible support to gdb in a later patch. 

> Does this sound reasonable to you?

It's not really up to me, but sure.  I was just curious if it perhaps
already worked, but not enough to apply the patches and find out.

Tom

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

* Re: [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)
  2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
                   ` (5 preceding siblings ...)
  2024-03-29 18:09 ` [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Tom Tromey
@ 2024-03-30 13:57 ` Kees Cook
  6 siblings, 0 replies; 34+ messages in thread
From: Kees Cook @ 2024-03-30 13:57 UTC (permalink / raw)
  To: Qing Zhao
  Cc: josmyers, richard.guenther, siddhesh, uecker, isanbard, gcc-patches

On Fri, Mar 29, 2024 at 04:06:58PM +0000, Qing Zhao wrote:
> This is the 8th version of the patch.

Thanks for the updated version!

I've done a full Linux kernel build and run through my behavioral
regression test suite. Everything is working as expected.

-Kees

-- 
Kees Cook

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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-03-29 16:06 ` [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896) Qing Zhao
@ 2024-04-10 17:35   ` Joseph Myers
  2024-04-10 18:05     ` Qing Zhao
  2024-04-10 18:25     ` Martin Uecker
  0 siblings, 2 replies; 34+ messages in thread
From: Joseph Myers @ 2024-04-10 17:35 UTC (permalink / raw)
  To: Qing Zhao
  Cc: richard.guenther, siddhesh, uecker, keescook, isanbard, gcc-patches

On Fri, 29 Mar 2024, Qing Zhao wrote:

> +  /* Issue error when there is a counted_by attribute with a different
> +     field as the argument for the same flexible array member field.  */

There's another case of this to consider, though I'm not sure where best 
to check for it (Martin might have suggestions) - of course this case will 
need testcases as well.

Suppose, as allowed in C23, a structure is defined twice in the same 
scope, but the two definitions of the structure use inconsistent 
counted_by attributes.  I'd say that, when the declarations are in the 
same scope (thus required to be consistent), it should be an error for the 
two definitions of what is meant to be the same structure to use 
incompatible counted_by attributes (even though the member declarations 
are otherwise the same).

In C23 structures defined with the same tag in different scopes are 
compatible given requirements including compatible types for corresponding 
elements.  It would seem most appropriate to me for such structures with 
incompatible counted_by attributes to be considered *not* compatible types 
(but it would be valid to define structures with the same tag, different 
scopes, and elements the same except for counted_by - just not to use them 
in any way requiring them to be compatible).

> +The @code{counted_by} attribute may be attached to the C99 flexible array
> +member of a structure.  It indicates that the number of the elements of the
> +array is given by the field "@var{count}" in the same structure as the

As noted previously, the "" quotes should be removed there (or replaced by 
``'' quotes).

-- 
Joseph S. Myers
josmyers@redhat.com


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 17:35   ` Joseph Myers
@ 2024-04-10 18:05     ` Qing Zhao
  2024-04-10 18:44       ` Joseph Myers
  2024-04-10 18:25     ` Martin Uecker
  1 sibling, 1 reply; 34+ messages in thread
From: Qing Zhao @ 2024-04-10 18:05 UTC (permalink / raw)
  To: Joseph Myers, uecker
  Cc: Richard Biener, Siddhesh Poyarekar, uecker, Kees Cook, isanbard,
	gcc-patches

Thanks for the comments.

> On Apr 10, 2024, at 13:35, Joseph Myers <josmyers@redhat.com> wrote:
> 
> On Fri, 29 Mar 2024, Qing Zhao wrote:
> 
>> +  /* Issue error when there is a counted_by attribute with a different
>> +     field as the argument for the same flexible array member field.  */
> 
> There's another case of this to consider, though I'm not sure where best 
> to check for it (Martin might have suggestions) - of course this case will 
> need testcases as well.

Looks like this additional case relates to the new C23 feature, where is the
documentation on this new feature, I need to study a little bit on this, thanks.

> 
> Suppose, as allowed in C23, a structure is defined twice in the same 
> scope,

A stupid question first, the same scope means the same file? (Or same function)

Is there a testing case for this feature in current GCC source tree I can take a look? (and
Then I can use it to construct the new testing case for the counted-by attribute).

> but the two definitions of the structure use inconsistent 
> counted_by attributes.

Where in the current C FE to handle the same structure is defined twice in the same scope? Which routine
In the C FE?

>  I'd say that, when the declarations are in the 
> same scope (thus required to be consistent), it should be an error for the 
> two definitions of what is meant to be the same structure to use 
> incompatible counted_by attributes (even though the member declarations 
> are otherwise the same).

Agreed. Wil add such checking. 

> 
> In C23 structures defined with the same tag in different scopes are 
> compatible given requirements including compatible types for corresponding 
> elements.
Again, which routine in the C FE handle such case? I’d like to take a look at the current
Handling and how to update it for the counted-by attribute. 


>  It would seem most appropriate to me for such structures with 
> incompatible counted_by attributes to be considered *not* compatible types

Is there a utility routine for checking “compatible type”? 


> (but it would be valid to define structures with the same tag, different 
> scopes, and elements the same except for counted_by - just not to use them 
> in any way requiring them to be compatible).

Updating that routine (checking compatible type) with the new “counted-by” attribute
Might be enough for this purpose, I guess. 
> 
>> +The @code{counted_by} attribute may be attached to the C99 flexible array
>> +member of a structure.  It indicates that the number of the elements of the
>> +array is given by the field "@var{count}" in the same structure as the
> 
> As noted previously, the "" quotes should be removed there (or replaced by 
> ``'' quotes).

Okay, will update this.

thanks.

Qing
> 
> -- 
> Joseph S. Myers
> josmyers@redhat.com
> 


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 17:35   ` Joseph Myers
  2024-04-10 18:05     ` Qing Zhao
@ 2024-04-10 18:25     ` Martin Uecker
  2024-04-10 19:05       ` Martin Uecker
  1 sibling, 1 reply; 34+ messages in thread
From: Martin Uecker @ 2024-04-10 18:25 UTC (permalink / raw)
  To: Joseph Myers, Qing Zhao
  Cc: richard.guenther, siddhesh, keescook, isanbard, gcc-patches

Am Mittwoch, dem 10.04.2024 um 17:35 +0000 schrieb Joseph Myers:
> On Fri, 29 Mar 2024, Qing Zhao wrote:
> 
> > +  /* Issue error when there is a counted_by attribute with a different
> > +     field as the argument for the same flexible array member field.  */
> 
> There's another case of this to consider, though I'm not sure where best 
> to check for it (Martin might have suggestions) - of course this case will 
> need testcases as well.
> 
> Suppose, as allowed in C23, a structure is defined twice in the same 
> scope, but the two definitions of the structure use inconsistent 
> counted_by attributes.  I'd say that, when the declarations are in the 
> same scope (thus required to be consistent), it should be an error for the 
> two definitions of what is meant to be the same structure to use 
> incompatible counted_by attributes (even though the member declarations 
> are otherwise the same).

I think the right place could be comp_types_attributes in
attributes.cc.  It may be sufficient to set the
affects_type_identify flag.

This should then give a redefinition error as it should do for
"packed".

> 
> In C23 structures defined with the same tag in different scopes are 
> compatible given requirements including compatible types for corresponding 
> elements.  It would seem most appropriate to me for such structures with 
> incompatible counted_by attributes to be considered *not* compatible types 
> (but it would be valid to define structures with the same tag, different 
> scopes, and elements the same except for counted_by - just not to use them 
> in any way requiring them to be compatible).

Another option might be to warn about the case when those types
are then used together in a way where they are required to
be compatible.  Then comp_types_attributes would have to return 2.


Martin

> 
> > +The @code{counted_by} attribute may be attached to the C99 flexible array
> > +member of a structure.  It indicates that the number of the elements of the
> > +array is given by the field "@var{count}" in the same structure as the
> 
> As noted previously, the "" quotes should be removed there (or replaced by 
> ``'' quotes).
> 


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

* Re: [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE.
  2024-03-29 16:07 ` [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE Qing Zhao
@ 2024-04-10 18:36   ` Joseph Myers
  2024-04-10 19:38     ` Qing Zhao
  2024-04-11 13:27     ` Qing Zhao
  0 siblings, 2 replies; 34+ messages in thread
From: Joseph Myers @ 2024-04-10 18:36 UTC (permalink / raw)
  To: Qing Zhao
  Cc: richard.guenther, siddhesh, uecker, keescook, isanbard, gcc-patches

On Fri, 29 Mar 2024, Qing Zhao wrote:

> +/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
> +   the object that represents its counted_by per the attribute counted_by
> +   attached to this field if it's a flexible array member field, otherwise
> +   return NULL_TREE.
> +   set COUNTED_BY_TYPE to the TYPE of the counted_by field.

Use an uppercase letter at the start of a sentence, "Set".

> +static tree
> +build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
> +{
> +  tree type = TREE_TYPE (datum);
> +  if (!(c_flexible_array_member_type_p (TREE_TYPE (subdatum))))
> +    return NULL_TREE;

There are redundant parentheses here around the call to 
c_flexible_array_member_type_p.

The C front-end changes in this patch are OK for GCC 15 (after GCC 14 has 
branched, and once a version of patch 1 has also been approved) with those 
fixes.

-- 
Joseph S. Myers
josmyers@redhat.com


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

* Re: [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer.
  2024-03-29 16:07 ` [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer Qing Zhao
@ 2024-04-10 18:37   ` Joseph Myers
  2024-04-10 21:46   ` Siddhesh Poyarekar
  1 sibling, 0 replies; 34+ messages in thread
From: Joseph Myers @ 2024-04-10 18:37 UTC (permalink / raw)
  To: Qing Zhao
  Cc: richard.guenther, siddhesh, uecker, keescook, isanbard, gcc-patches

The C front-end changes in this patch are OK for GCC 15.

-- 
Joseph S. Myers
josmyers@redhat.com


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

* Re: [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE
  2024-03-29 16:07 ` [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE Qing Zhao
@ 2024-04-10 18:38   ` Joseph Myers
  2024-04-10 21:48   ` Siddhesh Poyarekar
  1 sibling, 0 replies; 34+ messages in thread
From: Joseph Myers @ 2024-04-10 18:38 UTC (permalink / raw)
  To: Qing Zhao
  Cc: richard.guenther, siddhesh, uecker, keescook, isanbard, gcc-patches

The C front-end changes in this patch are OK for GCC 15.

-- 
Joseph S. Myers
josmyers@redhat.com


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 18:05     ` Qing Zhao
@ 2024-04-10 18:44       ` Joseph Myers
  2024-04-10 19:21         ` Qing Zhao
  0 siblings, 1 reply; 34+ messages in thread
From: Joseph Myers @ 2024-04-10 18:44 UTC (permalink / raw)
  To: Qing Zhao
  Cc: uecker, Richard Biener, Siddhesh Poyarekar, Kees Cook, isanbard,
	gcc-patches

On Wed, 10 Apr 2024, Qing Zhao wrote:

> A stupid question first, the same scope means the same file? (Or same function)

struct X { int a; };
struct X { int a; };

is an example of the same scope (file scope, in this case).  The 
structures must have the same contents (in an appropriate sense) and are 
then considered the same type.

struct X { int a; };
void f() { struct X { int a; }; }

is not the same scope - but C23 makes the types compatible (not the same).  
It's OK to have incompatible types with the same tag in different scopes 
as well

struct X { int a; };
void f() { struct X { long b; }; }

but if you use them in a way requiring compatibility, then the contents 
must be compatible

struct X { int a; } v;
void f() { struct X { int a; } *p = &v; }

> Is there a testing case for this feature in current GCC source tree I can take a look? (and
> Then I can use it to construct the new testing case for the counted-by attribute).

See gcc.dg/c23-tag-*.c for many tests of different cases involving the tag 
compatibility rules (and gcc.dg/gnu23-tag-* where GNU extensions are 
involved).

-- 
Joseph S. Myers
josmyers@redhat.com


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 18:25     ` Martin Uecker
@ 2024-04-10 19:05       ` Martin Uecker
  2024-04-10 19:35         ` Qing Zhao
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Uecker @ 2024-04-10 19:05 UTC (permalink / raw)
  To: Joseph Myers, Qing Zhao
  Cc: richard.guenther, siddhesh, keescook, isanbard, gcc-patches

Am Mittwoch, dem 10.04.2024 um 20:25 +0200 schrieb Martin Uecker:
> Am Mittwoch, dem 10.04.2024 um 17:35 +0000 schrieb Joseph Myers:
> > On Fri, 29 Mar 2024, Qing Zhao wrote:
> > 
> > > +  /* Issue error when there is a counted_by attribute with a different
> > > +     field as the argument for the same flexible array member field.  */
> > 
> > There's another case of this to consider, though I'm not sure where best 
> > to check for it (Martin might have suggestions) - of course this case will 
> > need testcases as well.
> > 
> > Suppose, as allowed in C23, a structure is defined twice in the same 
> > scope, but the two definitions of the structure use inconsistent 
> > counted_by attributes.  I'd say that, when the declarations are in the 
> > same scope (thus required to be consistent), it should be an error for the 
> > two definitions of what is meant to be the same structure to use 
> > incompatible counted_by attributes (even though the member declarations 
> > are otherwise the same).
> 
> I think the right place could be comp_types_attributes in
> attributes.cc.  It may be sufficient to set the
> affects_type_identify flag.
> 
> This should then give a redefinition error as it should do for
> "packed".

Thinking about this a bit more, this will not work here, because
the counted_by attribute is not applied to the struct type but
one of the members.

So probably there should be a check added directly
to tagged_types_tu_compatible_p

Martin

> 
> > 
> > In C23 structures defined with the same tag in different scopes are 
> > compatible given requirements including compatible types for corresponding 
> > elements.  It would seem most appropriate to me for such structures with 
> > incompatible counted_by attributes to be considered *not* compatible types 
> > (but it would be valid to define structures with the same tag, different 
> > scopes, and elements the same except for counted_by - just not to use them 
> > in any way requiring them to be compatible).
> 
> Another option might be to warn about the case when those types
> are then used together in a way where they are required to
> be compatible.  Then comp_types_attributes would have to return 2.
> 
> 
> Martin
> 
> > 
> > > +The @code{counted_by} attribute may be attached to the C99 flexible array
> > > +member of a structure.  It indicates that the number of the elements of the
> > > +array is given by the field "@var{count}" in the same structure as the
> > 
> > As noted previously, the "" quotes should be removed there (or replaced by 
> > ``'' quotes).
> > 
> 


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 18:44       ` Joseph Myers
@ 2024-04-10 19:21         ` Qing Zhao
  2024-04-10 21:56           ` Joseph Myers
  0 siblings, 1 reply; 34+ messages in thread
From: Qing Zhao @ 2024-04-10 19:21 UTC (permalink / raw)
  To: Joseph Myers
  Cc: uecker, Richard Biener, Siddhesh Poyarekar, Kees Cook, isanbard,
	gcc-patches



> On Apr 10, 2024, at 14:44, Joseph Myers <josmyers@redhat.com> wrote:
> 
> On Wed, 10 Apr 2024, Qing Zhao wrote:
> 
>> A stupid question first, the same scope means the same file? (Or same function)
> 
> struct X { int a; };
> struct X { int a; };
> 
> is an example of the same scope (file scope, in this case).  The 
> structures must have the same contents (in an appropriate sense) and are 
> then considered the same type.
> 
> struct X { int a; };
> void f() { struct X { int a; }; }
> 
> is not the same scope - but C23 makes the types compatible (not the same).  
> It's OK to have incompatible types with the same tag in different scopes 
> as well
> 
> struct X { int a; };
> void f() { struct X { long b; }; }
> 
> but if you use them in a way requiring compatibility, then the contents 
> must be compatible
> 
> struct X { int a; } v;
> void f() { struct X { int a; } *p = &v; }

Okay, the above is very clear, thanks a lot for the explanation.
So, basically, for “counted-by” attribute:
**The following is good:
struct f { 
  int b;
  int c;
  int a[]  __attribute__ ((counted_by (b))) };
struct f {
  int b;
  int c;
  int a[] __attribute__ ((counted_by (b))) };

**The following should error:

struct f { 
  int b;
  int c;
  int a[]  __attribute__ ((counted_by (b))) };
struct f {
  int b;
  int c;
  int a[] __attribute__ ((counted_by (c))) };  /* error here */

For the same tag in different scopes case:

struct f { 
  int b;
  int c;
  int a[]  __attribute__ ((counted_by (b))) }  y0;

void test1(void) 
{   
struct f {
  int b;
  int c;
  int a[] __attribute__ ((counted_by (c))) } x;

  y0 = x;  /* will report incompatible type error here */
}

Are the above complete?

> 
>> Is there a testing case for this feature in current GCC source tree I can take a look? (and
>> Then I can use it to construct the new testing case for the counted-by attribute).
> 
> See gcc.dg/c23-tag-*.c for many tests of different cases involving the tag 
> compatibility rules (and gcc.dg/gnu23-tag-* where GNU extensions are 
> involved).

Got it. Will take a look on them.

thanks.

Qing

> 
> -- 
> Joseph S. Myers
> josmyers@redhat.com
> 


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 19:05       ` Martin Uecker
@ 2024-04-10 19:35         ` Qing Zhao
  2024-04-11  6:02           ` Martin Uecker
  0 siblings, 1 reply; 34+ messages in thread
From: Qing Zhao @ 2024-04-10 19:35 UTC (permalink / raw)
  To: Martin Uecker
  Cc: Joseph Myers, Richard Biener, Siddhesh Poyarekar, Kees Cook,
	isanbard, gcc-patches



> On Apr 10, 2024, at 15:05, Martin Uecker <uecker@tugraz.at> wrote:
> 
> Am Mittwoch, dem 10.04.2024 um 20:25 +0200 schrieb Martin Uecker:
>> Am Mittwoch, dem 10.04.2024 um 17:35 +0000 schrieb Joseph Myers:
>>> On Fri, 29 Mar 2024, Qing Zhao wrote:
>>> 
>>>> +  /* Issue error when there is a counted_by attribute with a different
>>>> +     field as the argument for the same flexible array member field.  */
>>> 
>>> There's another case of this to consider, though I'm not sure where best 
>>> to check for it (Martin might have suggestions) - of course this case will 
>>> need testcases as well.
>>> 
>>> Suppose, as allowed in C23, a structure is defined twice in the same 
>>> scope, but the two definitions of the structure use inconsistent 
>>> counted_by attributes.  I'd say that, when the declarations are in the 
>>> same scope (thus required to be consistent), it should be an error for the 
>>> two definitions of what is meant to be the same structure to use 
>>> incompatible counted_by attributes (even though the member declarations 
>>> are otherwise the same).
>> 
>> I think the right place could be comp_types_attributes in
>> attributes.cc.  It may be sufficient to set the
>> affects_type_identify flag.
>> 
>> This should then give a redefinition error as it should do for
>> "packed".
> 
> Thinking about this a bit more, this will not work here, because
> the counted_by attribute is not applied to the struct type but
> one of the members.
> 
> So probably there should be a check added directly
> to tagged_types_tu_compatible_p


There are two cases we will check:

  A. Both definitions are in the same scope;
      Then if the 2nd definition has a counted-by attribute different from the 1st definition, the 2nd definition will be given a redefinition error; 

  B. These two definitions are in different scope;
      When these two definitions are used in a way need to be compatible, an incompatible error need to be issued at that
Point;


My question is, Will the routine “tagged_types_tu_compatible_p” can handle both A and B?

Thanks.

Qing
> 
> Martin
> 
>> 
>>> 
>>> In C23 structures defined with the same tag in different scopes are 
>>> compatible given requirements including compatible types for corresponding 
>>> elements.  It would seem most appropriate to me for such structures with 
>>> incompatible counted_by attributes to be considered *not* compatible types 
>>> (but it would be valid to define structures with the same tag, different 
>>> scopes, and elements the same except for counted_by - just not to use them 
>>> in any way requiring them to be compatible).
>> 
>> Another option might be to warn about the case when those types
>> are then used together in a way where they are required to
>> be compatible.  Then comp_types_attributes would have to return 2.
>> 
>> 
>> Martin
>> 
>>> 
>>>> +The @code{counted_by} attribute may be attached to the C99 flexible array
>>>> +member of a structure.  It indicates that the number of the elements of the
>>>> +array is given by the field "@var{count}" in the same structure as the
>>> 
>>> As noted previously, the "" quotes should be removed there (or replaced by 
>>> ``'' quotes).
>>> 
>> 
> 


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

* Re: [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE.
  2024-04-10 18:36   ` Joseph Myers
@ 2024-04-10 19:38     ` Qing Zhao
  2024-04-11 13:27     ` Qing Zhao
  1 sibling, 0 replies; 34+ messages in thread
From: Qing Zhao @ 2024-04-10 19:38 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Richard Biener, Siddhesh Poyarekar, uecker, Kees Cook, isanbard,
	gcc-patches

Thanks a lot for your review.

Will fix these typos before committing to GCC15.

Qing

> On Apr 10, 2024, at 14:36, Joseph Myers <josmyers@redhat.com> wrote:
> 
> On Fri, 29 Mar 2024, Qing Zhao wrote:
> 
>> +/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
>> +   the object that represents its counted_by per the attribute counted_by
>> +   attached to this field if it's a flexible array member field, otherwise
>> +   return NULL_TREE.
>> +   set COUNTED_BY_TYPE to the TYPE of the counted_by field.
> 
> Use an uppercase letter at the start of a sentence, "Set".
> 
>> +static tree
>> +build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>> +{
>> +  tree type = TREE_TYPE (datum);
>> +  if (!(c_flexible_array_member_type_p (TREE_TYPE (subdatum))))
>> +    return NULL_TREE;
> 
> There are redundant parentheses here around the call to 
> c_flexible_array_member_type_p.
> 
> The C front-end changes in this patch are OK for GCC 15 (after GCC 14 has 
> branched, and once a version of patch 1 has also been approved) with those 
> fixes.
> 
> -- 
> Joseph S. Myers
> josmyers@redhat.com
> 


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

* Re: [PATCH v8 3/5] Use the .ACCESS_WITH_SIZE in builtin object size.
  2024-03-29 16:07 ` [PATCH v8 3/5] Use the .ACCESS_WITH_SIZE in builtin object size Qing Zhao
@ 2024-04-10 21:45   ` Siddhesh Poyarekar
  2024-04-11 13:19     ` Qing Zhao
  0 siblings, 1 reply; 34+ messages in thread
From: Siddhesh Poyarekar @ 2024-04-10 21:45 UTC (permalink / raw)
  To: Qing Zhao, josmyers, richard.guenther, uecker
  Cc: keescook, isanbard, gcc-patches

On 2024-03-29 12:07, Qing Zhao wrote:
> gcc/ChangeLog:
> 
> 	* tree-object-size.cc (access_with_size_object_size): New function.
> 	(call_object_size): Call the new function.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/builtin-object-size-common.h: Add a new macro EXPECT.
> 	* gcc.dg/flex-array-counted-by-3.c: New test.
> 	* gcc.dg/flex-array-counted-by-4.c: New test.
> 	* gcc.dg/flex-array-counted-by-5.c: New test.

This version looks fine to me for stage 1, but I'm not a maintainer so 
you'll need an ack from one to commit.

Thanks,
Sid

> ---
>   .../gcc.dg/builtin-object-size-common.h       |  11 ++
>   .../gcc.dg/flex-array-counted-by-3.c          |  63 +++++++
>   .../gcc.dg/flex-array-counted-by-4.c          | 178 ++++++++++++++++++
>   .../gcc.dg/flex-array-counted-by-5.c          |  48 +++++
>   gcc/tree-object-size.cc                       |  60 ++++++
>   5 files changed, 360 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>   create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>   create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
> 
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-common.h b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
> index 66ff7cdd953a..b677067c6e6b 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-common.h
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
> @@ -30,3 +30,14 @@ unsigned nfails = 0;
>         __builtin_abort ();						      \
>       return 0;								      \
>     } while (0)
> +
> +#define EXPECT(p, _v) do {						      \
> +  size_t v = _v;							      \
> +  if (p == v)								      \
> +    __builtin_printf ("ok:  %s == %zd\n", #p, p);			      \
> +  else									      \
> +    {									      \
> +      __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v);	      \
> +      FAIL ();								      \
> +    }									      \
> +} while (0);
> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
> new file mode 100644
> index 000000000000..78f50230e891
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
> @@ -0,0 +1,63 @@
> +/* Test the attribute counted_by and its usage in
> + * __builtin_dynamic_object_size.  */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include "builtin-object-size-common.h"
> +
> +struct flex {
> +  int b;
> +  int c[];
> +} *array_flex;
> +
> +struct annotated {
> +  int b;
> +  int c[] __attribute__ ((counted_by (b)));
> +} *array_annotated;
> +
> +struct nested_annotated {
> +  struct {
> +    union {
> +      int b;
> +      float f;	
> +    };
> +    int n;
> +  };
> +  int c[] __attribute__ ((counted_by (b)));
> +} *array_nested_annotated;
> +
> +void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
> +{
> +  array_flex
> +    = (struct flex *)malloc (sizeof (struct flex)
> +			     + normal_count *  sizeof (int));
> +  array_flex->b = normal_count;
> +
> +  array_annotated
> +    = (struct annotated *)malloc (sizeof (struct annotated)
> +				  + attr_count *  sizeof (int));
> +  array_annotated->b = attr_count;
> +
> +  array_nested_annotated
> +    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
> +					 + attr_count *  sizeof (int));
> +  array_nested_annotated->b = attr_count;
> +
> +  return;
> +}
> +
> +void __attribute__((__noinline__)) test ()
> +{
> +    EXPECT(__builtin_dynamic_object_size(array_flex->c, 1), -1);
> +    EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1),
> +	   array_annotated->b * sizeof (int));
> +    EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1),
> +	   array_nested_annotated->b * sizeof (int));
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  setup (10,10);
> +  test ();
> +  DONE ();
> +}
> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
> new file mode 100644
> index 000000000000..20103d58ef51
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
> @@ -0,0 +1,178 @@
> +/* Test the attribute counted_by and its usage in
> +__builtin_dynamic_object_size: what's the correct behavior when the
> +allocation size mismatched with the value of counted_by attribute?
> +We should always use the latest value that is hold by the counted_by
> +field.  */
> +/* { dg-do run } */
> +/* { dg-options "-O -fstrict-flex-arrays=3" } */
> +
> +#include "builtin-object-size-common.h"
> +
> +struct annotated {
> +  size_t foo;
> +  char others;
> +  char array[] __attribute__((counted_by (foo)));
> +};
> +
> +#define noinline __attribute__((__noinline__))
> +#define SIZE_BUMP 10
> +#define MAX(a, b) ((a) > (b) ? (a) : (b))
> +
> +/* In general, Due to type casting, the type for the pointee of a pointer
> +   does not say anything about the object it points to,
> +   So, __builtin_object_size can not directly use the type of the pointee
> +   to decide the size of the object the pointer points to.
> +
> +   There are only two reliable ways:
> +   A. observed allocations  (call to the allocation functions in the routine)
> +   B. observed accesses     (read or write access to the location of the
> +                             pointer points to)
> +
> +   That provide information about the type/existence of an object at
> +   the corresponding address.
> +
> +   For A, we use the "alloc_size" attribute for the corresponding allocation
> +   functions to determine the object size;
> +   (We treat counted_by attribute the same as the "alloc_size" attribute)
> +
> +   For B, we use the SIZE info of the TYPE attached to the corresponding access.
> +
> +   The only other way in C which ensures that a pointer actually points
> +   to an object of the correct type is 'static':
> +
> +   void foo(struct P *p[static 1]);
> +
> +   See https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624814.html
> +   for more details.  */
> +
> +/* In the following function, malloc allocated more space than the value
> +   of counted_by attribute.  Then what's the correct behavior we expect
> +   the __builtin_dynamic_object_size should have for each of the cases?  */
> +
> +static struct annotated * noinline alloc_buf_more (size_t index)
> +{
> +  struct annotated *p;
> +  size_t allocated_size
> +    = MAX (sizeof (struct annotated),
> +	   (__builtin_offsetof (struct annotated, array[0])
> +	    + (index + SIZE_BUMP) * sizeof (char)));
> +  p = (struct annotated *) malloc (allocated_size);
> +
> +  p->foo = index;
> +
> +  /* When checking the observed access p->array, we have info on both
> +    observered allocation and observed access,
> +    A.1 from observed allocation:
> +    	allocated_size - offsetof (struct annotated, array[0])
> +
> +    A.2 from the counted-by attribute:
> +    	p->foo * sizeof (char)
> +
> +    We always use the latest value that is hold by the counted-by field.
> +   */
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 0),
> +	 (p->foo) * sizeof(char));
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 1),
> +	 (p->foo) * sizeof(char));
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 2),
> +	 (p->foo) * sizeof(char));
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 3),
> +	 (p->foo) * sizeof(char));
> +
> +  /* When checking the pointer p, we only have info on the observed allocation.
> +    So, the object size info can only been obtained from the call to malloc.
> +    For both MAXIMUM and MINIMUM: A = (index + SIZE_BUMP) * sizeof (char)  */
> +  EXPECT(__builtin_dynamic_object_size(p, 0), allocated_size);
> +  EXPECT(__builtin_dynamic_object_size(p, 1), allocated_size);
> +  EXPECT(__builtin_dynamic_object_size(p, 2), allocated_size);
> +  EXPECT(__builtin_dynamic_object_size(p, 3), allocated_size);
> +  return p;
> +}
> +
> +/* In the following function, malloc allocated less space than the value
> +   of counted_by attribute.  Then what's the correct behavior we expect
> +   the __builtin_dynamic_object_size should have for each of the cases?
> +   NOTE: this is an user error, GCC should issue warnings for such case.
> +   This is a seperate issue we should address later.  */
> +
> +static struct annotated * noinline alloc_buf_less (size_t index)
> +{
> +  struct annotated *p;
> +  size_t allocated_size
> +    = MAX (sizeof (struct annotated),
> +	   (__builtin_offsetof (struct annotated, array[0])
> +	    + (index) * sizeof (char)));
> +  p = (struct annotated *) malloc (allocated_size);
> +
> +  p->foo = index + SIZE_BUMP;
> +
> +  /* When checking the observed access p->array, we have info on both
> +    observered allocation and observed access,
> +    A.1 from observed allocation:
> +    	allocated_size - offsetof (struct annotated, array[0])
> +    A.2 from the counted-by attribute:
> +    	p->foo * sizeof (char)
> +
> +    We always use the latest value that is hold by the counted-by field.
> +   */
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 0),
> +	 (p->foo) * sizeof(char));
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 1),
> +	 (p->foo) * sizeof(char));
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 2),
> +	 (p->foo) * sizeof(char));
> +
> +  EXPECT(__builtin_dynamic_object_size(p->array, 3),
> +	 (p->foo) * sizeof(char));
> +
> +  /* When checking the pointer p, we only have info on the observed
> +    allocation. So, the object size info can only been obtained from
> +    the call to malloc.  */
> +  EXPECT(__builtin_dynamic_object_size(p, 0), allocated_size);
> +  EXPECT(__builtin_dynamic_object_size(p, 1), allocated_size);
> +  EXPECT(__builtin_dynamic_object_size(p, 2), allocated_size);
> +  EXPECT(__builtin_dynamic_object_size(p, 3), allocated_size);
> +  return p;
> +}
> +
> +int main ()
> +{
> +  struct annotated *p, *q;
> +  p = alloc_buf_more (10);
> +  q = alloc_buf_less (10);
> +
> +  /* When checking the access p->array, we only have info on the counted-by
> +    value.  */
> +  EXPECT(__builtin_dynamic_object_size(p->array, 0), p->foo * sizeof(char));
> +  EXPECT(__builtin_dynamic_object_size(p->array, 1), p->foo * sizeof(char));
> +  EXPECT(__builtin_dynamic_object_size(p->array, 2), p->foo * sizeof(char));
> +  EXPECT(__builtin_dynamic_object_size(p->array, 3), p->foo * sizeof(char));
> +  /* When checking the pointer p, we have no observed allocation nor observed
> +    access, therefore, we cannot determine the size info here.  */
> +  EXPECT(__builtin_dynamic_object_size(p, 0), -1);
> +  EXPECT(__builtin_dynamic_object_size(p, 1), -1);
> +  EXPECT(__builtin_dynamic_object_size(p, 2), 0);
> +  EXPECT(__builtin_dynamic_object_size(p, 3), 0);
> +
> +  /* When checking the access p->array, we only have info on the counted-by
> +    value.  */
> +  EXPECT(__builtin_dynamic_object_size(q->array, 0), q->foo * sizeof(char));
> +  EXPECT(__builtin_dynamic_object_size(q->array, 1), q->foo * sizeof(char));
> +  EXPECT(__builtin_dynamic_object_size(q->array, 2), q->foo * sizeof(char));
> +  EXPECT(__builtin_dynamic_object_size(q->array, 3), q->foo * sizeof(char));
> +  /* When checking the pointer p, we have no observed allocation nor observed
> +    access, therefore, we cannot determine the size info here.  */
> +  EXPECT(__builtin_dynamic_object_size(q, 0), -1);
> +  EXPECT(__builtin_dynamic_object_size(q, 1), -1);
> +  EXPECT(__builtin_dynamic_object_size(q, 2), 0);
> +  EXPECT(__builtin_dynamic_object_size(q, 3), 0);
> +
> +  DONE ();
> +}
> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
> new file mode 100644
> index 000000000000..68f9b0f7c8d2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
> @@ -0,0 +1,48 @@
> +/* Test the attribute counted_by and its usage in
> + * __builtin_dynamic_object_size: when the counted_by field is negative.  */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include "builtin-object-size-common.h"
> +
> +struct annotated {
> +  int b;
> +  int c[] __attribute__ ((counted_by (b)));
> +} *array_annotated;
> +
> +struct nested_annotated {
> +  struct {
> +    union {
> +      int b;
> +      float f;	
> +    };
> +    int n;
> +  };
> +  int c[] __attribute__ ((counted_by (b)));
> +} *array_nested_annotated;
> +
> +void __attribute__((__noinline__)) setup (int attr_count)
> +{
> +  array_annotated
> +    = (struct annotated *)malloc (sizeof (struct annotated));
> +  array_annotated->b = attr_count;
> +
> +  array_nested_annotated
> +    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated));
> +  array_nested_annotated->b = attr_count -1;
> +
> +  return;
> +}
> +
> +void __attribute__((__noinline__)) test ()
> +{
> +    EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1), 0);
> +    EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1), 0);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  setup (-10);
> +  test ();
> +  DONE ();
> +}
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 018fbc30cbb6..8de264d1dee2 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "attribs.h"
>   #include "builtins.h"
>   #include "gimplify-me.h"
> +#include "gimplify.h"
>   
>   struct object_size_info
>   {
> @@ -60,6 +61,7 @@ static tree compute_object_offset (tree, const_tree);
>   static bool addr_object_size (struct object_size_info *,
>   			      const_tree, int, tree *, tree *t = NULL);
>   static tree alloc_object_size (const gcall *, int);
> +static tree access_with_size_object_size (const gcall *, int);
>   static tree pass_through_call (const gcall *);
>   static void collect_object_sizes_for (struct object_size_info *, tree);
>   static void expr_object_size (struct object_size_info *, tree, tree);
> @@ -749,6 +751,60 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>     return false;
>   }
>   
> +/* Compute __builtin_object_size for a CALL to .ACCESS_WITH_SIZE,
> +   OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
> +   The 2nd, 3rd, and the 4th parameters of the call determine the size of
> +   the CALL:
> +
> +   2nd argument REF_TO_SIZE: The reference to the size of the object,
> +   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
> +     0: the number of bytes;
> +     1: the number of the elements of the object type;
> +   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
> +    of the object referenced by REF_TO_SIZE
> +
> +   The size of the element can be retrived from the result type of the call,
> +   which is the pointer to the array type.  */
> +static tree
> +access_with_size_object_size (const gcall *call, int object_size_type)
> +{
> +  /* If not for dynamic object size, return.  */
> +  if ((object_size_type & OST_DYNAMIC) == 0)
> +    return size_unknown (object_size_type);
> +
> +  gcc_assert (gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE));
> +  /* Result type is a pointer type to the original flexible array type.  */
> +  tree result_type = gimple_call_return_type (call);
> +  gcc_assert (POINTER_TYPE_P (result_type));
> +  tree element_size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (result_type)));
> +  tree ref_to_size = gimple_call_arg (call, 1);
> +  unsigned int class_of_size = TREE_INT_CST_LOW (gimple_call_arg (call, 2));
> +  tree type = TREE_TYPE (gimple_call_arg (call, 3));
> +
> +  tree size = fold_build2 (MEM_REF, type, ref_to_size,
> +			   build_int_cst (ptr_type_node, 0));
> +
> +  /* If size is negative value, treat it as zero.  */
> +  if (!TYPE_UNSIGNED (type))
> +  {
> +    tree cond_expr = fold_build2 (LT_EXPR, boolean_type_node,
> +				  unshare_expr (size), build_zero_cst (type));
> +    size = fold_build3 (COND_EXPR, integer_type_node, cond_expr,
> +			build_zero_cst (type), size);
> +  }
> +
> +  if (class_of_size == 1)
> +    size = size_binop (MULT_EXPR,
> +		       fold_convert (sizetype, size),
> +		       fold_convert (sizetype, element_size));
> +  else
> +    size = fold_convert (sizetype, size);
> +
> +  if (!todo)
> +    todo = TODO_update_ssa_only_virtuals;
> +
> +  return size;
> +}
>   
>   /* Compute __builtin_object_size for CALL, which is a GIMPLE_CALL.
>      Handles calls to functions declared with attribute alloc_size.
> @@ -1350,8 +1406,12 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
>   
>     bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
>     bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
> +  bool is_access_with_size
> +	 = gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE);
>     if (is_strdup || is_strndup)
>       bytes = strdup_object_size (call, object_size_type, is_strndup);
> +  else if (is_access_with_size)
> +    bytes = access_with_size_object_size (call, object_size_type);
>     else
>       bytes = alloc_object_size (call, object_size_type);
>   

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

* Re: [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer.
  2024-03-29 16:07 ` [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer Qing Zhao
  2024-04-10 18:37   ` Joseph Myers
@ 2024-04-10 21:46   ` Siddhesh Poyarekar
  2024-04-11 13:22     ` Qing Zhao
  1 sibling, 1 reply; 34+ messages in thread
From: Siddhesh Poyarekar @ 2024-04-10 21:46 UTC (permalink / raw)
  To: Qing Zhao, josmyers, richard.guenther, uecker
  Cc: keescook, isanbard, gcc-patches

On 2024-03-29 12:07, Qing Zhao wrote:
> gcc/c-family/ChangeLog:
> 
> 	* c-ubsan.cc (get_bound_from_access_with_size): New function.
> 	(ubsan_instrument_bounds): Handle call to .ACCESS_WITH_SIZE.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/ubsan/flex-array-counted-by-bounds-2.c: New test.
> 	* gcc.dg/ubsan/flex-array-counted-by-bounds-3.c: New test.
> 	* gcc.dg/ubsan/flex-array-counted-by-bounds-4.c: New test.
> 	* gcc.dg/ubsan/flex-array-counted-by-bounds.c: New test.
> ---

This version looks fine to me for stage 1, but I'm not a maintainer so 
you'll need an ack from one to commit.

Thanks,
Sid

>   gcc/c-family/c-ubsan.cc                       | 42 +++++++++++++++++
>   .../ubsan/flex-array-counted-by-bounds-2.c    | 45 ++++++++++++++++++
>   .../ubsan/flex-array-counted-by-bounds-3.c    | 34 ++++++++++++++
>   .../ubsan/flex-array-counted-by-bounds-4.c    | 34 ++++++++++++++
>   .../ubsan/flex-array-counted-by-bounds.c      | 46 +++++++++++++++++++
>   5 files changed, 201 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
>   create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
>   create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
>   create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
> 
> diff --git a/gcc/c-family/c-ubsan.cc b/gcc/c-family/c-ubsan.cc
> index 940982819ddf..7cd3c6aa5b88 100644
> --- a/gcc/c-family/c-ubsan.cc
> +++ b/gcc/c-family/c-ubsan.cc
> @@ -376,6 +376,40 @@ ubsan_instrument_return (location_t loc)
>     return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data));
>   }
>   
> +/* Get the tree that represented the number of counted_by, i.e, the maximum
> +   number of the elements of the object that the call to .ACCESS_WITH_SIZE
> +   points to, this number will be the bound of the corresponding array.  */
> +static tree
> +get_bound_from_access_with_size (tree call)
> +{
> +  if (!is_access_with_size_p (call))
> +    return NULL_TREE;
> +
> +  tree ref_to_size = CALL_EXPR_ARG (call, 1);
> +  unsigned int class_of_size = TREE_INT_CST_LOW (CALL_EXPR_ARG (call, 2));
> +  tree type = TREE_TYPE (CALL_EXPR_ARG (call, 3));
> +  tree size = fold_build2 (MEM_REF, type, unshare_expr (ref_to_size),
> +			   build_int_cst (ptr_type_node, 0));
> +  /* If size is negative value, treat it as zero.  */
> +  if (!TYPE_UNSIGNED (type))
> +  {
> +    tree cond = fold_build2 (LT_EXPR, boolean_type_node,
> +			     unshare_expr (size), build_zero_cst (type));
> +    size = fold_build3 (COND_EXPR, type, cond,
> +			build_zero_cst (type), size);
> +  }
> +
> +  /* Only when class_of_size is 1, i.e, the number of the elements of
> +     the object type, return the size.  */
> +  if (class_of_size != 1)
> +    return NULL_TREE;
> +  else
> +    size = fold_convert (sizetype, size);
> +
> +  return size;
> +}
> +
> +
>   /* Instrument array bounds for ARRAY_REFs.  We create special builtin,
>      that gets expanded in the sanopt pass, and make an array dimension
>      of it.  ARRAY is the array, *INDEX is an index to the array.
> @@ -401,6 +435,14 @@ ubsan_instrument_bounds (location_t loc, tree array, tree *index,
>   	  && COMPLETE_TYPE_P (type)
>   	  && integer_zerop (TYPE_SIZE (type)))
>   	bound = build_int_cst (TREE_TYPE (TYPE_MIN_VALUE (domain)), -1);
> +      else if (INDIRECT_REF_P (array)
> +	       && is_access_with_size_p ((TREE_OPERAND (array, 0))))
> +	{
> +	  bound = get_bound_from_access_with_size ((TREE_OPERAND (array, 0)));
> +	  bound = fold_build2 (MINUS_EXPR, TREE_TYPE (bound),
> +			       bound,
> +			       build_int_cst (TREE_TYPE (bound), 1));
> +	}
>         else
>   	return NULL_TREE;
>       }
> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
> new file mode 100644
> index 000000000000..b503320628d2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
> @@ -0,0 +1,45 @@
> +/* Test the attribute counted_by and its usage in
> +   bounds sanitizer combined with VLA.  */
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=bounds" } */
> +/* { dg-output "index 11 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*index 20 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*index 11 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*index 10 out of bounds for type 'int \\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
> +
> +
> +#include <stdlib.h>
> +
> +void __attribute__((__noinline__)) setup_and_test_vla (int n, int m)
> +{
> +   struct foo {
> +       int n;
> +       int p[][n] __attribute__((counted_by(n)));
> +   } *f;
> +
> +   f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n]));
> +   f->n = m;
> +   f->p[m][n-1]=1;
> +   return;
> +}
> +
> +void __attribute__((__noinline__)) setup_and_test_vla_1 (int n1, int n2, int m)
> +{
> +  struct foo {
> +    int n;
> +    int p[][n2][n1] __attribute__((counted_by(n)));
> +  } *f;
> +
> +  f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n2][n1]));
> +  f->n = m;
> +  f->p[m][n2][n1]=1;
> +  return;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  setup_and_test_vla (10, 11);
> +  setup_and_test_vla_1 (10, 11, 20);
> +  return 0;
> +}
> +
> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
> new file mode 100644
> index 000000000000..9da25644af3e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
> @@ -0,0 +1,34 @@
> +/* Test the attribute counted_by and its usage in bounds
> +   sanitizer. when counted_by field is negative value.  */
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=bounds" } */
> +
> +#include <stdlib.h>
> +
> +struct annotated {
> +  int b;
> +  int c[] __attribute__ ((counted_by (b)));
> +} *array_annotated;
> +
> +void __attribute__((__noinline__)) setup (int annotated_count)
> +{
> +  array_annotated
> +    = (struct annotated *)malloc (sizeof (struct annotated));
> +  array_annotated->b = annotated_count;
> +
> +  return;
> +}
> +
> +void __attribute__((__noinline__)) test (int annotated_index)
> +{
> +  array_annotated->c[annotated_index] = 2;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  setup (-3);
> +  test (2);
> +  return 0;
> +}
> +
> +/* { dg-output "24:21: runtime error: index 2 out of bounds for type" } */
> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
> new file mode 100644
> index 000000000000..bd7e144274fc
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
> @@ -0,0 +1,34 @@
> +/* Test the attribute counted_by and its usage in bounds
> +   sanitizer. when counted_by field is zero value.  */
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=bounds" } */
> +
> +#include <stdlib.h>
> +
> +struct annotated {
> +  int b;
> +  int c[] __attribute__ ((counted_by (b)));
> +} *array_annotated;
> +
> +void __attribute__((__noinline__)) setup (int annotated_count)
> +{
> +  array_annotated
> +    = (struct annotated *)malloc (sizeof (struct annotated));
> +  array_annotated->b = annotated_count;
> +
> +  return;
> +}
> +
> +void __attribute__((__noinline__)) test (int annotated_index)
> +{
> +  array_annotated->c[annotated_index] = 2;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  setup (0);
> +  test (1);
> +  return 0;
> +}
> +
> +/* { dg-output "24:21: runtime error: index 1 out of bounds for type" } */
> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
> new file mode 100644
> index 000000000000..e2b911dde626
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
> @@ -0,0 +1,46 @@
> +/* Test the attribute counted_by and its usage in
> +   bounds sanitizer.  */
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=bounds" } */
> +
> +#include <stdlib.h>
> +
> +struct flex {
> +  int b;
> +  int c[];
> +} *array_flex;
> +
> +struct annotated {
> +  int b;
> +  int c[] __attribute__ ((counted_by (b)));
> +} *array_annotated;
> +
> +void __attribute__((__noinline__)) setup (int normal_count, int annotated_count)
> +{
> +  array_flex
> +    = (struct flex *)malloc (sizeof (struct flex)
> +			     + normal_count *  sizeof (int));
> +  array_flex->b = normal_count;
> +
> +  array_annotated
> +    = (struct annotated *)malloc (sizeof (struct annotated)
> +				  + annotated_count *  sizeof (int));
> +  array_annotated->b = annotated_count;
> +
> +  return;
> +}
> +
> +void __attribute__((__noinline__)) test (int normal_index, int annotated_index)
> +{
> +  array_flex->c[normal_index] = 1;
> +  array_annotated->c[annotated_index] = 2;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  setup (10, 10);
> +  test (10, 10);
> +  return 0;
> +}
> +
> +/* { dg-output "36:21: runtime error: index 10 out of bounds for type" } */

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

* Re: [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE
  2024-03-29 16:07 ` [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE Qing Zhao
  2024-04-10 18:38   ` Joseph Myers
@ 2024-04-10 21:48   ` Siddhesh Poyarekar
  2024-04-11 13:24     ` Qing Zhao
  1 sibling, 1 reply; 34+ messages in thread
From: Siddhesh Poyarekar @ 2024-04-10 21:48 UTC (permalink / raw)
  To: Qing Zhao, josmyers, richard.guenther, uecker
  Cc: keescook, isanbard, gcc-patches

On 2024-03-29 12:07, Qing Zhao wrote:
> to carry the TYPE of the flexible array.
> 
> Such information is needed during tree-object-size.cc.
> 
> We cannot use the result type or the type of the 1st argument
> of the routine .ACCESS_WITH_SIZE to decide the element type
> of the original array due to possible type casting in the
> source code.
> 
> gcc/c/ChangeLog:
> 
> 	* c-typeck.cc (build_access_with_size_for_counted_by): Add the 6th
> 	argument to .ACCESS_WITH_SIZE.
> 
> gcc/ChangeLog:
> 
> 	* tree-object-size.cc (access_with_size_object_size): Use the type
> 	of the 6th argument for the type of the element.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/flex-array-counted-by-6.c: New test.

This version looks fine to me for stage 1, but I'm not a maintainer so 
you'll need an ack from one to commit.

Thanks,
Sid

> ---
>   gcc/c/c-typeck.cc                             | 11 +++--
>   gcc/internal-fn.cc                            |  2 +
>   .../gcc.dg/flex-array-counted-by-6.c          | 46 +++++++++++++++++++
>   gcc/tree-object-size.cc                       | 16 ++++---
>   4 files changed, 66 insertions(+), 9 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
> 
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index f7b0e08459b0..05948f76039e 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -2608,7 +2608,8 @@ build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>   
>      to:
>   
> -   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
> +   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1,
> +			(TYPE_OF_ARRAY *)0))
>   
>      NOTE: The return type of this function is the POINTER type pointing
>      to the original flexible array type.
> @@ -2620,6 +2621,9 @@ build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>      The 4th argument of the call is a constant 0 with the TYPE of the
>      object pointed by COUNTED_BY_REF.
>   
> +   The 6th argument of the call is a constant 0 with the pointer TYPE
> +   to the original flexible array type.
> +
>     */
>   static tree
>   build_access_with_size_for_counted_by (location_t loc, tree ref,
> @@ -2632,12 +2636,13 @@ build_access_with_size_for_counted_by (location_t loc, tree ref,
>   
>     tree call
>       = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
> -				    result_type, 5,
> +				    result_type, 6,
>   				    array_to_pointer_conversion (loc, ref),
>   				    counted_by_ref,
>   				    build_int_cst (integer_type_node, 1),
>   				    build_int_cst (counted_by_type, 0),
> -				    build_int_cst (integer_type_node, -1));
> +				    build_int_cst (integer_type_node, -1),
> +				    build_int_cst (result_type, 0));
>     /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
>     call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
>     SET_EXPR_LOCATION (call, loc);
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index e744080ee670..34e4a4aea534 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -3411,6 +3411,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>        1: read_only
>        2: write_only
>        3: read_write
> +   6th argument: A constant 0 with the pointer TYPE to the original flexible
> +     array type.
>   
>      Both the return type and the type of the first argument of this
>      function have been converted from the incomplete array type to
> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
> new file mode 100644
> index 000000000000..65fa01443d95
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
> @@ -0,0 +1,46 @@
> +/* Test the attribute counted_by and its usage in
> + * __builtin_dynamic_object_size: when the type of the flexible array member
> + * is casting to another type.  */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include "builtin-object-size-common.h"
> +
> +typedef unsigned short u16;
> +
> +struct info {
> +       u16 data_len;
> +       char data[] __attribute__((counted_by(data_len)));
> +};
> +
> +struct foo {
> +       int a;
> +       int b;
> +};
> +
> +static __attribute__((__noinline__))
> +struct info *setup ()
> +{
> + struct info *p;
> + size_t bytes = 3 * sizeof(struct foo);
> +
> + p = (struct info *)malloc (sizeof (struct info) + bytes);
> + p->data_len = bytes;
> +
> + return p;
> +}
> +
> +static void
> +__attribute__((__noinline__)) report (struct info *p)
> +{
> + struct foo *bar = (struct foo *)p->data;
> + EXPECT(__builtin_dynamic_object_size((char *)(bar + 1), 1), 16);
> + EXPECT(__builtin_dynamic_object_size((char *)(bar + 2), 1), 8);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + struct info *p = setup();
> + report(p);
> + return 0;
> +}
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 8de264d1dee2..4c1fa9b555fa 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -762,9 +762,11 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>        1: the number of the elements of the object type;
>      4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
>       of the object referenced by REF_TO_SIZE
> +   6th argument: A constant 0 with the pointer TYPE to the original flexible
> +     array type.
>   
> -   The size of the element can be retrived from the result type of the call,
> -   which is the pointer to the array type.  */
> +   The size of the element can be retrived from the TYPE of the 6th argument
> +   of the call, which is the pointer to the array type.  */
>   static tree
>   access_with_size_object_size (const gcall *call, int object_size_type)
>   {
> @@ -773,10 +775,12 @@ access_with_size_object_size (const gcall *call, int object_size_type)
>       return size_unknown (object_size_type);
>   
>     gcc_assert (gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE));
> -  /* Result type is a pointer type to the original flexible array type.  */
> -  tree result_type = gimple_call_return_type (call);
> -  gcc_assert (POINTER_TYPE_P (result_type));
> -  tree element_size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (result_type)));
> +  /* The type of the 6th argument type is the pointer TYPE to the original
> +     flexible array type.  */
> +  tree pointer_to_array_type = TREE_TYPE (gimple_call_arg (call, 5));
> +  gcc_assert (POINTER_TYPE_P (pointer_to_array_type));
> +  tree element_type = TREE_TYPE (TREE_TYPE (pointer_to_array_type));
> +  tree element_size = TYPE_SIZE_UNIT (element_type);
>     tree ref_to_size = gimple_call_arg (call, 1);
>     unsigned int class_of_size = TREE_INT_CST_LOW (gimple_call_arg (call, 2));
>     tree type = TREE_TYPE (gimple_call_arg (call, 3));

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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 19:21         ` Qing Zhao
@ 2024-04-10 21:56           ` Joseph Myers
  2024-04-11 13:17             ` Qing Zhao
  0 siblings, 1 reply; 34+ messages in thread
From: Joseph Myers @ 2024-04-10 21:56 UTC (permalink / raw)
  To: Qing Zhao
  Cc: uecker, Richard Biener, Siddhesh Poyarekar, Kees Cook, isanbard,
	gcc-patches

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

On Wed, 10 Apr 2024, Qing Zhao wrote:

> Okay, the above is very clear, thanks a lot for the explanation.
> So, basically, for “counted-by” attribute:
> **The following is good:
> struct f { 
>   int b;
>   int c;
>   int a[]  __attribute__ ((counted_by (b))) };
> struct f {
>   int b;
>   int c;
>   int a[] __attribute__ ((counted_by (b))) };
> 
> **The following should error:
> 
> struct f { 
>   int b;
>   int c;
>   int a[]  __attribute__ ((counted_by (b))) };
> struct f {
>   int b;
>   int c;
>   int a[] __attribute__ ((counted_by (c))) };  /* error here */
> 
> For the same tag in different scopes case:
> 
> struct f { 
>   int b;
>   int c;
>   int a[]  __attribute__ ((counted_by (b))) }  y0;
> 
> void test1(void) 
> {   
> struct f {
>   int b;
>   int c;
>   int a[] __attribute__ ((counted_by (c))) } x;
> 
>   y0 = x;  /* will report incompatible type error here */
> }
> 
> Are the above complete?

Yes, that looks like what should be tested (with the addition of the case 
of same tag, different scopes, same counted_by so compatible).

-- 
Joseph S. Myers
josmyers@redhat.com

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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 19:35         ` Qing Zhao
@ 2024-04-11  6:02           ` Martin Uecker
  2024-04-11 13:16             ` Qing Zhao
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Uecker @ 2024-04-11  6:02 UTC (permalink / raw)
  To: Qing Zhao
  Cc: Joseph Myers, Richard Biener, Siddhesh Poyarekar, Kees Cook,
	isanbard, gcc-patches

Am Mittwoch, dem 10.04.2024 um 19:35 +0000 schrieb Qing Zhao:
> 
> > On Apr 10, 2024, at 15:05, Martin Uecker <uecker@tugraz.at> wrote:
> > 
> > Am Mittwoch, dem 10.04.2024 um 20:25 +0200 schrieb Martin Uecker:
> > > Am Mittwoch, dem 10.04.2024 um 17:35 +0000 schrieb Joseph Myers:
> > > > On Fri, 29 Mar 2024, Qing Zhao wrote:
> > > > 
> > > > > +  /* Issue error when there is a counted_by attribute with a different
> > > > > +     field as the argument for the same flexible array member field.  */
> > > > 
> > > > There's another case of this to consider, though I'm not sure where best 
> > > > to check for it (Martin might have suggestions) - of course this case will 
> > > > need testcases as well.
> > > > 
> > > > Suppose, as allowed in C23, a structure is defined twice in the same 
> > > > scope, but the two definitions of the structure use inconsistent 
> > > > counted_by attributes.  I'd say that, when the declarations are in the 
> > > > same scope (thus required to be consistent), it should be an error for the 
> > > > two definitions of what is meant to be the same structure to use 
> > > > incompatible counted_by attributes (even though the member declarations 
> > > > are otherwise the same).
> > > 
> > > I think the right place could be comp_types_attributes in
> > > attributes.cc.  It may be sufficient to set the
> > > affects_type_identify flag.
> > > 
> > > This should then give a redefinition error as it should do for
> > > "packed".
> > 
> > Thinking about this a bit more, this will not work here, because
> > the counted_by attribute is not applied to the struct type but
> > one of the members.
> > 
> > So probably there should be a check added directly
> > to tagged_types_tu_compatible_p
> 
> 
> There are two cases we will check:
> 
>   A. Both definitions are in the same scope;
>       Then if the 2nd definition has a counted-by attribute different from the 1st definition, the 2nd definition will be given a redefinition error; 
> 
>   B. These two definitions are in different scope;
>       When these two definitions are used in a way need to be compatible, an incompatible error need to be issued at that
> Point;
> 
> 
> My question is, Will the routine “tagged_types_tu_compatible_p” can handle both A and B?

Yes, changing this function should address both cases if I am
not missing something.

Martin

> 
> Thanks.
> 
> Qing
> > 
> > Martin
> > 
> > > 
> > > > 
> > > > In C23 structures defined with the same tag in different scopes are 
> > > > compatible given requirements including compatible types for corresponding 
> > > > elements.  It would seem most appropriate to me for such structures with 
> > > > incompatible counted_by attributes to be considered *not* compatible types 
> > > > (but it would be valid to define structures with the same tag, different 
> > > > scopes, and elements the same except for counted_by - just not to use them 
> > > > in any way requiring them to be compatible).
> > > 
> > > Another option might be to warn about the case when those types
> > > are then used together in a way where they are required to
> > > be compatible.  Then comp_types_attributes would have to return 2.
> > > 
> > > 
> > > Martin
> > > 
> > > > 
> > > > > +The @code{counted_by} attribute may be attached to the C99 flexible array
> > > > > +member of a structure.  It indicates that the number of the elements of the
> > > > > +array is given by the field "@var{count}" in the same structure as the
> > > > 
> > > > As noted previously, the "" quotes should be removed there (or replaced by 
> > > > ``'' quotes).
> > > > 
> > > 
> > 
> 


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-11  6:02           ` Martin Uecker
@ 2024-04-11 13:16             ` Qing Zhao
  0 siblings, 0 replies; 34+ messages in thread
From: Qing Zhao @ 2024-04-11 13:16 UTC (permalink / raw)
  To: Martin Uecker
  Cc: Joseph Myers, Richard Biener, Siddhesh Poyarekar, Kees Cook,
	isanbard, gcc-patches



> On Apr 11, 2024, at 02:02, Martin Uecker <uecker@tugraz.at> wrote:
> 
> Am Mittwoch, dem 10.04.2024 um 19:35 +0000 schrieb Qing Zhao:
>> 
>>> On Apr 10, 2024, at 15:05, Martin Uecker <uecker@tugraz.at> wrote:
>>> 
>>> Am Mittwoch, dem 10.04.2024 um 20:25 +0200 schrieb Martin Uecker:
>>>> Am Mittwoch, dem 10.04.2024 um 17:35 +0000 schrieb Joseph Myers:
>>>>> On Fri, 29 Mar 2024, Qing Zhao wrote:
>>>>> 
>>>>>> +  /* Issue error when there is a counted_by attribute with a different
>>>>>> +     field as the argument for the same flexible array member field.  */
>>>>> 
>>>>> There's another case of this to consider, though I'm not sure where best 
>>>>> to check for it (Martin might have suggestions) - of course this case will 
>>>>> need testcases as well.
>>>>> 
>>>>> Suppose, as allowed in C23, a structure is defined twice in the same 
>>>>> scope, but the two definitions of the structure use inconsistent 
>>>>> counted_by attributes.  I'd say that, when the declarations are in the 
>>>>> same scope (thus required to be consistent), it should be an error for the 
>>>>> two definitions of what is meant to be the same structure to use 
>>>>> incompatible counted_by attributes (even though the member declarations 
>>>>> are otherwise the same).
>>>> 
>>>> I think the right place could be comp_types_attributes in
>>>> attributes.cc.  It may be sufficient to set the
>>>> affects_type_identify flag.
>>>> 
>>>> This should then give a redefinition error as it should do for
>>>> "packed".
>>> 
>>> Thinking about this a bit more, this will not work here, because
>>> the counted_by attribute is not applied to the struct type but
>>> one of the members.
>>> 
>>> So probably there should be a check added directly
>>> to tagged_types_tu_compatible_p
>> 
>> 
>> There are two cases we will check:
>> 
>>  A. Both definitions are in the same scope;
>>      Then if the 2nd definition has a counted-by attribute different from the 1st definition, the 2nd definition will be given a redefinition error; 
>> 
>>  B. These two definitions are in different scope;
>>      When these two definitions are used in a way need to be compatible, an incompatible error need to be issued at that
>> Point;
>> 
>> 
>> My question is, Will the routine “tagged_types_tu_compatible_p” can handle both A and B?
> 
> Yes, changing this function should address both cases if I am
> not missing something.
> 
Thanks for the help.
Will study this routine in more details and update the patch.

Qing
> Martin
> 
>> 
>> Thanks.
>> 
>> Qing
>>> 
>>> Martin
>>> 
>>>> 
>>>>> 
>>>>> In C23 structures defined with the same tag in different scopes are 
>>>>> compatible given requirements including compatible types for corresponding 
>>>>> elements.  It would seem most appropriate to me for such structures with 
>>>>> incompatible counted_by attributes to be considered *not* compatible types 
>>>>> (but it would be valid to define structures with the same tag, different 
>>>>> scopes, and elements the same except for counted_by - just not to use them 
>>>>> in any way requiring them to be compatible).
>>>> 
>>>> Another option might be to warn about the case when those types
>>>> are then used together in a way where they are required to
>>>> be compatible.  Then comp_types_attributes would have to return 2.
>>>> 
>>>> 
>>>> Martin
>>>> 
>>>>> 
>>>>>> +The @code{counted_by} attribute may be attached to the C99 flexible array
>>>>>> +member of a structure.  It indicates that the number of the elements of the
>>>>>> +array is given by the field "@var{count}" in the same structure as the
>>>>> 
>>>>> As noted previously, the "" quotes should be removed there (or replaced by 
>>>>> ``'' quotes).
>>>>> 
>>>> 
>>> 
>> 
> 


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

* Re: [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896)
  2024-04-10 21:56           ` Joseph Myers
@ 2024-04-11 13:17             ` Qing Zhao
  0 siblings, 0 replies; 34+ messages in thread
From: Qing Zhao @ 2024-04-11 13:17 UTC (permalink / raw)
  To: Joseph Myers
  Cc: uecker, Richard Biener, Siddhesh Poyarekar, Kees Cook, isanbard,
	gcc-patches



> On Apr 10, 2024, at 17:56, Joseph Myers <josmyers@redhat.com> wrote:
> 
> On Wed, 10 Apr 2024, Qing Zhao wrote:
> 
>> Okay, the above is very clear, thanks a lot for the explanation.
>> So, basically, for “counted-by” attribute:
>> **The following is good:
>> struct f { 
>>  int b;
>>  int c;
>>  int a[]  __attribute__ ((counted_by (b))) };
>> struct f {
>>  int b;
>>  int c;
>>  int a[] __attribute__ ((counted_by (b))) };
>> 
>> **The following should error:
>> 
>> struct f { 
>>  int b;
>>  int c;
>>  int a[]  __attribute__ ((counted_by (b))) };
>> struct f {
>>  int b;
>>  int c;
>>  int a[] __attribute__ ((counted_by (c))) };  /* error here */
>> 
>> For the same tag in different scopes case:
>> 
>> struct f { 
>>  int b;
>>  int c;
>>  int a[]  __attribute__ ((counted_by (b))) }  y0;
>> 
>> void test1(void) 
>> {   
>> struct f {
>>  int b;
>>  int c;
>>  int a[] __attribute__ ((counted_by (c))) } x;
>> 
>>  y0 = x;  /* will report incompatible type error here */
>> }
>> 
>> Are the above complete?
> 
> Yes, that looks like what should be tested (with the addition of the case 
> of same tag, different scopes, same counted_by so compatible).

Okay, thanks for the help.

Qing
> 
> -- 
> Joseph S. Myers
> josmyers@redhat.com


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

* Re: [PATCH v8 3/5] Use the .ACCESS_WITH_SIZE in builtin object size.
  2024-04-10 21:45   ` Siddhesh Poyarekar
@ 2024-04-11 13:19     ` Qing Zhao
  0 siblings, 0 replies; 34+ messages in thread
From: Qing Zhao @ 2024-04-11 13:19 UTC (permalink / raw)
  To: Siddhesh Poyarekar, Richard Biener
  Cc: Joseph Myers, Richard Biener, uecker, Kees Cook, isanbard, gcc-patches

Sid,

Thanks a lot for your review!


> On Apr 10, 2024, at 17:45, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> 
> On 2024-03-29 12:07, Qing Zhao wrote:
>> gcc/ChangeLog:
>> 	* tree-object-size.cc (access_with_size_object_size): New function.
>> 	(call_object_size): Call the new function.
>> gcc/testsuite/ChangeLog:
>> 	* gcc.dg/builtin-object-size-common.h: Add a new macro EXPECT.
>> 	* gcc.dg/flex-array-counted-by-3.c: New test.
>> 	* gcc.dg/flex-array-counted-by-4.c: New test.
>> 	* gcc.dg/flex-array-counted-by-5.c: New test.
> 
> This version looks fine to me for stage 1, but I'm not a maintainer so you'll need an ack from one to commit.

Richard, 

Could you please comment on this patch? Is this one okay for stage 1?

Thanks

Qing


> 
> Thanks,
> Sid
> 
>> ---
>>  .../gcc.dg/builtin-object-size-common.h       |  11 ++
>>  .../gcc.dg/flex-array-counted-by-3.c          |  63 +++++++
>>  .../gcc.dg/flex-array-counted-by-4.c          | 178 ++++++++++++++++++
>>  .../gcc.dg/flex-array-counted-by-5.c          |  48 +++++
>>  gcc/tree-object-size.cc                       |  60 ++++++
>>  5 files changed, 360 insertions(+)
>>  create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>>  create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>>  create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-common.h b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
>> index 66ff7cdd953a..b677067c6e6b 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-common.h
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
>> @@ -30,3 +30,14 @@ unsigned nfails = 0;
>>        __builtin_abort ();						      \
>>      return 0;								      \
>>    } while (0)
>> +
>> +#define EXPECT(p, _v) do {						      \
>> +  size_t v = _v;							      \
>> +  if (p == v)								      \
>> +    __builtin_printf ("ok:  %s == %zd\n", #p, p);			      \
>> +  else									      \
>> +    {									      \
>> +      __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v);	      \
>> +      FAIL ();								      \
>> +    }									      \
>> +} while (0);
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>> new file mode 100644
>> index 000000000000..78f50230e891
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
>> @@ -0,0 +1,63 @@
>> +/* Test the attribute counted_by and its usage in
>> + * __builtin_dynamic_object_size.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-O2" } */
>> +
>> +#include "builtin-object-size-common.h"
>> +
>> +struct flex {
>> +  int b;
>> +  int c[];
>> +} *array_flex;
>> +
>> +struct annotated {
>> +  int b;
>> +  int c[] __attribute__ ((counted_by (b)));
>> +} *array_annotated;
>> +
>> +struct nested_annotated {
>> +  struct {
>> +    union {
>> +      int b;
>> +      float f;	
>> +    };
>> +    int n;
>> +  };
>> +  int c[] __attribute__ ((counted_by (b)));
>> +} *array_nested_annotated;
>> +
>> +void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
>> +{
>> +  array_flex
>> +    = (struct flex *)malloc (sizeof (struct flex)
>> +			     + normal_count *  sizeof (int));
>> +  array_flex->b = normal_count;
>> +
>> +  array_annotated
>> +    = (struct annotated *)malloc (sizeof (struct annotated)
>> +				  + attr_count *  sizeof (int));
>> +  array_annotated->b = attr_count;
>> +
>> +  array_nested_annotated
>> +    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
>> +					 + attr_count *  sizeof (int));
>> +  array_nested_annotated->b = attr_count;
>> +
>> +  return;
>> +}
>> +
>> +void __attribute__((__noinline__)) test ()
>> +{
>> +    EXPECT(__builtin_dynamic_object_size(array_flex->c, 1), -1);
>> +    EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1),
>> +	   array_annotated->b * sizeof (int));
>> +    EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1),
>> +	   array_nested_annotated->b * sizeof (int));
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +  setup (10,10);
>> +  test ();
>> +  DONE ();
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>> new file mode 100644
>> index 000000000000..20103d58ef51
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
>> @@ -0,0 +1,178 @@
>> +/* Test the attribute counted_by and its usage in
>> +__builtin_dynamic_object_size: what's the correct behavior when the
>> +allocation size mismatched with the value of counted_by attribute?
>> +We should always use the latest value that is hold by the counted_by
>> +field.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-O -fstrict-flex-arrays=3" } */
>> +
>> +#include "builtin-object-size-common.h"
>> +
>> +struct annotated {
>> +  size_t foo;
>> +  char others;
>> +  char array[] __attribute__((counted_by (foo)));
>> +};
>> +
>> +#define noinline __attribute__((__noinline__))
>> +#define SIZE_BUMP 10
>> +#define MAX(a, b) ((a) > (b) ? (a) : (b))
>> +
>> +/* In general, Due to type casting, the type for the pointee of a pointer
>> +   does not say anything about the object it points to,
>> +   So, __builtin_object_size can not directly use the type of the pointee
>> +   to decide the size of the object the pointer points to.
>> +
>> +   There are only two reliable ways:
>> +   A. observed allocations  (call to the allocation functions in the routine)
>> +   B. observed accesses     (read or write access to the location of the
>> +                             pointer points to)
>> +
>> +   That provide information about the type/existence of an object at
>> +   the corresponding address.
>> +
>> +   For A, we use the "alloc_size" attribute for the corresponding allocation
>> +   functions to determine the object size;
>> +   (We treat counted_by attribute the same as the "alloc_size" attribute)
>> +
>> +   For B, we use the SIZE info of the TYPE attached to the corresponding access.
>> +
>> +   The only other way in C which ensures that a pointer actually points
>> +   to an object of the correct type is 'static':
>> +
>> +   void foo(struct P *p[static 1]);
>> +
>> +   See https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624814.html
>> +   for more details.  */
>> +
>> +/* In the following function, malloc allocated more space than the value
>> +   of counted_by attribute.  Then what's the correct behavior we expect
>> +   the __builtin_dynamic_object_size should have for each of the cases?  */
>> +
>> +static struct annotated * noinline alloc_buf_more (size_t index)
>> +{
>> +  struct annotated *p;
>> +  size_t allocated_size
>> +    = MAX (sizeof (struct annotated),
>> +	   (__builtin_offsetof (struct annotated, array[0])
>> +	    + (index + SIZE_BUMP) * sizeof (char)));
>> +  p = (struct annotated *) malloc (allocated_size);
>> +
>> +  p->foo = index;
>> +
>> +  /* When checking the observed access p->array, we have info on both
>> +    observered allocation and observed access,
>> +    A.1 from observed allocation:
>> +    	allocated_size - offsetof (struct annotated, array[0])
>> +
>> +    A.2 from the counted-by attribute:
>> +    	p->foo * sizeof (char)
>> +
>> +    We always use the latest value that is hold by the counted-by field.
>> +   */
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 0),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 1),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 2),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 3),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  /* When checking the pointer p, we only have info on the observed allocation.
>> +    So, the object size info can only been obtained from the call to malloc.
>> +    For both MAXIMUM and MINIMUM: A = (index + SIZE_BUMP) * sizeof (char)  */
>> +  EXPECT(__builtin_dynamic_object_size(p, 0), allocated_size);
>> +  EXPECT(__builtin_dynamic_object_size(p, 1), allocated_size);
>> +  EXPECT(__builtin_dynamic_object_size(p, 2), allocated_size);
>> +  EXPECT(__builtin_dynamic_object_size(p, 3), allocated_size);
>> +  return p;
>> +}
>> +
>> +/* In the following function, malloc allocated less space than the value
>> +   of counted_by attribute.  Then what's the correct behavior we expect
>> +   the __builtin_dynamic_object_size should have for each of the cases?
>> +   NOTE: this is an user error, GCC should issue warnings for such case.
>> +   This is a seperate issue we should address later.  */
>> +
>> +static struct annotated * noinline alloc_buf_less (size_t index)
>> +{
>> +  struct annotated *p;
>> +  size_t allocated_size
>> +    = MAX (sizeof (struct annotated),
>> +	   (__builtin_offsetof (struct annotated, array[0])
>> +	    + (index) * sizeof (char)));
>> +  p = (struct annotated *) malloc (allocated_size);
>> +
>> +  p->foo = index + SIZE_BUMP;
>> +
>> +  /* When checking the observed access p->array, we have info on both
>> +    observered allocation and observed access,
>> +    A.1 from observed allocation:
>> +    	allocated_size - offsetof (struct annotated, array[0])
>> +    A.2 from the counted-by attribute:
>> +    	p->foo * sizeof (char)
>> +
>> +    We always use the latest value that is hold by the counted-by field.
>> +   */
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 0),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 1),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 2),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 3),
>> +	 (p->foo) * sizeof(char));
>> +
>> +  /* When checking the pointer p, we only have info on the observed
>> +    allocation. So, the object size info can only been obtained from
>> +    the call to malloc.  */
>> +  EXPECT(__builtin_dynamic_object_size(p, 0), allocated_size);
>> +  EXPECT(__builtin_dynamic_object_size(p, 1), allocated_size);
>> +  EXPECT(__builtin_dynamic_object_size(p, 2), allocated_size);
>> +  EXPECT(__builtin_dynamic_object_size(p, 3), allocated_size);
>> +  return p;
>> +}
>> +
>> +int main ()
>> +{
>> +  struct annotated *p, *q;
>> +  p = alloc_buf_more (10);
>> +  q = alloc_buf_less (10);
>> +
>> +  /* When checking the access p->array, we only have info on the counted-by
>> +    value.  */
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 0), p->foo * sizeof(char));
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 1), p->foo * sizeof(char));
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 2), p->foo * sizeof(char));
>> +  EXPECT(__builtin_dynamic_object_size(p->array, 3), p->foo * sizeof(char));
>> +  /* When checking the pointer p, we have no observed allocation nor observed
>> +    access, therefore, we cannot determine the size info here.  */
>> +  EXPECT(__builtin_dynamic_object_size(p, 0), -1);
>> +  EXPECT(__builtin_dynamic_object_size(p, 1), -1);
>> +  EXPECT(__builtin_dynamic_object_size(p, 2), 0);
>> +  EXPECT(__builtin_dynamic_object_size(p, 3), 0);
>> +
>> +  /* When checking the access p->array, we only have info on the counted-by
>> +    value.  */
>> +  EXPECT(__builtin_dynamic_object_size(q->array, 0), q->foo * sizeof(char));
>> +  EXPECT(__builtin_dynamic_object_size(q->array, 1), q->foo * sizeof(char));
>> +  EXPECT(__builtin_dynamic_object_size(q->array, 2), q->foo * sizeof(char));
>> +  EXPECT(__builtin_dynamic_object_size(q->array, 3), q->foo * sizeof(char));
>> +  /* When checking the pointer p, we have no observed allocation nor observed
>> +    access, therefore, we cannot determine the size info here.  */
>> +  EXPECT(__builtin_dynamic_object_size(q, 0), -1);
>> +  EXPECT(__builtin_dynamic_object_size(q, 1), -1);
>> +  EXPECT(__builtin_dynamic_object_size(q, 2), 0);
>> +  EXPECT(__builtin_dynamic_object_size(q, 3), 0);
>> +
>> +  DONE ();
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
>> new file mode 100644
>> index 000000000000..68f9b0f7c8d2
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-5.c
>> @@ -0,0 +1,48 @@
>> +/* Test the attribute counted_by and its usage in
>> + * __builtin_dynamic_object_size: when the counted_by field is negative.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-O2" } */
>> +
>> +#include "builtin-object-size-common.h"
>> +
>> +struct annotated {
>> +  int b;
>> +  int c[] __attribute__ ((counted_by (b)));
>> +} *array_annotated;
>> +
>> +struct nested_annotated {
>> +  struct {
>> +    union {
>> +      int b;
>> +      float f;	
>> +    };
>> +    int n;
>> +  };
>> +  int c[] __attribute__ ((counted_by (b)));
>> +} *array_nested_annotated;
>> +
>> +void __attribute__((__noinline__)) setup (int attr_count)
>> +{
>> +  array_annotated
>> +    = (struct annotated *)malloc (sizeof (struct annotated));
>> +  array_annotated->b = attr_count;
>> +
>> +  array_nested_annotated
>> +    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated));
>> +  array_nested_annotated->b = attr_count -1;
>> +
>> +  return;
>> +}
>> +
>> +void __attribute__((__noinline__)) test ()
>> +{
>> +    EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1), 0);
>> +    EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1), 0);
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +  setup (-10);
>> +  test ();
>> +  DONE ();
>> +}
>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>> index 018fbc30cbb6..8de264d1dee2 100644
>> --- a/gcc/tree-object-size.cc
>> +++ b/gcc/tree-object-size.cc
>> @@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "attribs.h"
>>  #include "builtins.h"
>>  #include "gimplify-me.h"
>> +#include "gimplify.h"
>>    struct object_size_info
>>  {
>> @@ -60,6 +61,7 @@ static tree compute_object_offset (tree, const_tree);
>>  static bool addr_object_size (struct object_size_info *,
>>  			      const_tree, int, tree *, tree *t = NULL);
>>  static tree alloc_object_size (const gcall *, int);
>> +static tree access_with_size_object_size (const gcall *, int);
>>  static tree pass_through_call (const gcall *);
>>  static void collect_object_sizes_for (struct object_size_info *, tree);
>>  static void expr_object_size (struct object_size_info *, tree, tree);
>> @@ -749,6 +751,60 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>>    return false;
>>  }
>>  +/* Compute __builtin_object_size for a CALL to .ACCESS_WITH_SIZE,
>> +   OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>> +   The 2nd, 3rd, and the 4th parameters of the call determine the size of
>> +   the CALL:
>> +
>> +   2nd argument REF_TO_SIZE: The reference to the size of the object,
>> +   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
>> +     0: the number of bytes;
>> +     1: the number of the elements of the object type;
>> +   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
>> +    of the object referenced by REF_TO_SIZE
>> +
>> +   The size of the element can be retrived from the result type of the call,
>> +   which is the pointer to the array type.  */
>> +static tree
>> +access_with_size_object_size (const gcall *call, int object_size_type)
>> +{
>> +  /* If not for dynamic object size, return.  */
>> +  if ((object_size_type & OST_DYNAMIC) == 0)
>> +    return size_unknown (object_size_type);
>> +
>> +  gcc_assert (gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE));
>> +  /* Result type is a pointer type to the original flexible array type.  */
>> +  tree result_type = gimple_call_return_type (call);
>> +  gcc_assert (POINTER_TYPE_P (result_type));
>> +  tree element_size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (result_type)));
>> +  tree ref_to_size = gimple_call_arg (call, 1);
>> +  unsigned int class_of_size = TREE_INT_CST_LOW (gimple_call_arg (call, 2));
>> +  tree type = TREE_TYPE (gimple_call_arg (call, 3));
>> +
>> +  tree size = fold_build2 (MEM_REF, type, ref_to_size,
>> +			   build_int_cst (ptr_type_node, 0));
>> +
>> +  /* If size is negative value, treat it as zero.  */
>> +  if (!TYPE_UNSIGNED (type))
>> +  {
>> +    tree cond_expr = fold_build2 (LT_EXPR, boolean_type_node,
>> +				  unshare_expr (size), build_zero_cst (type));
>> +    size = fold_build3 (COND_EXPR, integer_type_node, cond_expr,
>> +			build_zero_cst (type), size);
>> +  }
>> +
>> +  if (class_of_size == 1)
>> +    size = size_binop (MULT_EXPR,
>> +		       fold_convert (sizetype, size),
>> +		       fold_convert (sizetype, element_size));
>> +  else
>> +    size = fold_convert (sizetype, size);
>> +
>> +  if (!todo)
>> +    todo = TODO_update_ssa_only_virtuals;
>> +
>> +  return size;
>> +}
>>    /* Compute __builtin_object_size for CALL, which is a GIMPLE_CALL.
>>     Handles calls to functions declared with attribute alloc_size.
>> @@ -1350,8 +1406,12 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
>>      bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
>>    bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
>> +  bool is_access_with_size
>> +	 = gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE);
>>    if (is_strdup || is_strndup)
>>      bytes = strdup_object_size (call, object_size_type, is_strndup);
>> +  else if (is_access_with_size)
>> +    bytes = access_with_size_object_size (call, object_size_type);
>>    else
>>      bytes = alloc_object_size (call, object_size_type);
>>  


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

* Re: [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer.
  2024-04-10 21:46   ` Siddhesh Poyarekar
@ 2024-04-11 13:22     ` Qing Zhao
  0 siblings, 0 replies; 34+ messages in thread
From: Qing Zhao @ 2024-04-11 13:22 UTC (permalink / raw)
  To: Siddhesh Poyarekar
  Cc: Joseph Myers, Richard Biener, uecker, Kees Cook, isanbard, gcc-patches

Sid,

Thanks a lot for the review.


> On Apr 10, 2024, at 17:46, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> 
> On 2024-03-29 12:07, Qing Zhao wrote:
>> gcc/c-family/ChangeLog:
>> 	* c-ubsan.cc (get_bound_from_access_with_size): New function.
>> 	(ubsan_instrument_bounds): Handle call to .ACCESS_WITH_SIZE.
>> gcc/testsuite/ChangeLog:
>> 	* gcc.dg/ubsan/flex-array-counted-by-bounds-2.c: New test.
>> 	* gcc.dg/ubsan/flex-array-counted-by-bounds-3.c: New test.
>> 	* gcc.dg/ubsan/flex-array-counted-by-bounds-4.c: New test.
>> 	* gcc.dg/ubsan/flex-array-counted-by-bounds.c: New test.
>> ---
> 
> This version looks fine to me for stage 1, but I'm not a maintainer so you'll need an ack from one to commit.

This patch is purely C FE changes. Joseph already approved it.

thanks.

Qing
> 
> Thanks,
> Sid
> 
>>  gcc/c-family/c-ubsan.cc                       | 42 +++++++++++++++++
>>  .../ubsan/flex-array-counted-by-bounds-2.c    | 45 ++++++++++++++++++
>>  .../ubsan/flex-array-counted-by-bounds-3.c    | 34 ++++++++++++++
>>  .../ubsan/flex-array-counted-by-bounds-4.c    | 34 ++++++++++++++
>>  .../ubsan/flex-array-counted-by-bounds.c      | 46 +++++++++++++++++++
>>  5 files changed, 201 insertions(+)
>>  create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
>>  create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
>>  create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
>>  create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
>> diff --git a/gcc/c-family/c-ubsan.cc b/gcc/c-family/c-ubsan.cc
>> index 940982819ddf..7cd3c6aa5b88 100644
>> --- a/gcc/c-family/c-ubsan.cc
>> +++ b/gcc/c-family/c-ubsan.cc
>> @@ -376,6 +376,40 @@ ubsan_instrument_return (location_t loc)
>>    return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data));
>>  }
>>  +/* Get the tree that represented the number of counted_by, i.e, the maximum
>> +   number of the elements of the object that the call to .ACCESS_WITH_SIZE
>> +   points to, this number will be the bound of the corresponding array.  */
>> +static tree
>> +get_bound_from_access_with_size (tree call)
>> +{
>> +  if (!is_access_with_size_p (call))
>> +    return NULL_TREE;
>> +
>> +  tree ref_to_size = CALL_EXPR_ARG (call, 1);
>> +  unsigned int class_of_size = TREE_INT_CST_LOW (CALL_EXPR_ARG (call, 2));
>> +  tree type = TREE_TYPE (CALL_EXPR_ARG (call, 3));
>> +  tree size = fold_build2 (MEM_REF, type, unshare_expr (ref_to_size),
>> +			   build_int_cst (ptr_type_node, 0));
>> +  /* If size is negative value, treat it as zero.  */
>> +  if (!TYPE_UNSIGNED (type))
>> +  {
>> +    tree cond = fold_build2 (LT_EXPR, boolean_type_node,
>> +			     unshare_expr (size), build_zero_cst (type));
>> +    size = fold_build3 (COND_EXPR, type, cond,
>> +			build_zero_cst (type), size);
>> +  }
>> +
>> +  /* Only when class_of_size is 1, i.e, the number of the elements of
>> +     the object type, return the size.  */
>> +  if (class_of_size != 1)
>> +    return NULL_TREE;
>> +  else
>> +    size = fold_convert (sizetype, size);
>> +
>> +  return size;
>> +}
>> +
>> +
>>  /* Instrument array bounds for ARRAY_REFs.  We create special builtin,
>>     that gets expanded in the sanopt pass, and make an array dimension
>>     of it.  ARRAY is the array, *INDEX is an index to the array.
>> @@ -401,6 +435,14 @@ ubsan_instrument_bounds (location_t loc, tree array, tree *index,
>>  	  && COMPLETE_TYPE_P (type)
>>  	  && integer_zerop (TYPE_SIZE (type)))
>>  	bound = build_int_cst (TREE_TYPE (TYPE_MIN_VALUE (domain)), -1);
>> +      else if (INDIRECT_REF_P (array)
>> +	       && is_access_with_size_p ((TREE_OPERAND (array, 0))))
>> +	{
>> +	  bound = get_bound_from_access_with_size ((TREE_OPERAND (array, 0)));
>> +	  bound = fold_build2 (MINUS_EXPR, TREE_TYPE (bound),
>> +			       bound,
>> +			       build_int_cst (TREE_TYPE (bound), 1));
>> +	}
>>        else
>>  	return NULL_TREE;
>>      }
>> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
>> new file mode 100644
>> index 000000000000..b503320628d2
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
>> @@ -0,0 +1,45 @@
>> +/* Test the attribute counted_by and its usage in
>> +   bounds sanitizer combined with VLA.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-fsanitize=bounds" } */
>> +/* { dg-output "index 11 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "\[^\n\r]*index 20 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "\[^\n\r]*index 11 out of bounds for type 'int \\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "\[^\n\r]*index 10 out of bounds for type 'int \\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
>> +
>> +
>> +#include <stdlib.h>
>> +
>> +void __attribute__((__noinline__)) setup_and_test_vla (int n, int m)
>> +{
>> +   struct foo {
>> +       int n;
>> +       int p[][n] __attribute__((counted_by(n)));
>> +   } *f;
>> +
>> +   f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n]));
>> +   f->n = m;
>> +   f->p[m][n-1]=1;
>> +   return;
>> +}
>> +
>> +void __attribute__((__noinline__)) setup_and_test_vla_1 (int n1, int n2, int m)
>> +{
>> +  struct foo {
>> +    int n;
>> +    int p[][n2][n1] __attribute__((counted_by(n)));
>> +  } *f;
>> +
>> +  f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n2][n1]));
>> +  f->n = m;
>> +  f->p[m][n2][n1]=1;
>> +  return;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +  setup_and_test_vla (10, 11);
>> +  setup_and_test_vla_1 (10, 11, 20);
>> +  return 0;
>> +}
>> +
>> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
>> new file mode 100644
>> index 000000000000..9da25644af3e
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
>> @@ -0,0 +1,34 @@
>> +/* Test the attribute counted_by and its usage in bounds
>> +   sanitizer. when counted_by field is negative value.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-fsanitize=bounds" } */
>> +
>> +#include <stdlib.h>
>> +
>> +struct annotated {
>> +  int b;
>> +  int c[] __attribute__ ((counted_by (b)));
>> +} *array_annotated;
>> +
>> +void __attribute__((__noinline__)) setup (int annotated_count)
>> +{
>> +  array_annotated
>> +    = (struct annotated *)malloc (sizeof (struct annotated));
>> +  array_annotated->b = annotated_count;
>> +
>> +  return;
>> +}
>> +
>> +void __attribute__((__noinline__)) test (int annotated_index)
>> +{
>> +  array_annotated->c[annotated_index] = 2;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +  setup (-3);
>> +  test (2);
>> +  return 0;
>> +}
>> +
>> +/* { dg-output "24:21: runtime error: index 2 out of bounds for type" } */
>> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
>> new file mode 100644
>> index 000000000000..bd7e144274fc
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-4.c
>> @@ -0,0 +1,34 @@
>> +/* Test the attribute counted_by and its usage in bounds
>> +   sanitizer. when counted_by field is zero value.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-fsanitize=bounds" } */
>> +
>> +#include <stdlib.h>
>> +
>> +struct annotated {
>> +  int b;
>> +  int c[] __attribute__ ((counted_by (b)));
>> +} *array_annotated;
>> +
>> +void __attribute__((__noinline__)) setup (int annotated_count)
>> +{
>> +  array_annotated
>> +    = (struct annotated *)malloc (sizeof (struct annotated));
>> +  array_annotated->b = annotated_count;
>> +
>> +  return;
>> +}
>> +
>> +void __attribute__((__noinline__)) test (int annotated_index)
>> +{
>> +  array_annotated->c[annotated_index] = 2;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +  setup (0);
>> +  test (1);
>> +  return 0;
>> +}
>> +
>> +/* { dg-output "24:21: runtime error: index 1 out of bounds for type" } */
>> diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
>> new file mode 100644
>> index 000000000000..e2b911dde626
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
>> @@ -0,0 +1,46 @@
>> +/* Test the attribute counted_by and its usage in
>> +   bounds sanitizer.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-fsanitize=bounds" } */
>> +
>> +#include <stdlib.h>
>> +
>> +struct flex {
>> +  int b;
>> +  int c[];
>> +} *array_flex;
>> +
>> +struct annotated {
>> +  int b;
>> +  int c[] __attribute__ ((counted_by (b)));
>> +} *array_annotated;
>> +
>> +void __attribute__((__noinline__)) setup (int normal_count, int annotated_count)
>> +{
>> +  array_flex
>> +    = (struct flex *)malloc (sizeof (struct flex)
>> +			     + normal_count *  sizeof (int));
>> +  array_flex->b = normal_count;
>> +
>> +  array_annotated
>> +    = (struct annotated *)malloc (sizeof (struct annotated)
>> +				  + annotated_count *  sizeof (int));
>> +  array_annotated->b = annotated_count;
>> +
>> +  return;
>> +}
>> +
>> +void __attribute__((__noinline__)) test (int normal_index, int annotated_index)
>> +{
>> +  array_flex->c[normal_index] = 1;
>> +  array_annotated->c[annotated_index] = 2;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +  setup (10, 10);
>> +  test (10, 10);
>> +  return 0;
>> +}
>> +
>> +/* { dg-output "36:21: runtime error: index 10 out of bounds for type" } */


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

* Re: [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE
  2024-04-10 21:48   ` Siddhesh Poyarekar
@ 2024-04-11 13:24     ` Qing Zhao
  0 siblings, 0 replies; 34+ messages in thread
From: Qing Zhao @ 2024-04-11 13:24 UTC (permalink / raw)
  To: Siddhesh Poyarekar, Richard Biener
  Cc: Joseph Myers, Richard Biener, uecker, Kees Cook, isanbard, gcc-patches

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

Sid,

Thanks a lot for your review.

Richard,

Could you please comment on this patch? Is the middle-end part Okay for stage 1?

Thanks.

Qing

On Apr 10, 2024, at 17:48, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:

On 2024-03-29 12:07, Qing Zhao wrote:
to carry the TYPE of the flexible array.
Such information is needed during tree-object-size.cc.
We cannot use the result type or the type of the 1st argument
of the routine .ACCESS_WITH_SIZE to decide the element type
of the original array due to possible type casting in the
source code.
gcc/c/ChangeLog:
* c-typeck.cc (build_access_with_size_for_counted_by): Add the 6th
argument to .ACCESS_WITH_SIZE.
gcc/ChangeLog:
* tree-object-size.cc (access_with_size_object_size): Use the type
of the 6th argument for the type of the element.
gcc/testsuite/ChangeLog:
* gcc.dg/flex-array-counted-by-6.c: New test.

This version looks fine to me for stage 1, but I'm not a maintainer so you'll need an ack from one to commit.

Thanks,
Sid

---
 gcc/c/c-typeck.cc                             | 11 +++--
 gcc/internal-fn.cc                            |  2 +
 .../gcc.dg/flex-array-counted-by-6.c          | 46 +++++++++++++++++++
 gcc/tree-object-size.cc                       | 16 ++++---
 4 files changed, 66 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index f7b0e08459b0..05948f76039e 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -2608,7 +2608,8 @@ build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
      to:
 -   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
+   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1,
+ (TYPE_OF_ARRAY *)0))
      NOTE: The return type of this function is the POINTER type pointing
    to the original flexible array type.
@@ -2620,6 +2621,9 @@ build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
    The 4th argument of the call is a constant 0 with the TYPE of the
    object pointed by COUNTED_BY_REF.
 +   The 6th argument of the call is a constant 0 with the pointer TYPE
+   to the original flexible array type.
+
   */
 static tree
 build_access_with_size_for_counted_by (location_t loc, tree ref,
@@ -2632,12 +2636,13 @@ build_access_with_size_for_counted_by (location_t loc, tree ref,
     tree call
     = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
-     result_type, 5,
+     result_type, 6,
      array_to_pointer_conversion (loc, ref),
      counted_by_ref,
      build_int_cst (integer_type_node, 1),
      build_int_cst (counted_by_type, 0),
-     build_int_cst (integer_type_node, -1));
+     build_int_cst (integer_type_node, -1),
+     build_int_cst (result_type, 0));
   /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
   call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
   SET_EXPR_LOCATION (call, loc);
diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
index e744080ee670..34e4a4aea534 100644
--- a/gcc/internal-fn.cc
+++ b/gcc/internal-fn.cc
@@ -3411,6 +3411,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
      1: read_only
      2: write_only
      3: read_write
+   6th argument: A constant 0 with the pointer TYPE to the original flexible
+     array type.
      Both the return type and the type of the first argument of this
    function have been converted from the incomplete array type to
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
new file mode 100644
index 000000000000..65fa01443d95
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-6.c
@@ -0,0 +1,46 @@
+/* Test the attribute counted_by and its usage in
+ * __builtin_dynamic_object_size: when the type of the flexible array member
+ * is casting to another type.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+typedef unsigned short u16;
+
+struct info {
+       u16 data_len;
+       char data[] __attribute__((counted_by(data_len)));
+};
+
+struct foo {
+       int a;
+       int b;
+};
+
+static __attribute__((__noinline__))
+struct info *setup ()
+{
+ struct info *p;
+ size_t bytes = 3 * sizeof(struct foo);
+
+ p = (struct info *)malloc (sizeof (struct info) + bytes);
+ p->data_len = bytes;
+
+ return p;
+}
+
+static void
+__attribute__((__noinline__)) report (struct info *p)
+{
+ struct foo *bar = (struct foo *)p->data;
+ EXPECT(__builtin_dynamic_object_size((char *)(bar + 1), 1), 16);
+ EXPECT(__builtin_dynamic_object_size((char *)(bar + 2), 1), 8);
+}
+
+int main(int argc, char *argv[])
+{
+ struct info *p = setup();
+ report(p);
+ return 0;
+}
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 8de264d1dee2..4c1fa9b555fa 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -762,9 +762,11 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
      1: the number of the elements of the object type;
    4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
     of the object referenced by REF_TO_SIZE
+   6th argument: A constant 0 with the pointer TYPE to the original flexible
+     array type.
 -   The size of the element can be retrived from the result type of the call,
-   which is the pointer to the array type.  */
+   The size of the element can be retrived from the TYPE of the 6th argument
+   of the call, which is the pointer to the array type.  */
 static tree
 access_with_size_object_size (const gcall *call, int object_size_type)
 {
@@ -773,10 +775,12 @@ access_with_size_object_size (const gcall *call, int object_size_type)
     return size_unknown (object_size_type);
     gcc_assert (gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE));
-  /* Result type is a pointer type to the original flexible array type.  */
-  tree result_type = gimple_call_return_type (call);
-  gcc_assert (POINTER_TYPE_P (result_type));
-  tree element_size = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (result_type)));
+  /* The type of the 6th argument type is the pointer TYPE to the original
+     flexible array type.  */
+  tree pointer_to_array_type = TREE_TYPE (gimple_call_arg (call, 5));
+  gcc_assert (POINTER_TYPE_P (pointer_to_array_type));
+  tree element_type = TREE_TYPE (TREE_TYPE (pointer_to_array_type));
+  tree element_size = TYPE_SIZE_UNIT (element_type);
   tree ref_to_size = gimple_call_arg (call, 1);
   unsigned int class_of_size = TREE_INT_CST_LOW (gimple_call_arg (call, 2));
   tree type = TREE_TYPE (gimple_call_arg (call, 3));


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

* Re: [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE.
  2024-04-10 18:36   ` Joseph Myers
  2024-04-10 19:38     ` Qing Zhao
@ 2024-04-11 13:27     ` Qing Zhao
  1 sibling, 0 replies; 34+ messages in thread
From: Qing Zhao @ 2024-04-11 13:27 UTC (permalink / raw)
  To: Siddhesh Poyarekar, Richard Biener, Joseph Myers
  Cc: uecker, Kees Cook, isanbard, gcc-patches

Sid and Richard,

The C FE changes in this patch has been approved by Joseph.

Could you please review the Middle-end change?

thanks.

Qing

> On Apr 10, 2024, at 14:36, Joseph Myers <josmyers@redhat.com> wrote:
> 
> On Fri, 29 Mar 2024, Qing Zhao wrote:
> 
>> +/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
>> +   the object that represents its counted_by per the attribute counted_by
>> +   attached to this field if it's a flexible array member field, otherwise
>> +   return NULL_TREE.
>> +   set COUNTED_BY_TYPE to the TYPE of the counted_by field.
> 
> Use an uppercase letter at the start of a sentence, "Set".
> 
>> +static tree
>> +build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>> +{
>> +  tree type = TREE_TYPE (datum);
>> +  if (!(c_flexible_array_member_type_p (TREE_TYPE (subdatum))))
>> +    return NULL_TREE;
> 
> There are redundant parentheses here around the call to 
> c_flexible_array_member_type_p.
> 
> The C front-end changes in this patch are OK for GCC 15 (after GCC 14 has 
> branched, and once a version of patch 1 has also been approved) with those 
> fixes.
> 
> -- 
> Joseph S. Myers
> josmyers@redhat.com
> 


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

end of thread, other threads:[~2024-04-11 13:27 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-29 16:06 [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Qing Zhao
2024-03-29 16:06 ` [PATCH v8 1/5] Provide counted_by attribute to flexible array member field (PR108896) Qing Zhao
2024-04-10 17:35   ` Joseph Myers
2024-04-10 18:05     ` Qing Zhao
2024-04-10 18:44       ` Joseph Myers
2024-04-10 19:21         ` Qing Zhao
2024-04-10 21:56           ` Joseph Myers
2024-04-11 13:17             ` Qing Zhao
2024-04-10 18:25     ` Martin Uecker
2024-04-10 19:05       ` Martin Uecker
2024-04-10 19:35         ` Qing Zhao
2024-04-11  6:02           ` Martin Uecker
2024-04-11 13:16             ` Qing Zhao
2024-03-29 16:07 ` [PATCH v8 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE Qing Zhao
2024-04-10 18:36   ` Joseph Myers
2024-04-10 19:38     ` Qing Zhao
2024-04-11 13:27     ` Qing Zhao
2024-03-29 16:07 ` [PATCH v8 3/5] Use the .ACCESS_WITH_SIZE in builtin object size Qing Zhao
2024-04-10 21:45   ` Siddhesh Poyarekar
2024-04-11 13:19     ` Qing Zhao
2024-03-29 16:07 ` [PATCH v8 4/5] Use the .ACCESS_WITH_SIZE in bound sanitizer Qing Zhao
2024-04-10 18:37   ` Joseph Myers
2024-04-10 21:46   ` Siddhesh Poyarekar
2024-04-11 13:22     ` Qing Zhao
2024-03-29 16:07 ` [PATCH v8 5/5] Add the 6th argument to .ACCESS_WITH_SIZE Qing Zhao
2024-04-10 18:38   ` Joseph Myers
2024-04-10 21:48   ` Siddhesh Poyarekar
2024-04-11 13:24     ` Qing Zhao
2024-03-29 18:09 ` [PATCH v8 0/5] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Tom Tromey
2024-03-29 19:16   ` Kees Cook
2024-03-29 19:58     ` Qing Zhao
2024-03-30  0:16       ` Tom Tromey
2024-03-30  0:15     ` Tom Tromey
2024-03-30 13:57 ` Kees Cook

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).