On 2025-05-07 12:59, Qing Zhao wrote:
Hi,
This is the 2nd version of the patch for:
Evaluate the object size by the size of the pointee type when the type
is a structure with flexible array member which is annotated with
counted_by.
Per the following discussion: (Questions on replacing a structure
pointer reference to a call to .ACCESS_WITH_SIZE in C FE)
https://gcc.gnu.org/pipermail/gcc-patches/2025-April/680540.html
https://gcc.gnu.org/pipermail/gcc-patches/2025-April/681229.html
The summary of the above discussion:
A. It’s not safe in general to replace a structure pointer
reference to a call to .ACCESS_WITH_SIZE in C FE.
Since data-flow analysis is needed to make sure that the access
to the size member is valid, i.e, the structure is accessible
and initialized, etc.
B. It should be safe to generate the reference to field member
when we evaluate the BDOS builtin as the 1st version of the
patch . And doing this in tree-object-size should also cover
-fsanitize=object-size.
C. When generating the reference to the field member in tree-object-size,
we should guard this reference with a checking on the pointer to
the structure is valid.
Thank you for working on this, and sorry it took me so long to get to
this. There's one more key point I had mentioned in response to Martin
in that discussion, which may be pertinent even for tree-object-size,
not just the frontend:
>> Yes. But there could be other issues. The memory could be
>> protected.
>
> True, or the pointer could be arbitrary/uninitialized like you >
> mentioned above. That is indeed a problem. Maybe one way to hedge
> against this could be to generate the size expression (and
> consequently, .ACCESS_WITH_SIZE) only when the pointer is read *and*
> dereferenced (could be separate sites, but for the same pointer) in
> the routine? That way we only add a pointer dereference for the size
> expression when there already is a dereference; we're not adding any
> *new* potential undefined behaviour.
A NULL check alone does not solve this problem because the pointer could
be arbitrary and invalid. I thought some more about adding the size
expression only if there's an existing access like I suggested in that
email, but I realize that doesn't make sense either, since code that
uses __bdos typically would look to the __bdos result to determine
validity of the access.
Can you see a way to validate the pointer in addition to the NULL check?
Sid