> On Nov 2, 2023, at 9:54 AM, Richard Biener <richard.guent...@gmail.com> wrote:
>
> On Thu, Nov 2, 2023 at 2:50 PM Qing Zhao <qing.z...@oracle.com> wrote:
>>
>>
>>
>>> On Nov 2, 2023, at 3:57 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?).
>>
>> Yes. Will add this into the user documentation.
>>
>>> 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.
>>
>> What do you mean by “re-obtained”?
>
> do
>
> p = &container.array[0];
>
> after any adjustments to 'array' or 'len' again and base further accesses on
> the new 'p'.
Then for the following example form Kees:
struct foo *f;
char *p;
int i;
f = alloc(maximum_possible);
f->count = 0;
p = f->buf;
for (i; data_is_available() && i < maximum_possible; i++) {
f->count ++;
p[i] = next_data_item();
}
Will not work?
We have to change it as:
struct foo *f;
char *p;
int i;
f = alloc(maximum_possible);
f->count = 0;
p = f->buf;
for (i; data_is_available() && i < maximum_possible; i++) {
f->count ++;
p = f->buf; // add this?
p[i] = next_data_item();
}
Why?
>
>>> 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.
>>
>> Through an lvalue with the containing type?
>>
>> Yes, will add this too.
>>
>>
>>> That is,
>>>
>>> size_t *s = &a.size;
>>> *s = 1;
>>>
>>> is invoking undefined behavior,
>>
>> right.
>>
>>> 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).
>> Is there a small example for the undefined behavior for this?
>
> a.len = len;
> posix_memalign (&a.buf, 16, len);
>
> we would probably have to somehow instrument this.
For the above, the buffer and the size are still synchronized even after the
buffer is modified? Should we allow this?
Qing
>
>> Qing
>>>
>>> Richard.
>>>
>>>> Qing
>>>>>
>>>>> --
>>>>> Joseph S. Myers
>>>>> jos...@codesourcery.com
>>>>
>>
