[clang] [BoundsSafety][Sema] Allow counted_by and counted_by_or_null on pointers where the pointee type is incomplete but potentially completable (PR #106321)

Dan Liew via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 28 12:46:33 PDT 2024


================
@@ -3862,6 +3878,27 @@ CountAttributedType::CountAttributedType(
     DeclSlot[i] = CoupledDecls[i];
 }
 
+StringRef CountAttributedType::GetAttributeName(bool WithMacroPrefix) const {
----------------
delcypher wrote:

>  but the way this works for AttributedTypes is that the Attr* is stored in the AttributedTypeLoc

How does that help in the general case? If all you have is the `AttributedType` then you can't get a `AttributedTypeLoc` from that. I suspect that's by design because I guess an attributed type could be spelled out in multiple locations in a source file.

> just store it in and then get it from the CountAttributedTypeLoc instead.

Do mean store a `CountAttributedType` pointer in `CountAttributedTypeLoc`?

In general using TypeLocs here seems like it's going to be complicated.

My naive understanding of TypeLocs is that you have to fetch those from declarations using `getTypeSourceInfo()->getTypeLoc()`, is that right or am I missing something? This means we'd have the functions that issue diagnostics traverse expressions to find the relevant decls to find the TypeLocs. That might be ok for upstream Clang where the attribute is only allowed on `FieldDecl`s but it's more complicated for our internal fork because the Attribute is allowed in many more places.

I'm also not sure how that is supposed to work with casts.

```
void test(void) {
  int count = 5;
  // How would you get a TypeLoc for a the C-style cast below? The cast isn't a decl.
  char* __counted_by(count) ptr = (char* __counted_by(count)) malloc(count);
}
```

If I've understood your suggestion correctly then what you're suggesting does sound like an improvement (that would also solve the issue this patch has with trying to get the SourceRange for the attribute) but I think the best way to proceed here would be to file an issue about this problem and refer to it in a comment in `CountAttributedType::GetAttributeName` saying that this should be implemented in a different way. That way landing this patch isn't blocked on this issue. And once I've landed this PR then I can start looking into how to fix that issue that is compatible with upstream and our internal fork and then upstream that.

Does that seem reasonable to you?


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


More information about the cfe-commits mailing list