On Wed, Oct 02, 2024 at 03:26:26PM +0000, Qing Zhao wrote:
> From: qing zhao <[email protected]>
>
> When handling the counted_by attribute, if the corresponding field
> doesn't exit, in additiion to issue error, we should also remove
> the already added non-existing "counted_by" attribute from the
> field_decl.
>
> bootstrapped and regression tested on both x86 and aarch64.
> Okay for committing?
>
> thanks.
>
> Qing
For next time, the subject should look more like:
[PATCH] c: ICE in build_counted_by_ref [PR116735]
> ==============================
>
> C/PR 116735
This needs to be PR c/116735
> gcc/c/ChangeLog:
>
> * c-decl.cc (verify_counted_by_attribute): Remove the attribute
> when error.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/flex-array-counted-by-pr116735.c: New test.
> ---
> gcc/c/c-decl.cc | 31 ++++++++++++-------
> .../gcc.dg/flex-array-counted-by-pr116735.c | 19 ++++++++++++
> 2 files changed, 38 insertions(+), 12 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-pr116735.c
>
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index aa7f69d1b7b..ce28b0a1022 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9502,14 +9502,18 @@ verify_counted_by_attribute (tree struct_type, tree
> field_decl)
>
> tree counted_by_field = lookup_field (struct_type, fieldname);
>
> - /* Error when the field is not found in the containing structure. */
> + /* Error when the field is not found in the containing structure and
> + remove the corresponding counted_by attribute from the field_decl. */
> 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);
> -
> + {
> + 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)),
Why use get_attribute_name when we know it must be "counted_by"? And below
too.
> + field_decl);
> + DECL_ATTRIBUTES (field_decl)
> + = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl));
> + }
LGTM.
> else
> /* Error when the field is not with an integer type. */
> {
> @@ -9518,11 +9522,14 @@ verify_counted_by_attribute (tree struct_type, tree
> field_decl)
> 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)));
> -
> + {
> + error_at (DECL_SOURCE_LOCATION (field_decl),
> + "argument %qE to the %qE attribute is not a field
> declaration"
This line is too long now.
> + " with an integer type", fieldname,
> + (get_attribute_name (attr_counted_by)));
> + DECL_ATTRIBUTES (field_decl)
> + = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl));
> + }
> }
Is there a test for this second hunk?
> return;
This return is pointless.
> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-pr116735.c
> b/gcc/testsuite/gcc.dg/flex-array-counted-by-pr116735.c
> new file mode 100644
> index 00000000000..958636512b7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-pr116735.c
Please rename this to flex-array-counted-by-9.c.
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
Please add /* PR c/116735 */ here.
> +/* { dg-options "-O" } */
Why -O?
> +struct foo {
> + int len;
> + int element[] __attribute__ ((__counted_by__ (lenx))); /* { dg-error
> "attribute is not a field declaration in the same structure as" } */
> +};
> +
> +int main ()
> +{
> + struct foo *p = __builtin_malloc (sizeof (struct foo) + 3 * sizeof (int));
> + p->len = 1;
> + p->element[0] = 17;
> + p->len = 2;
> + p->element[1] = 13;
> + p->len = 1;
> + int x = p->element[1];
> + return x;
> +}
> +
> --
> 2.43.5
>
Thanks,
Marek
