> On Jun 17, 2025, at 17:15, Siddhesh Poyarekar <siddh...@gotplt.org> wrote:
> 
> 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?

Right now, we only generate the size expression for the following cases:

1. When the pointer is read as the first parameter of the  
__builtin_dynamic_object_size (p, 0);
AND
2. p != NULL. 

Is the above 1 & 2 not enough to avoid the introduced undefined behavior? 
I thought that the above 1 should validate the pointer already, let me know if 
this is not the case. 
If So, could you please provide me a small testing case that illustrates the 
issue? 

Thanks a lot for the review and help.

Qing
> 
> Sid


Reply via email to