> 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
