[clang] [BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C (PR #90786)

Bill Wendling via cfe-commits cfe-commits at lists.llvm.org
Mon May 13 14:45:16 PDT 2024


bwendling wrote:

> > It's not a lie, because the contents of a pointer don't contribute to the size of the struct containing that pointer.
> 
> Consider this example. It tries to illustrate why putting `__counted_by()` on a pointer to a structs containing flexible array members doesn't make sense.
> 
> ```c
> struct HasFAM {
>     int count;
>     char buffer[] __counted_by(count); // This is OK
> };
> 
> struct BufferOfFAMS {
>     int count;
>     struct HasFAM* fams __counted_by(count); // This is invalid
> };
> 
> struct BufferOfFAMS* setup(void) {
>     const int numFams = 2;
>     const int famSizes[] = { 8, 16};
> 
>     struct BufferOfFAMS *f = (struct BufferOfFAMS *)malloc(
>         sizeof(struct BufferOfFAMS) + (sizeof(struct HasFam *) * numFams));
> 
>     f->count = numFams;
> 
>     size_t famsBufferSize = 0;
>     for (int i=0; i < numFams; ++i) {
>         famsBufferSize += sizeof(struct HasFAM) + sizeof(char)* famSizes[i];
>     }
>     f->fams = (struct HasFAM*) malloc(famsBufferSize);
>     memset(f->fams, 0, famsBufferSize);
> 
>     size_t byteOffset = 0;
>     for (int i=0; i < numFams; ++i) {
>         struct HasFAM* cur = (struct HasFAM*) (((char*)f->fams) + byteOffset);
>         cur->count = famSizes[i];
>         byteOffset = sizeof(struct HasFAM) + (sizeof(char) * famSizes[i]);
>     }
>     return f;
> }
> 
> int main(void) {
>     // How would we compute the bounds of f::fams here??
>     // `sizeof(struct HasFAM) * f->count` is WRONG. It's too small but this
>     // is what writing `__counted_by(count)` on `HasFAM::buffer` means.
>     //
>     // It's actually
>     // `(sizeof(struct HasFAM) * f->count) + (sizeof(char) * (8 + 16))`
>     //
>     // This means we can't use the `__counted_by()` attribute on
>     // `BufferOfFAMS::fams` to compute its bounds. But allowing the bounds
>     // to be computed is the whole point of the attribute.
>     struct BufferOfFAMS* f = setup();
> 
>     // Similary doing any kind of indexing on the pointer 
>     // is extremely problematic for exactly the same reason.
>     // 
>     // The address is completely wrong because the offset is computed using
>     // `sizeof(struct HasFAM)` which won't include the array at the end.
>     //
>     // So `bogus_ptr` points to the first byte of the first `HasFAM::buffer`,
>     // instead of the second `struct HasFAM`.
>     struct HasFAM* bogus_ptr = &f->fams[1];
>     return 0;
> }
> ```

I agree that it doesn't make sense, but not every struct will have a flexible array member, of course. :-)

I guess as long as there's a followup with the support @kees mentioned above then I have no issues with this PR also.

https://github.com/llvm/llvm-project/pull/90786


More information about the cfe-commits mailing list