https://github.com/davidbolvansky created https://github.com/llvm/llvm-project/pull/226811
Whole-union accesses currently fall back to omnipotent-char TBAA, even with new struct-path TBAA. This loses the outer access path and can make a union copy appear to clobber an unrelated field. Represent unions as aggregate nodes in the new format, with every member at offset zero. Whole-union accesses then retain their type and range. This is sound because overlapping members are recorded at the same offset, so a whole-union access still aliases every direct and nested member. Accesses through union member expressions remain conservative, and old TBAA is unchanged. The test includes direct and nested escaped-member controls. The motivating GCC `alias-access-path-13.c` case now folds the unaffected sibling load. Tests: - `ninja check-clang-codegen` (6157 discovered, 2313 passed, 3 expected failures) - focused `tbaa.cpp`, `tbaa-struct.cpp`, and `tbaa-union-access.c` From 83dd88f03dd9024bd1cef2b34bb4df4ba0727e3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Bolvansk=C3=BD?= <[email protected]> Date: Sun, 27 Sep 2026 19:14:31 +0200 Subject: [PATCH] [Clang][TBAA] Represent unions in new struct-path TBAA --- clang/lib/CodeGen/CodeGenTBAA.cpp | 35 ++++++++++----- clang/test/CodeGen/tbaa-struct.cpp | 10 +++-- clang/test/CodeGen/tbaa-union-access.c | 61 ++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 15 deletions(-) create mode 100644 clang/test/CodeGen/tbaa-union-access.c diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp index 1854df7c7c0f11..682c7f0096dd88 100644 --- a/clang/lib/CodeGen/CodeGenTBAA.cpp +++ b/clang/lib/CodeGen/CodeGenTBAA.cpp @@ -141,7 +141,7 @@ static bool TypeHasMayAlias(QualType QTy) { } /// Check if the given type is a valid base type to be used in access tags. -static bool isValidBaseType(QualType QTy) { +static bool isValidBaseType(QualType QTy, bool NewStructPathTBAA) { if (const auto *RD = QTy->getAsRecordDecl()) { // Incomplete types are not valid base access types. if (!RD->isCompleteDefinition()) @@ -149,8 +149,8 @@ static bool isValidBaseType(QualType QTy) { if (RD->hasFlexibleArrayMember()) return false; // RD can be struct, union, class, interface or enum. - // For now, we only handle struct and class. - if (RD->isStruct() || RD->isClass()) + // The new format can represent overlapping union members. + if (RD->isStruct() || RD->isClass() || (NewStructPathTBAA && RD->isUnion())) return true; } return false; @@ -389,7 +389,7 @@ llvm::MDNode *CodeGenTBAA::getTypeInfo(QualType QTy) { // be considered may-alias too. // TODO: Combine getTypeInfo() and getValidBaseTypeInfo() into a single // function. - if (isValidBaseType(QTy)) + if (isValidBaseType(QTy, CodeGenOpts.NewStructPathTBAA)) return getValidBaseTypeInfo(QTy); const Type *Ty = Context.getCanonicalType(QTy).getTypePtr(); @@ -538,9 +538,10 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { const CXXRecordDecl *BaseRD = BaseQTy->getAsCXXRecordDecl(); if (BaseRD->isEmpty()) continue; - llvm::MDNode *TypeNode = isValidBaseType(BaseQTy) - ? getValidBaseTypeInfo(BaseQTy) - : getTypeInfo(BaseQTy); + llvm::MDNode *TypeNode = + isValidBaseType(BaseQTy, CodeGenOpts.NewStructPathTBAA) + ? getValidBaseTypeInfo(BaseQTy) + : getTypeInfo(BaseQTy); if (!TypeNode) return nullptr; uint64_t Offset = Layout.getBaseClassOffset(BaseRD).getQuantity(); @@ -563,9 +564,10 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { if (Field->isZeroSize(Context) || Field->isUnnamedBitField()) continue; QualType FieldQTy = Field->getType(); - llvm::MDNode *TypeNode = isValidBaseType(FieldQTy) - ? getValidBaseTypeInfo(FieldQTy) - : getTypeInfo(FieldQTy); + llvm::MDNode *TypeNode = + isValidBaseType(FieldQTy, CodeGenOpts.NewStructPathTBAA) + ? getValidBaseTypeInfo(FieldQTy) + : getTypeInfo(FieldQTy); if (!TypeNode) return nullptr; @@ -576,6 +578,12 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { TypeNode)); } + // New struct-path TBAA represents all union members at offset zero. Keep + // their actual types so a whole-union access aliases pointers to any of + // its members. Accesses through union member expressions remain may-alias. + assert((!RD->isUnion() || CodeGenOpts.NewStructPathTBAA) && + "Union base types require new struct-path TBAA"); + SmallString<256> OutName; if (Features.CPlusPlus) { // Don't use the mangler for C code. @@ -604,7 +612,8 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { } llvm::MDNode *CodeGenTBAA::getValidBaseTypeInfo(QualType QTy) { - assert(isValidBaseType(QTy) && "Must be a valid base type"); + assert(isValidBaseType(QTy, CodeGenOpts.NewStructPathTBAA) && + "Must be a valid base type"); const Type *Ty = Context.getCanonicalType(QTy).getTypePtr(); @@ -623,7 +632,9 @@ llvm::MDNode *CodeGenTBAA::getValidBaseTypeInfo(QualType QTy) { } llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) { - return isValidBaseType(QTy) ? getValidBaseTypeInfo(QTy) : nullptr; + return isValidBaseType(QTy, CodeGenOpts.NewStructPathTBAA) + ? getValidBaseTypeInfo(QTy) + : nullptr; } llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) { diff --git a/clang/test/CodeGen/tbaa-struct.cpp b/clang/test/CodeGen/tbaa-struct.cpp index 2776ea2e4e8619..4b8636947ab602 100644 --- a/clang/test/CodeGen/tbaa-struct.cpp +++ b/clang/test/CodeGen/tbaa-struct.cpp @@ -217,7 +217,9 @@ void copy12(UnionMember2 *a1, UnionMember2 *a2) { // CHECK-NEW: [[META8]] = !{[[META4]], i64 2, !"short"} // CHECK-NEW: [[TBAA12]] = !{[[META13:![0-9]+]], [[META13]], i64 0, i64 24} // CHECK-NEW: [[META13]] = !{[[META4]], i64 24, !"_ZTS1B", [[META4]], i64 0, i64 1, [[META7]], i64 4, i64 16, [[META3]], i64 20, i64 4} -// CHECK-NEW: [[TBAA15]] = !{[[META4]], [[META4]], i64 0, i64 12} +// CHECK-NEW: [[TBAA15]] = !{[[UNION_U:![0-9]+]], [[UNION_U]], i64 0, i64 12} +// CHECK-NEW: [[UNION_U]] = !{[[META4]], i64 12, !"_ZTS1U", [[META4]], i64 0, i64 8, [[STRUCT_S:![0-9]+]], i64 0, i64 12} +// CHECK-NEW: [[STRUCT_S]] = !{[[META4]], i64 12, !"_ZTS1S", [[META4]], i64 0, i64 2, [[META4]], i64 4, i64 8} // CHECK-NEW: [[TBAA17]] = !{[[META18:![0-9]+]], [[META18]], i64 0, i64 3} // CHECK-NEW: [[META18]] = !{[[META4]], i64 3, !"_ZTS1C", [[META4]], i64 0, i64 1, [[META4]], i64 1, i64 1, [[META4]], i64 2, i64 1} // CHECK-NEW: [[TBAA20]] = !{[[META21:![0-9]+]], [[META21]], i64 0, i64 6} @@ -231,7 +233,9 @@ void copy12(UnionMember2 *a1, UnionMember2 *a2) { // CHECK-NEW: [[TBAA33]] = !{[[META34:![0-9]+]], [[META34]], i64 0, i64 16} // CHECK-NEW: [[META34]] = !{[[META4]], i64 16, !"_ZTS15NamedBitfields3", [[META3]], i64 1, i64 4, [[META3]], i64 2, i64 4, [[META26]], i64 8, i64 8} // CHECK-NEW: [[TBAA37]] = !{[[META38:![0-9]+]], [[META38]], i64 0, i64 16} -// CHECK-NEW: [[META38]] = !{[[META4]], i64 16, !"_ZTS12UnionMember1", [[META4]], i64 0, i64 8, [[META3]], i64 8, i64 4} +// CHECK-NEW: [[META38]] = !{[[META4]], i64 16, !"_ZTS12UnionMember1", [[UNION_U2:![0-9]+]], i64 0, i64 8, [[META3]], i64 8, i64 4} +// CHECK-NEW: [[UNION_U2]] = !{[[META4]], i64 8, !"_ZTS2U2", [[META26]], i64 0, i64 8, [[FLOAT:![0-9]+]], i64 0, i64 4} +// CHECK-NEW: [[FLOAT]] = !{[[META4]], i64 4, !"float"} // CHECK-NEW: [[TBAA41]] = !{[[META42:![0-9]+]], [[META42]], i64 0, i64 16} -// CHECK-NEW: [[META42]] = !{[[META4]], i64 16, !"_ZTS12UnionMember2", [[META3]], i64 0, i64 4, [[META4]], i64 8, i64 8} +// CHECK-NEW: [[META42]] = !{[[META4]], i64 16, !"_ZTS12UnionMember2", [[META3]], i64 0, i64 4, [[UNION_U2]], i64 8, i64 8} //. diff --git a/clang/test/CodeGen/tbaa-union-access.c b/clang/test/CodeGen/tbaa-union-access.c new file mode 100644 index 00000000000000..a84030820ef123 --- /dev/null +++ b/clang/test/CodeGen/tbaa-union-access.c @@ -0,0 +1,61 @@ +// RUN: %clang_cc1 -triple x86_64-linux -O1 -emit-llvm %s -o - | \ +// RUN: FileCheck %s --check-prefix=OLD +// RUN: %clang_cc1 -triple x86_64-linux -O1 -new-struct-path-tbaa \ +// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=NEW +// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes \ +// RUN: -new-struct-path-tbaa -emit-llvm %s -o - | \ +// RUN: FileCheck %s --check-prefix=IR + +struct Pair { + int x, y; +}; +union U { + int i; + struct Pair pair; +}; +struct Outer { + union U u; + int sibling; +} *outer; +union U *dest, *src; + +// OLD-LABEL: define{{.*}} i32 @distinct_object( +// OLD: load i32, ptr +// OLD: ret i32 +// NEW-LABEL: define{{.*}} i32 @distinct_object( +// NEW-NOT: load i32, ptr +// NEW: ret i32 123 +int distinct_object(void) { + outer->sibling = 123; + *dest = *src; + return outer->sibling; +} + +// A pointer to a union member must still alias a whole-union store. +// NEW-LABEL: define{{.*}} i32 @escaped_member( +// NEW: load i32, ptr +// NEW: ret i32 +int escaped_member(void) { + int *p = &dest->i; + *p = 123; + *dest = *src; + return *p; +} + +// This also applies to pointers to nested members. +// NEW-LABEL: define{{.*}} i32 @escaped_nested_member( +// NEW: load i32, ptr +// NEW: ret i32 +int escaped_nested_member(void) { + int *p = &dest->pair.y; + *p = 123; + *dest = *src; + return *p; +} + +// IR: call void @llvm.memcpy{{.*}}, !tbaa [[TAG_U:![0-9]+]] +// IR-DAG: [[CHAR:![0-9]+]] = !{!{{[0-9]+}}, i64 1, !"omnipotent char"} +// IR-DAG: [[INT:![0-9]+]] = !{[[CHAR]], i64 4, !"int"} +// IR-DAG: [[PAIR:![0-9]+]] = !{[[CHAR]], i64 8, !"Pair", [[INT]], i64 0, i64 4, [[INT]], i64 4, i64 4} +// IR-DAG: [[UNION:![0-9]+]] = !{[[CHAR]], i64 8, !"U", [[INT]], i64 0, i64 4, [[PAIR]], i64 0, i64 8} +// IR-DAG: [[TAG_U]] = !{[[UNION]], [[UNION]], i64 0, i64 8} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
