Am Dienstag, dem 03.12.2024 um 14:31 +0000 schrieb Qing Zhao:
>
> > On Dec 3, 2024, at 01:33, Martin Uecker <[email protected]> wrote:
> >
> > Am Montag, dem 02.12.2024 um 22:58 +0000 schrieb Qing Zhao:
> > >
> > > > On Dec 2, 2024, at 16:13, Martin Uecker <[email protected]> wrote:
> > > >
> > > > Am Montag, dem 02.12.2024 um 20:15 +0000 schrieb Qing Zhao:
> > > > >
> > > > > > On Nov 30, 2024, at 07:22, Martin Uecker <[email protected]> wrote:
> > > > > >
> > > > > > Am Dienstag, dem 26.11.2024 um 20:59 +0000 schrieb Qing Zhao:
> > > > > > > Think this over these days, I have another thought that need some
> > > > > > > feedback:
> > > > > > >
> > > > > > > The major issue right now is:
> > > > > > >
> > > > > > > 1. For the following structure in which the “counted_by”
> > > > > > > attributes is attached to the pointer field.
> > > > > > >
> > > > > > > struct foo {
> > > > > > > int n;
> > > > > > > char *p __attribute__ ((counted_by (n)));
> > > > > > > } *x;
> > > > > >
> > > > > > BTW: I also do not like the syntax 'n' for a lookup of a member
> > > > > > in the member namespace. I think it should be '.n'. For FAM this
> > > > > > is less problematic because it always at the end, but here it is
> > > > > > problematic.
> > > > > During my implementation of extending this attribute to pointer
> > > > > fields of structure.
> > > > > I didn’t notice issue with the current syntax ’n’ for the pointer
> > > > > fields so far,
> > > > > even though when the field “n” is declared after the corresponding
> > > > > pointer field, i.e:
> > > > >
> > > > > struct foo {
> > > > > {
> > > > > char *p __attribute__ ((counted_by (n)));
> > > > > int n;
> > > > > }
> > > > >
> > > > > So, could you please explain a little bit more on what’s the
> > > > > potential issue here?
> > > >
> > > > My issue with it is that it is not consistent to how C's scoping
> > > > of identifiers work. In
> > > >
> > > > constexpr int n = 3;
> > > > struct foo {
> > > > {
> > > > char (*p)[n] __attribute__ ((counted_by (n))
> > > > int n;
> > > > }
> > > >
> > > > the n in the type of p refers to the previous n in scope while
> > > > the n in your attribute would refer to the member.
> > >
> > > Okay, I see your point here -:).
> > >
> > > Yes, I agree that if the compiler can accept the syntax “.n” in general,
> > > then it will make the
> > > “counted_by” attribute to allow more complicated expressions in general.
> > >
> > > We had this similar discussion before the design and implementation for
> > > the “counted_by” attribute
> > > on FAMs, and we agreed to delay the approach of accepting the syntax “.n”
> > > in the future possible
> > > language standard at that time.
> > >
> > > So, for the “counted_by attribute on FAMs, the implementation is,
> > > searching the “n” in all the fields of the
> > > containing structure and locating that specific field.
> > >
> > > Now, when extending “counted_by” attribute to pointer fields of
> > > structures, the implementation is similar.
> >
> > > >
> > > > This is incoherent and confusing. It becomes worse should
> > > > you ever want to allow more complicated expressions.
> > >
> > > You are right, it’s hard to allow more complicated expressions for
> > > “counted_by” based on the current
> > > design.
> > >
> > > If we agree to accept the “.n” syntax in GCC in general, that’s of course
> > > better.
> > > Then how about the current “counted_by” for FAMs? Shall we also change it
> > > to accept “.n” syntax?
> >
> > My recommendation would be to change it. It is also not ideal for
> > this case - only less problematic.
>
> If I remembered correctly, the “counted_by” attribute for FAM has been added
> into Linux Kernel already.
> Changing its syntax now also involves changing Linux Kernel source code.
>
> Kees, what’s your comments and suggestions for this?
>
> Another issue is, we had the similar discussion before implementing the
> “counted_by” attribute for FAM, at that time,
> We decided to use the current syntax, and CLANG also used the same syntax.
> Changing the syntax for the “counted_by” attribute for FAM at this moment is
> not very simple.
We could add the new syntax and still allow the old syntax only for the
FAM case (possibly deprecating it).
>
> Why it’s LESS problematic for FAM case? (From implementation point of view,
> they are the same for both FAM or pointer inside a structure).
Because the FAM is always last, so there is less room for confusion.
It is still annoying, which is why I pointed this out before.
> >
> > > >
> > > > It would be clearer if you the syntax ".n" which resembles
> > > > the syntax for designated initializers that is already used
> > > > in initializers to refer to struct members.
> > > >
> > > > constexpr int n;
> > > > struct foo {
> > > > {
> > > > char (*p)[n] __attribute__ ((counted_by (.n))
> > > > int n;
> > > > }
> > > >
> > > Yes, I agree.
> > > >
> > > > >
> > > > >
> > > > > > >
> > > > > > > There is one important additional requirement:
> > > > > > >
> > > > > > > x->n, x->p can ONLY be changed by changing the whole structure at
> > > > > > > the same time.
> > > > > > > Otherwise, x->n might not be consistent with x->p.
> > > > > >
> > > > > > By itself, this would still not fix the issue I pointed out.
> > > > > >
> > > > > > struct foo x;
> > > > > > x = .. ; // set the whole structure
> > > > > > char *p = x->p;
> > > > > > x = ... ; // set the whole structure
> > > > > >
> > > > > > What is the bound for 'p' ?
> > > > >
> > > > > Since p was set to the pointer field of the old structure, then the
> > > > > bound of it should be the old bound.
> > > > > > With current rules it would be the old bound.
> > > > >
> > > > > I thought that this should be the correct behavior, isn’t it?
> > > >
> > > > Yes, sorry, what I meant was "with the current rules it would be
> > > > the *new* bound”.
> > >
> > > struct foo x;
> > > x=… ; // set the whole structure 1
> > > char *p = x->p;
> > > x=… ; // set the whole structure 2
> > >
> > > In the above, when “set the whole structure 1”, x1, x1->n and x1->p are
> > > set at the same time;
> > > After *p = x->p; the pointer “p” is pointing to “x1->p”, it’s bound is
> > > “x1->n”;
> >
> > I agree.
> > >
> > > Then when “set the whole structure 2”, x2 is different than x1, x2->n
> > > and x2->p are set at the same time, the pointer
> > > ‘p’ still points to “x1->p”, therefore it’s bound should be “x1->n”.
> > >
> > > So, as long as the whole structure is set at the same time, should be
> > > fine.
> > >
> > > Do I miss anything here?
> >
> > I was talking aout the pointer "p" which was obtained before setting the
> > struct the second time in
> >
> > char *p = x->p;
> >
> > This pointer is still set to x1->p but the bound refers to x.n which is
> > now set to x2->n.
>
> You mean:
>
> struct foo x;
> x=… ; // set the whole structure 1
> char *p = x->p;
> x=… ; // set the whole structure 2
> p[index] = 10; // at this point, p’s bound is x2->n, not x1->n?
>
> Yes, you are right here.
>
> So, is there similar problem with the corresponding language extension?
>
The language extension does not exist yet, so there is no problem.
But I hope we will get it and then specify it so that this works
correctly without this footgun.
Of course, if GCC gets the "counted_by" attribute wrong, there will
be arguments later in WG14 why the feature is then different to it.
Martin
