On Thu, Nov 2, 2023 at 1:00 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Wed, Nov 1, 2023 at 3:47 PM Qing Zhao <qing.z...@oracle.com> wrote:
> >
> >
> >
> > > On Oct 31, 2023, at 6:14 PM, Joseph Myers <jos...@codesourcery.com> wrote:
> > >
> > > On Tue, 31 Oct 2023, Qing Zhao wrote:
> > >
> > >> 2.3 A new semantic requirement in the user documentation of "counted_by"
> > >>
> > >> For the following structure including a FAM with a counted_by attribute:
> > >>
> > >> struct A
> > >> {
> > >> size_t size;
> > >> char buf[] __attribute__((counted_by(size)));
> > >> };
> > >>
> > >> for any object with such type:
> > >>
> > >> struct A *obj = __builtin_malloc (sizeof(struct A) + sz * sizeof(char));
> > >>
> > >> The setting to the size field should be done before the first reference
> > >> to the FAM field.
> > >>
> > >> Such requirement to the user will guarantee that the first reference to
> > >> the FAM knows the size of the FAM.
> > >>
> > >> We need to add this additional requirement to the user document.
> > >
> > > Make sure the manual is very specific about exactly when size is
> > > considered to be an accurate representation of the space available for buf
> > > (given that, after malloc or realloc, it's going to be temporarily
> > > inaccurate). If the intent is that inaccurate size at such a time means
> > > undefined behavior, say so explicitly.
> >
> > Yes, good point. We need to define this clearly in the beginning.
> > We need to explicit say that
> >
> > the size of the FAM is defined by the latest “counted_by” value. And it’s
> > an undefined behavior when the size field is not defined when the FAM is
> > referenced.
> >
> > Is the above good enough?
> >
> >
> > >
> > >> 2.4 Replace FAM field accesses with the new function ACCESS_WITH_SIZE
> > >>
> > >> In C FE:
> > >>
> > >> for every reference to a FAM, for example, "obj->buf" in the small
> > >> example,
> > >> check whether the corresponding FIELD_DECL has a "counted_by" attribute?
> > >> if YES, replace the reference to "obj->buf" with a call to
> > >> .ACCESS_WITH_SIZE (obj->buf, obj->size, -1);
> > >
> > > This seems plausible - but you should also consider the case of static
> > > initializers - remember the GNU extension for statically allocated objects
> > > with flexible array members (unless you're not allowing it with
> > > counted_by).
> > >
> > > static struct A x = { sizeof "hello", "hello" };
> > > static char *y = &x.buf;
> > >
> > > I'd expect that to be valid - and unless you say such a usage is invalid,
> >
> > At this moment, I think that this should be valid.
> >
> > I,e, the following:
> >
> > struct A
> > {
> > size_t size;
> > char buf[] __attribute__((counted_by(size)));
> > };
> >
> > static struct A x = {sizeof "hello", "hello”};
> >
> > Should be valid, and x.size represents the number of elements of x.buf.
> > Both x.size and x.buf are initialized statically.
> >
> > > you should avoid the replacement in such a static initializer context when
> > > the FAM reference is to an object with a constant address (if
> > > .ACCESS_WITH_SIZE would not act as an lvalue whose address is a constant
> > > expression; if it works fine as a constant-address lvalue, then the
> > > replacement would be OK).
> >
> > Then if such usage for the “counted_by” is valid, we need to replace the FAM
> > reference by a call to .ACCESS_WITH_SIZE as well.
> > Otherwise the “counted_by” relationship will be lost to the Middle end.
> >
> > With the current definition of .ACCESS_WITH_SIZE
> >
> > PTR = .ACCESS_WITH_SIZE (PTR, SIZE, ACCESS_MODE)
> >
> > Isn’t the PTR (return value of the call) a LVALUE?
>
> You probably want to specify that when a pointer to the array is taken the
> pointer has to be to the first array element (or do we want to mangle the
> 'size' accordingly for the instrumentation?). You also want to specify that
> the 'size' associated with such pointer is assumed to be unchanging and
> after changing the size such pointer has to be re-obtained. Plus that
> changes to the allocated object/size have to be performed through an
> lvalue where the containing type and thus the 'counted_by' attribute is
> visible. That is,
>
> size_t *s = &a.size;
> *s = 1;
>
> is invoking undefined behavior, likewise modifying 'buf' (makes it a bit
> awkward since for example that wouldn't support using posix_memalign
> for allocation, though aligned_alloc would be fine).
>
I believe Qing's original documentation for counted_by makes the
relationship between 'size' and the FAM very clear and that without
agreement it'll result in undefined behavior. Though it might be best
to state that in a strong way.
-bw
