AaronBallman wrote:

> > > > > > I haven't given this a thorough review, but one question (probably 
> > > > > > for @rapidsna) that comes to mind is: because `__counted_by` is a 
> > > > > > declaration or type attribute, do we have to drop the information 
> > > > > > when taking the address? I would assume that would form a pointer 
> > > > > > to a counted by FAM?
> > > > > 
> > > > > 
> > > > > @AaronBallman Yes, counted_by can be attached to T (*)[]. But that 
> > > > > would be misleading when the pointer actually gets used in most cases 
> > > > > because the element size is still zero. Pointer arithmetic still has 
> > > > > stride 0, and indexing does too. With __bdos it returns -1, which is 
> > > > > likely not what the user intended.
> > > > 
> > > > 
> > > > Why is the element size zero? You can't have an array whose element 
> > > > type is incomplete, so I think we should always know the element size?
> > > 
> > > 
> > > Sorry, I was unclear — the "element" of a pointer-to-incomplete-array 
> > > (i.e. the element for pointer arithmetic) is the incomplete array `T[]`, 
> > > not `T`. `__counted_by` would be misleading there: if it were respected, 
> > > the stride would be `sizeof(T) * n`, but in practice the stride stays 
> > > zero because `sizeof(T[])` is zero.
> > 
> > 
> > I might just be dense, but how do you get into that scenario? 
> > https://godbolt.org/z/jeEafnT9n
> > Or are you talking about pointer arithmetic on the structure type which 
> > contains the FAM?
> 
> Sorry, we're talking past each other. Concretely, given
> 
> `struct S { int len; char fam[] __counted_by(len); };` the array's element 
> type is char — complete, no issue there. The type I'm calling out is the one 
> produced by `&p->fam`, which is `char (*)[]`. Its pointee is `char[]` — the 
> incomplete array itself.

Phew, I'm following along so far. :-)

> The original issue this PR addresses is a common C idiom: given `char arr[] 
> __counted_by(n)`, programmers often write `&arr` when they meant `arr` or 
> `&arr[0]` (e.g., passing to a `void *` parameter). In plain C that's harmless 
> in practice — nobody actually does sizeof or pointer arithmetic on the 
> resulting `char (*)[]`, so no visible misbehavior. But once bounds enter the 
> picture — via __builtin_[dynamic_]object_size or -fbounds-safety — the upper 
> bound differs meaningfully between &p->fam and p->fam/&p->fam[0]: for the 
> former, the pointee is char[] (incomplete), so the upper bound is unknown; 
> for the latter, it's p->fam + n. E.g., __bdos(&p->fam, 1) returns -1, while 
> __bdos(p->fam, 1) returns the count-derived size. And &p->fam is almost 
> always a mistake, not intent.

Ahhhhh, thank you, this is what I was missing.

> Arguably, __counted_by could be honored on the incomplete-array pointee 
> itself (`char (*)[__counted_by(n)]`) to reconstruct the upper bound. I'd 
> initially thought that would be problematic because I assumed `sizeof(arr[])` 
> and `q+1` silently became stride-0 — but on rechecking, they're rejected 
> outright, so honoring the attribute on the pointee is less fraught than I 
> thought. Still, absent that work, warning users off the idiom is the 
> pragmatic call.

Yeah, this was my thinking as well; basically, `__counted_by` would actually be 
involved in all of those cases because it's providing more information which 
the type system should be free to use, so I was expecting us to treat it like 
`char (*)[__counted_by(n)]`.

If we don't go that route, then I agree the diagnostic in the PR makes sense. 
So maybe this is a temporary diagnostic while research is done to see whether 
we can make use of the extra type information?

https://github.com/llvm/llvm-project/pull/206760
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to