Am Donnerstag, dem 24.04.2025 um 15:15 +0000 schrieb Qing Zhao:
> Hi,
>
> Kees reported a segmentation failure when he used the patch to compiler
> kernel,
> and the reduced the testing case is something like the following:
>
> struct f {
> void *g __attribute__((__counted_by__(h)));
> long h;
> };
>
> extern struct f *my_alloc (int);
>
> int i(void) {
> struct f *iov = my_alloc (10);
> int *j = (int *)iov->g;
> return __builtin_dynamic_object_size(iov->g, 0);
> }
>
> Basically, the problem is relating to the pointee type of the pointer array
> being “void”,
> As a result, the element size of the array is not available in the IR.
> Therefore segmentation
> fault when calculating the size of the whole object.
>
> Although it’s easy to fix this segmentation failure, I am not quite sure
> what’s the best
> solution to this issue:
>
> 1. Reject such usage of “counted_by” in the very beginning by reporting
> warning to the
> User, and delete the counted_by attribute from the field.
>
> Or:
>
> 2. Accept such usage, but issue warnings when calculating the object_size in
> Middle-end.
>
> Personally, I prefer the above 1 since I think that when the pointee type is
> void, we don’t know
> The type of the element of the pointer array, there is no way to decide the
> size of the pointer array.
>
> So, the counted_by information is not useful for the
> __builtin_dynamic_object_size.
>
> But I am not sure whether the counted_by still can be used for bound
> sanitizer?
>
> Thanks for suggestions and help.
GNU C allows pointer arithmetic and sizeof on void pointers and
that treats void as having size 1. So you could also allow counted_by
and assume as size 1 for void.
https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html
Martin
>
> Qing
>
>
>
>
> > On Apr 23, 2025, at 15:45, Qing Zhao <qing.z...@oracle.com> wrote:
> >
> > Hi,
> >
> > This is the 2nd version of the patch set to extend "counted_by" attribute
> > to pointer fields of structures.
> >
> > the first version was submitted 3 months ago on 1/16/2025, and triggered
> > a lot of discussion on whether we need a new syntax for counted_by
> > attribute.
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673837.html
> >
> > After a long discussion since then:
> > (https://gcc.gnu.org/pipermail/gcc-patches/2025-March/677024.html)
> >
> > We agreed to the following compromised solution:
> >
> > 1. Keep the current syntax of counted_by for lone identifier;
> > 2. Add a new attribute "counted_by_exp" for expressions.
> >
> > Although there are still some discussion going on for the new
> > counted_by_exp attribute (In Clang community)
> > https://discourse.llvm.org/t/rfc-bounds-safety-in-c-syntax-compatibility-with-gcc/85885
> >
> > The syntax for the lone identifier is kept the same as before.
> >
> > So, I'd like to resubmit my previous patch of extending "counted_by"
> > to pointer fields of structures.
> >
> > The whole patch set has been rebased on the latest trunk, some testing case
> > adjustment, bootstrapped and regression tested on both aarch64 and x86.
> >
> > There will be a seperate patch set for the new "counted_by_exp"
> > attribute later to cover the expressions cases.
> >
> > The following are more details on this patch set:
> >
> > For example:
> >
> > struct PP {
> > size_t count2;
> > char other1;
> > char *array2 __attribute__ ((counted_by (count2)));
> > int other2;
> > } *pp;
> >
> > specifies that the "array2" is an array that is pointed by the
> > pointer field, and its number of elements is given by the field
> > "count2" in the same structure.
> >
> > There are the following importand facts about "counted_by" on pointer
> > fields compared to the "counted_by" on FAM fields:
> >
> > 1. one more new requirement for pointer fields with "counted_by" attribute:
> > pp->array2 and pp->count2 can ONLY be changed by changing the whole
> > structure
> > at the same time.
> >
> > 2. the following feature for FAM field with "counted_by" attribute is NOT
> > valid for the pointer field any more:
> >
> > " One important feature of the attribute is, a reference to the
> > flexible array member field uses the latest value assigned to the
> > field that represents the number of the elements before that
> > reference. For example,
> >
> > p->count = val1;
> > p->array[20] = 0; // ref1 to p->array
> > p->count = val2;
> > p->array[30] = 0; // ref2 to p->array
> >
> > in the above, 'ref1' uses 'val1' as the number of the elements in
> > 'p->array', and 'ref2' uses 'val2' as the number of elements in
> > 'p->array'. "
> >
> > This patch set includes 3 parts:
> >
> > 1.Extend "counted_by" attribute to pointer fields of structures.
> > 2.Convert a pointer reference with counted_by attribute to .ACCESS_WITH_SIZE
> > and use it in builtinin-object-size.
> > 3.Use the counted_by attribute of pointers in array bound checker.
> >
> > In which, the patch 1 and 2 are simple and straightforward, however, the
> > patch 3
> > is a little complicate due to the following reason:
> >
> > Current array bound checker only instruments ARRAY_REF, and the INDEX
> > information is the 2nd operand of the ARRAY_REF.
> >
> > When extending the array bound checker to pointer references with
> > counted_by attributes, the hardest part is to get the INDEX of the
> > corresponding array ref from the offset computation expression of
> > the pointer ref.
> >
> > I do need some careful review on the 3rd part of the patch. And I do wonder
> > for the access to pointer arrays:
> >
> > struct annotated {
> > int b;
> > int *c __attribute__ ((counted_by (b)));
> > } *p_array_annotated;
> >
> > p_array_annotated->c[annotated_index] = 2;
> >
> > Is it possible to generate ARRAY_REF instead of INDIRECT_REF for the above
> > p_array_annotated->c[annotated_index]
> > in C FE? then we can keep the INDEX info in the IR and avoid all the hacks
> > to get the index from the OFFSET computation expression.
> >
> > The whole patch set has been rebased on the latest trunk, bootstrapped
> > and regression tested on both aarch64 and x86.
> >
> > Let me know any comments and suggestions.
> >
> > Thanks.
> >
> > Qing
>
--
Univ.-Prof. Dr. rer. nat. Martin Uecker
Graz University of Technology
Institute of Biomedical Imaging
