[PATCH] D157445: [CodeGen][UBSan] Add support for handling attributed functions in getUBSanFunctionTypeHash.
rapidsna requested changes to this revision. rapidsna added a comment. This revision now requires changes to proceed. We should add a test to exercise when `Ty` is wrapped by other sugar types. Could you try with `typedef`? Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:573-575 + if (isa(Ty)) +Ty = Ty.getDesugaredType(getContext()); + Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:578 // noexcept function through a non-noexcept pointer. if (!isa(Ty)) Ty = getContext().getFunctionTypeWithExceptionSpec(Ty, EST_None); Checking `isa` still doesn't handle cases where `Ty` is wrapped in other sugar types (not just `AttributedType`). Instead of adding `if (isa(Ty))` above, I would use `Ty->isFunctionNoProtoType()` here. `isFunctionNoProtoType` is a helper function that uses `getAs()`. `getAs` removes any existing sugar until it reaches `T` or a non-sugar type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157445/new/ https://reviews.llvm.org/D157445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157445: [CodeGen][UBSan] Add support for handling attributed functions in getUBSanFunctionTypeHash.
rapidsna accepted this revision. rapidsna added a comment. This revision is now accepted and ready to land. The changes look good to me. Ideally, we could add tests with multiple attributes and possibly with other sugar types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157445/new/ https://reviews.llvm.org/D157445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148381: [Clang] Implement the 'counted_by' attribute
rapidsna added a comment.
struct V {
int Sizes[2];
int FAM[] __counted_by(Sizes[0]); // Thoughts?
};
`-fbounds-safety` doesn't allow this. In our internal adoption experience, we
haven't encountered such use cases yet. So, I think it's best to make the model
restrictive to avoid surprises. If we were to support it, I think it should at
least be limited to cases where the array subscript expression is known to be
in bounds at compile time, to avoid an OOB access when the counted_by argument
is evaluated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148381/new/
https://reviews.llvm.org/D148381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148381: [WIP][Clang] Add counted_by attribute
rapidsna added a comment.
> Future additions will include supporting, FAMs and counts in
sub-structures.
For sub-structures, are you referring to cases like this?
struct outer {
struct {
int count;
} sub;
int flex[__attribute__((counted_by(sub.count)))];
};
For `-fbounds-safety` we would limit it to anonymous sub-structs only because
otherwise the code can create a pointer to sub-struct and its count can easily
be updated to an invalid count. Were you considering the same restrictions for
this?
void foo(struct outer *p1, struct sub *p2) {
p1 = (int*)malloc(sizeof(int) *10);
p1->count = 10;
p2->count = INT_MAX;
}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148381/new/
https://reviews.llvm.org/D148381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148381: [WIP][Clang] Add counted_by attribute
rapidsna added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:7209 +/* ... */ +struct bar *fam[] __attribute__((counted_by(num_fam_elements))); + }; I don't think it's necessary for this patch but for [[ https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/113?u=rapidsna | -fbounds-safety ]], we'd eventually turn this into a type attribute that we will use to create a sugared type or qualified type. And we'd make the attribute work for **both** this position and inside the array bracket (e.g., `fam[__counted_by(n)]`). Would it work for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148381/new/ https://reviews.llvm.org/D148381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148381: [WIP][Clang] Add counted_by attribute
rapidsna added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:17931
+ auto It = llvm::find_if(RD->fields(), [&](const FieldDecl *Field) {
+return Field->getName() == ECA->getCountedByField()->getName();
+ });
What happens in this corner case where the `Field` is actually the field that
the attribute appertains to?
```
struct foo {
int count;
int counted[__attribute__((counted_by(counted)))];
};
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148381/new/
https://reviews.llvm.org/D148381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
