> 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