Am Freitag, dem 06.12.2024 um 14:16 +0000 schrieb Qing Zhao:
>
> > On Dec 5, 2024, at 17:31, Martin Uecker <uec...@tugraz.at> wrote:
> >
> > Am Donnerstag, dem 05.12.2024 um 21:09 +0000 schrieb Qing Zhao:
> > >
> > > > On Dec 3, 2024, at 10:29, Qing Zhao <qing.z...@oracle.com> wrote:
> >
> > ....
> >
> > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > 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.
> > > > Yeah, I should mention this as “corresponding future language
> > > > extension” -:)
> > > > >
> > > > > 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.
> > > >
> > > > I think that we need to resolve this issue first in the design of
> > > > “counted_by” for pointer fields.
> > > > I guess that we might need to come up with some additional limitations
> > > > for using the “counted_by”
> > > > attribute for pointer fields at the source code level in order to avoid
> > > > such potential error. But not
> > > > sure what exactly the additional limitation should be at this moment.
> > > >
> > > > Need some study here.
> > >
> > > Actually, I found out that this is really not a problem with the current
> > > design, for the following new testing case I added for my current
> > > implementation of the counted_by for pointer field:
> > >
> > > [ gcc.dg]$ cat pointer-counted-by-7.c
> > > /* Test the attribute counted_by for pointer field and its usage in
> > > * __builtin_dynamic_object_size. */
> > > /* { dg-do run } */
> > > /* { dg-options "-O2" } */
> > >
> > > #include "builtin-object-size-common.h"
> > >
> > > struct annotated {
> > > int b;
> > > int *c __attribute__ ((counted_by (b)));
> > > };
> > >
> > > struct annotated *__attribute__((__noinline__)) setup (int attr_count)
> > > {
> > > struct annotated *p_array_annotated
> > > = (struct annotated *) malloc (sizeof (struct annotated));
> > > p_array_annotated->c = (int *) malloc (sizeof (int) * attr_count);
> > > p_array_annotated->b = attr_count;
> > >
> > > return p_array_annotated;
> > > }
> > >
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > struct annotated *x = setup (10);
> > > int *p = x->c;
> > > x = setup (20);
> > > EXPECT(__builtin_dynamic_object_size (p, 1), 10 * sizeof (int));
> > > EXPECT(__builtin_dynamic_object_size (x->c, 1), 20 * sizeof (int));
> > > DONE ();
> > > }
> > >
> > > This test case passed without any issue.
> > >
> > > Our previous introduction of the new internal function .ACCESS_WITH_SIZE
> > > already resolved this issue.
> > >
> > > So, I think that as long as the whole structure is set at the same time,
> > > should be fine.
> > >
> > > Let me know if you have more comment here.
> >
> > I think the relevant scenario is where you assign the struct and
> > not a pointer to the struct, i.e. something like the following:
> >
> > #include "builtin-object-size-common.h"
> >
> > struct annotated {
> > int b;
> > int *c __attribute__ ((counted_by (b)));
> > };
> >
> > struct annotated __attribute__((__noinline__)) setup (int attr_count)
> > {
> > struct annotated p_array_annotated;
> > p_array_annotated.c = (int *) malloc (sizeof (int) * attr_count);
> > p_array_annotated.b = attr_count;
> >
> > return p_array_annotated;
> > }
> >
> >
> > int main(int argc, char *argv[])
> > {
> > struct annotated x = setup (10);
> > int *p = x.c;
> > x = setup (20);
> > EXPECT(__builtin_dynamic_object_size (p, 1), 10 * sizeof (int));
> > EXPECT(__builtin_dynamic_object_size (x.c, 1), 20 * sizeof (int));
> > DONE ();
> > }
> >
>
> With the above testing case, my current implementation based on
> .ACCESS_WITH_SIZE succeed without any issue. -:)
> The design of .ACCESS_WITH_SIZE already resolved this issue.
Ok, thanks! But I am a bit confused, because it seems it behaves
this way also for FAMs
https://godbolt.org/z/64a6z4cna
and I thought this was changed, i.e. so that the bounds
track the current value of b for this case. But apparently
I was mistaken. I am fine with the behavior as shown here.
Martin
>
> Qing
> > Martin
> >
> >
> >
>
--
Univ.-Prof. Dr. rer. nat. Martin Uecker
Graz University of Technology
Institute of Biomedical Imaging
