Skip to content

[TBAA] Don't emit pointer tbaa for unnamed structs or unions. #116596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fhahn merged 3 commits into from
Nov 21, 2024
Merged

[TBAA] Don't emit pointer tbaa for unnamed structs or unions. #116596

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df94602
[TBAA] Don't emit pointer tbaa for unnamed structs or unions.
fhahn Nov 17, 2024
fe01d7e
!fixup add clarifying note as suggested, thanks!
fhahn Nov 20, 2024
796ccf8
!fixup Move up getAs<>() as suggested
fhahn Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions clang/lib/CodeGen/CodeGenTBAA.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,21 @@ llvm::MDNode *CodeGenTBAA::getTypeInfoHelper(const Type *Ty) {
if (!Ty->isRecordType())
return AnyPtr;

// For unnamed structs or unions C's compatible types rule applies. Two
// compatible types in different compilation units can have different
// mangled names, meaning the metadata emitted below would incorrectly
// mark them as no-alias. Use AnyPtr for such types in both C and C++, as
// C and C++ types may be visible when doing LTO.
Copy link
Contributor

@rjmccall rjmccall Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like this to note that

  • using AnyPtr is over-conservative here, and we could make this summarize the members as per the C compatibility rule in the future and
  • this also covers anonymous structs and unions, which have a different compatibility rule, but it doesn't matter because you can never have a pointer to an anonymous struct or union.

Copy link
Contributor Author

@fhahn fhahn Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added the clarifications as suggested!

//
// Note that using AnyPtr is overly conservative. We could summarize the
// members of the type, as per the C compatibility rule in the future.
// This also covers anonymous structs and unions, which have a different
// compatibility rule, but it doesn't matter because you can never have a
// pointer to an anonymous struct or union.
const auto *RT = Ty->getAs<RecordType>();
if (RT && !RT->getDecl()->getDeclName())
Copy link
Contributor

@rjmccall rjmccall Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This getAs<> is now redundant with the isRecordType() above; please do the getAs<> above and test RT there, and then you can assume you have a record here.

Copy link
Contributor Author

@fhahn fhahn Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

return AnyPtr;

// For non-builtin types use the mangled name of the canonical type.
llvm::raw_svector_ostream TyOut(TyName);
MangleCtx->mangleCanonicalTypeName(QualType(Ty, 0), TyOut);
Expand Down
5 changes: 1 addition & 4 deletions clang/test/CodeGen/tbaa-pointers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,6 @@ typedef struct {
int i1;
} TypedefS;

// FIXME: The !tbaa tag for unnamed structs doesn't account for compatible
// types in C.
void unamed_struct_typedef(TypedefS *ptr) {
// COMMON-LABEL: define void @unamed_struct_typedef(
// COMMON-SAME: ptr noundef [[PTRA:%.+]])
Expand Down Expand Up @@ -238,5 +236,4 @@ void unamed_struct_typedef(TypedefS *ptr) {
// DEFAULT: [[S2_TY]] = !{!"S2", [[ANY_POINTER]], i64 0}
// COMMON: [[INT_TAG]] = !{[[INT_TY:!.+]], [[INT_TY]], i64 0}
// COMMON: [[INT_TY]] = !{!"int", [[CHAR]], i64 0}
// ENABLED: [[P1TYPEDEF]] = !{[[P1TYPEDEF_TY:!.+]], [[P1TYPEDEF_TY]], i64 0}
// ENABLED: [[P1TYPEDEF_TY]] = !{!"p1 _ZTS8TypedefS", [[ANY_POINTER]], i64 0}
// ENABLED: [[P1TYPEDEF]] = !{[[ANY_POINTER]], [[ANY_POINTER]], i64 0}
Loading