> On Oct 2, 2024, at 11:48, Marek Polacek <pola...@redhat.com> wrote:
>
> On Wed, Oct 02, 2024 at 03:26:26PM +0000, Qing Zhao wrote:
>> From: qing zhao <qing.z...@oracle.com>
>>
>> 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]
Okay.
>
>> ==============================
>>
>> C/PR 116735
>
> This needs to be PR c/116735
Okay.
>
>> 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.
Okay, will fix this.
>
>> + " 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?
Will add one.
>
>> 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.
Sure.
>
>> @@ -0,0 +1,19 @@
>> +/* { dg-do compile } */
>
> Please add /* PR c/116735 */ here.
sure.
>
>> +/* { dg-options "-O" } */
>
> Why -O?
Delete the optimization flag should also repeat the issue, I will delete it.
thanks.
Qing
>
>> +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
