diff mbox series

[v8,1/5] Provide counted_by attribute to flexible array member field (PR108896)

Message ID 20240329160703.4012941-2-qing.zhao@oracle.com
State New
Headers show
Series New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) | expand

Commit Message

Qing Zhao March 29, 2024, 4:06 p.m. UTC
'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

Comments

Joseph Myers April 10, 2024, 5:35 p.m. UTC | #1
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).
Qing Zhao April 10, 2024, 6:05 p.m. UTC | #2
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
>
Martin Uecker April 10, 2024, 6:25 p.m. UTC | #3
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).
>
Joseph Myers April 10, 2024, 6:44 p.m. UTC | #4
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).
Martin Uecker April 10, 2024, 7:05 p.m. UTC | #5
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).
> > 
>
Qing Zhao April 10, 2024, 7:21 p.m. UTC | #6
> 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
>
Qing Zhao April 10, 2024, 7:35 p.m. UTC | #7
> 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).
>>> 
>> 
>
Joseph Myers April 10, 2024, 9:56 p.m. UTC | #8
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).
Martin Uecker April 11, 2024, 6:02 a.m. UTC | #9
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).
> > > > 
> > > 
> > 
>
Qing Zhao April 11, 2024, 1:16 p.m. UTC | #10
> 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).
>>>>> 
>>>> 
>>> 
>> 
>
Qing Zhao April 11, 2024, 1:17 p.m. UTC | #11
> 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
diff mbox series

Patch

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)));
+};