Re: [PATCH v6 1/5] Provide counted_by attribute to flexible array member field (PR108896)
Sid,
Thanks a lot for your time to review the code.
See my reply below:
On Mar 11, 2024, at 10:57, Siddhesh Poyarekar wrote:
On 2024-02-16 14:47, Qing Zhao wrote:
return true;
else
return targetm.attribute_takes_identifier_p (attr_id);
@@ -2806,6 +2811,53 @@ 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);
+
+ /* This attribute only applies to field decls of a structure. */
+ if (TREE_CODE (decl) != FIELD_DECL)
+{
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "%qE attribute may not be specified for 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 may not be specified 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 may not be specified for a non"
+ " flexible array member field",
+ name);
+ *no_add_attrs = true;
+}
How about "not allowed" instead of "may not be specified"?
Okay, will update them.
+ /* The argument should be an identifier. */
+ else if (TREE_CODE (argval) != IDENTIFIER_NODE)
+{
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "% argument not an identifier");
+ *no_add_attrs = true;
+}
Validate that the attribute only applies to a C99 flexible array member of a
structure and the argument should be an identifier node. OK.
verify_counted_by_attribute does more extensive validation on argval.
Yes.
+
+ 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..56d828e3dfaf 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 a ISO C99 flexible array memeber type "[]". */
s/memeber/member/
Okay, will update it.
+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;
+}
+
Moved from c/c-decl.cc
Re: [PATCH v6 1/5] Provide counted_by attribute to flexible array member field (PR108896)
On 2024-02-16 14:47, Qing Zhao wrote:
'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 named "COUNT" in the
same structure as the flexible array member. GCC uses this
information to improve the results of 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 will report a warning and
ignore the attribute.
When the field that represents the number of the elements is assigned a
negative integer value, the compiler will treat 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' should 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 will report
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 will use 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' will use 'val1' as the number of the elements
in 'p->array', and 'ref2' will use 'val2' as the number of elements
in 'p->array'.
I can't approve of course, but here's a review of the code that should
hopefully make it easier for the C frontend maintainers.
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/c-family/c-attribs.cc| 54 -
gcc/c-family/c-common.cc | 13 +++
gcc/c-family/c-common.h | 1 +
gcc/c/c-decl.cc | 85
gcc/c/c-tree.h | 1 +
gcc/c/c-typeck.cc| 3 +-
gcc/doc/extend.texi | 64 +++
gcc/testsuite/gcc.dg/flex-array-counted-by.c | 40 +
8 files changed, 241 insertions(+), 20 deletions(-)
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..4395c0656b14 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 *) ;
[PATCH v6 1/5] Provide counted_by attribute to flexible array member field (PR108896)
'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 named "COUNT" in the
same structure as the flexible array member. GCC uses this
information to improve the results of 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 will report a warning and
ignore the attribute.
When the field that represents the number of the elements is assigned a
negative integer value, the compiler will treat 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' should 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 will report
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 will use 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' will use 'val1' as the number of the elements
in 'p->array', and 'ref2' will use '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/c-family/c-attribs.cc| 54 -
gcc/c-family/c-common.cc | 13 +++
gcc/c-family/c-common.h | 1 +
gcc/c/c-decl.cc | 85
gcc/c/c-tree.h | 1 +
gcc/c/c-typeck.cc| 3 +-
gcc/doc/extend.texi | 64 +++
gcc/testsuite/gcc.dg/flex-array-counted-by.c | 40 +
8 files changed, 241 insertions(+), 20 deletions(-)
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..4395c0656b14 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 },
