Hi, Martin, Thanks a lot for all your comments and questions, really helpful.
> On Apr 10, 2025, at 01:41, Martin Uecker <uec...@tugraz.at> wrote: > > Am Mittwoch, dem 09.04.2025 um 18:31 +0000 schrieb Qing Zhao: >> Hi, Joseph and Martin, >> >> When I implemented the patch to attach the counted_by information to an >> array reference (FAM reference) in C FE, >> The work was done inside the routine “build_component_ref” in >> gcc/c/c-typeck.cc <http://c-typeck.cc/>. and this is very reasonable and >> quite clean. >> >> Now, If we try to replace a structure pointer reference with counted_by >> attached to the FAM field, where in the C FE >> I should look at? >> >> For the following small example, I am trying to replace the pointer >> reference at line 28, “obj”, and line 34, “q” to a call to >> .ACCESS_WITH_SIZE. However, “p” at line 21 and “q” at line 33 should not be >> replaced. >> >> Do you have any suggestions? >> >> Thanks a lot for your help here. > > Hi Qing, > > sorry, I wasn't able to follow the thread. If I understand correctly, > you want to extend the reach of BDOS by using the information from > counted_by, but not only for the size of the FAM but also for the > size of the struct itself, for scenarious where BDOS can not find > the original allocation. Is this correct? Exactly! > > I have some questions about this: The access would add new reads > to the size field. For counted_by, so far, those are somehow > coupled to the access to the member. From this you would assume > that inserting the new accesses to the size field are ok (e.g. > one can assume the struct is accessible, there are no new race > conditions under some assumptions, it is initialized). If you > now want to read the size at different points, all this is even > less clear to me. struct annotated { size_t count; char array[] __attribute__((counted_by (count))); } *p; Currently, when we see: p->array We assume that the pointer p is accessible and initialized. Therefore, Inserting the new access to the size field is ok, i.e., p->count Is valid and safe to be inserted for p->array. However, when we only see the pointer p, it’s not always safe to insert p->count Since the pointer p might not be accessible nor initialized at that point. Only when we are sure which use of pointer p is accessible and initialized, we can replace the use of pointer p with size computations and a call to .ACCESS_WITH_SIZE. So, at this moment, I really feel that this might NOT be a task in FE since FE doesn’t have enough information to figure out whether a use of pointer p is accessible and initialized. Middle-end with data-flow might be more proper to do this? > > I may miss something, but I think one would have to first > define a clear criterion when it is ok to access the size > field. > > So to me it seems we can not simply insert it at specific > point in the C FE where a pointer to the struct is > used, but only when there is some specific trigger. Yes. > > 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. [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 }
