[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