> On Nov 2, 2023, at 8:13 PM, Bill Wendling <isanb...@gmail.com> wrote: > > 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.
I will update the counted-by documentation with the following additions: 1. The initialization of the size field should be done before the first reference to the FAM; And. 2. A later reference to the FAM will use the latest value assigned to the size field before that reference; I think adding these two on top of my current user documentation should be enough. I will update the counted-by user documentation and then send out for comment in a later email. thanks. Qing > > -bw