> On Apr 10, 2025, at 13:43, Martin Uecker <[email protected]> wrote:
>
> Am Donnerstag, dem 10.04.2025 um 17:05 +0000 schrieb Qing Zhao:
>> Hi, Martin,
>>
>> Thanks a lot for all your comments and questions, really helpful.
>>
>>
>
> ...
>>>
>>> An example I could imagine is when you memcpy
>>> the struct. (but it is also not entirely clear why this
>>> should not be allowed to go beyond the counted_by size
>>> if the underlying storage is larger).
>>>
>>> Maybe you could add it when a pointer to an annotated
>>> struct is passed as parameter, but also there it is not
>>> clear to me that we might want to materialize new
>>> accesses to the struct at this point.
>>
>> This is true too, and this is even true for the current implementation for
>> p->array,
>> as I checked with a small example as below:
>>
>> [opc@qinzhao-aarch64-ol8 counted_by_whole]$ cat t2.c
>> #include <stdlib.h>
>> #include <stddef.h>
>>
>> struct annotated {
>> size_t count;
>> char array[];
>> };
>>
>> static size_t __attribute__((__noinline__,__noipa__)) size_of (struct
>> annotated * obj)
>> {
>> return __builtin_dynamic_object_size (obj->array, 1);
>> }
>>
>> int main()
>> {
>> __builtin_printf ("the bdos whole is %ld\n", size_of (0));
>> return 0;
>> }
>>
>> In the above example, the parameter to the function “size_of” has pointer
>> type to “struct annotated”,
>> However, I passed a NULL to “size_of”, is this legal C? Looks like -Wall did
>> not issue any warning
>> for it.
>
> It is legal to pass a NULL pointer. Here, the issue is
> that the builtin does not evaluate its argument, so it
> is perhaps surprising that you can get a segfault. If
> the access it outside of the builtin, then this is not
> a problem
>
> static size_t __attribute__((__noinline__,__noipa__)) size_of (struct
> annotated * obj)
> {
> char *p = obj->array;
> return __builtin_dynamic_object_size (p, 1);
> }
The above acts the same as
static size_t __attribute__((__noinline__,__noipa__)) size_of (struct annotated
* obj)
{
return __builtin_dynamic_object_size (obj->array, 1);
}
Only after I changed it as:
static size_t __attribute__((__noinline__,__noipa__)) size_of (struct annotated
* obj)
{
char *p = obj->array;
p[1] = 0;
return __builtin_dynamic_object_size (p, 1);
}
I got a segmentation fault before evaluating _bdos.
>
> because you get the segault anyway when the first line
> is executed.
Yes. That’s right.
>
> Maybe we need to document that the BDOS builtin requires
> obj->array to be accessible even though it is not
> evaluated.
Yes, this needs to be at least documented.
>
> But I wonder whether there are other cases where
> the object-size path can walk into dead code and create
> accesses in this way. Not sure, but I found this bug though:
> https://godbolt.org/z/ejP918nW7
Thanks, will file bugs and fix them first.
thanks.
Qing
>
> Martin
>
>>
>>
>> [opc@qinzhao-aarch64-ol8 counted_by_whole]$ sh t
>> /home/opc/Install/latest-d/bin/gcc -O2 -Wall t2.c
>> the bdos whole is -1
>> 0
>>
>> Then when I added the counted_by attribute for FAM array as:
>>
>> [opc@qinzhao-aarch64-ol8 counted_by_whole]$ cat t2.c
>> #include <stdlib.h>
>> #include <stddef.h>
>>
>> struct annotated {
>> size_t count;
>> char array[] __attribute__ ((counted_by(count)));
>> };
>>
>> static size_t __attribute__((__noinline__,__noipa__)) size_of (struct
>> annotated * obj)
>> {
>> return __builtin_dynamic_object_size (obj->array, 1);
>> }
>>
>> int main()
>> {
>> __builtin_printf ("the bdos whole is %ld\n", size_of (0));
>> return 0;
>> }
>> [opc@qinzhao-aarch64-ol8 counted_by_whole]$ sh t
>> /home/opc/Install/latest-d/bin/gcc -O2 -Wall t2.c
>> t: line 13: 2944007 Segmentation fault (core dumped) ./a.out
>> 139
>>
>> This is because we insert the load from the &p->count for the size.
>>
>> Qing
>>
>>>
>>> An alternative approach could be to just do it when
>>> such a pointer is explicitely passed to the BDOS builtin.
>>>
>>> Martin
>>>
>>>
>>>>
>>>> Qing
>>>>
>>>> 1 #include <stdlib.h>
>>>> 2 #include <stddef.h>
>>>> 3
>>>> 4 struct annotated {
>>>> 5 size_t count;
>>>> 6 char array[] __attribute__((counted_by (count)));
>>>> 7 };
>>>> 8
>>>> 9 /* compute the minimum # of bytes needed to hold a structure “annotated”,
>>>> 10 whose # of elements of “array” is COUNT. */
>>>> 11 #define MAX(A, B) (A > B) ? (A) : (B)
>>>> 12 #define ALLOC_SIZE_ANNOTATED(COUNT) \
>>>> 13 MAX(sizeof (struct annotated), \
>>>> 14 offsetof(struct annotated, array[0]) + (COUNT) * sizeof(char))
>>>> 15
>>>> 16 /* allocate the memory for the structure with FAM,
>>>> 17 update “count” with the # of elements “index”. */
>>>> 18 static struct annotated * __attribute__((__noinline__)) alloc_buf (int
>>>> index)
>>>> 19 {
>>>> 20 struct annotated *p;
>>>> 21 p = (struct annotated *) malloc (ALLOC_SIZE_ANNOTATED(index));
>>>> 22 p->count = index;
>>>> 23 return p;
>>>> 24 }
>>>> 25
>>>> 26 static size_t __attribute__((__noinline__)) size_of (struct annotated *
>>>> obj)
>>>> 27 {
>>>> 28 return __builtin_dynamic_object_size (obj, 1);
>>>> 29 }
>>>> 30
>>>> 31 int main()
>>>> 32 {
>>>> 33 struct annotated *q = alloc_buf (10);
>>>> 34 __builtin_printf ("the bdos whole is %d\n", size_of (q));
>>>> 35 return 0;
>>>> 36 }
