[clang] 5b3247b - [tbaa] Handle base classes in struct tbaa
Jeroen Dobbelaere via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 6 05:38:12 PDT 2022
Author: Bruno De Fraine
Date: 2022-07-06T14:37:59+02:00
New Revision: 5b3247bf9f715cc1b399af1e17540b3a3ce9cdec
URL: https://github.com/llvm/llvm-project/commit/5b3247bf9f715cc1b399af1e17540b3a3ce9cdec
DIFF: https://github.com/llvm/llvm-project/commit/5b3247bf9f715cc1b399af1e17540b3a3ce9cdec.diff
LOG: [tbaa] Handle base classes in struct tbaa
This is a fix for the miscompilation reported in https://github.com/llvm/llvm-project/issues/55384
Not adding a new test case since existing test cases already cover base classes (including new-struct-path tbaa).
Reviewed By: jeroen.dobbelaere
Differential Revision: https://reviews.llvm.org/D126956
Added:
Modified:
clang/lib/CodeGen/CodeGenTBAA.cpp
clang/test/CodeGen/tbaa-class.cpp
clang/unittests/CodeGen/TBAAMetadataTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 95763d8e18b70..0cb63fbbe9e5c 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -335,7 +335,42 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) {
if (auto *TTy = dyn_cast<RecordType>(Ty)) {
const RecordDecl *RD = TTy->getDecl()->getDefinition();
const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
- SmallVector<llvm::MDBuilder::TBAAStructField, 4> Fields;
+ using TBAAStructField = llvm::MDBuilder::TBAAStructField;
+ SmallVector<TBAAStructField, 4> Fields;
+ if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ // Handle C++ base classes. Non-virtual bases can treated a a kind of
+ // field. Virtual bases are more complex and omitted, but avoid an
+ // incomplete view for NewStructPathTBAA.
+ if (CodeGenOpts.NewStructPathTBAA && CXXRD->getNumVBases() != 0)
+ return BaseTypeMetadataCache[Ty] = nullptr;
+ for (const CXXBaseSpecifier &B : CXXRD->bases()) {
+ if (B.isVirtual())
+ continue;
+ QualType BaseQTy = B.getType();
+ const CXXRecordDecl *BaseRD = BaseQTy->getAsCXXRecordDecl();
+ if (BaseRD->isEmpty())
+ continue;
+ llvm::MDNode *TypeNode = isValidBaseType(BaseQTy)
+ ? getBaseTypeInfo(BaseQTy)
+ : getTypeInfo(BaseQTy);
+ if (!TypeNode)
+ return BaseTypeMetadataCache[Ty] = nullptr;
+ uint64_t Offset = Layout.getBaseClassOffset(BaseRD).getQuantity();
+ uint64_t Size =
+ Context.getASTRecordLayout(BaseRD).getDataSize().getQuantity();
+ Fields.push_back(
+ llvm::MDBuilder::TBAAStructField(Offset, Size, TypeNode));
+ }
+ // The order in which base class subobjects are allocated is unspecified,
+ // so may
diff er from declaration order. In particular, Itanium ABI will
+ // allocate a primary base first.
+ // Since we exclude empty subobjects, the objects are not overlapping and
+ // their offsets are unique.
+ llvm::sort(Fields,
+ [](const TBAAStructField &A, const TBAAStructField &B) {
+ return A.Offset < B.Offset;
+ });
+ }
for (FieldDecl *Field : RD->fields()) {
if (Field->isZeroSize(Context) || Field->isUnnamedBitfield())
continue;
diff --git a/clang/test/CodeGen/tbaa-class.cpp b/clang/test/CodeGen/tbaa-class.cpp
index 7f413a6f323c2..8e485f7c1cb8f 100644
--- a/clang/test/CodeGen/tbaa-class.cpp
+++ b/clang/test/CodeGen/tbaa-class.cpp
@@ -51,6 +51,25 @@ class StructS2 : public StructS
uint32_t f32_2;
};
+class StructT {
+public:
+ uint32_t f32_2;
+ void foo();
+};
+class StructM1 : public StructS, public StructT {
+public:
+ uint16_t f16_2;
+};
+class StructDyn {
+public:
+ uint32_t f32_2;
+ virtual void foo();
+};
+class StructM2 : public StructS, public StructDyn {
+public:
+ uint16_t f16_2;
+};
+
uint32_t g(uint32_t *s, StructA *A, uint64_t count) {
// CHECK-LABEL: define{{.*}} i32 @_Z1g
// CHECK: store i32 1, i32* %{{.*}}, align 4, !tbaa [[TAG_i32:!.*]]
@@ -199,6 +218,30 @@ uint32_t g12(StructC *C, StructD *D, uint64_t count) {
return b1->a.f32;
}
+uint32_t g13(StructM1 *M, StructS *S) {
+ // CHECK-LABEL: define{{.*}} i32 @_Z3g13
+ // CHECK: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+ // CHECK: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+ // PATH-LABEL: define{{.*}} i32 @_Z3g13
+ // PATH: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_S_f16]]
+ // PATH: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_M1_f16_2:!.*]]
+ S->f16 = 1;
+ M->f16_2 = 4;
+ return S->f16;
+}
+
+uint32_t g14(StructM2 *M, StructS *S) {
+ // CHECK-LABEL: define{{.*}} i32 @_Z3g14
+ // CHECK: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+ // CHECK: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+ // PATH-LABEL: define{{.*}} i32 @_Z3g14
+ // PATH: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_S_f16]]
+ // PATH: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_M2_f16_2:!.*]]
+ S->f16 = 1;
+ M->f16_2 = 4;
+ return S->f16;
+}
+
// CHECK: [[TYPE_char:!.*]] = !{!"omnipotent char", [[TAG_cxx_tbaa:!.*]],
// CHECK: [[TAG_cxx_tbaa]] = !{!"Simple C++ TBAA"}
// CHECK: [[TAG_i32]] = !{[[TYPE_i32:!.*]], [[TYPE_i32]], i64 0}
@@ -222,11 +265,17 @@ uint32_t g12(StructC *C, StructD *D, uint64_t count) {
// OLD-PATH: [[TYPE_S]] = !{!"_ZTS7StructS", [[TYPE_SHORT]], i64 0, [[TYPE_INT]], i64 4}
// OLD-PATH: [[TAG_S_f16]] = !{[[TYPE_S]], [[TYPE_SHORT]], i64 0}
// OLD-PATH: [[TAG_S2_f32_2]] = !{[[TYPE_S2:!.*]], [[TYPE_INT]], i64 12}
-// OLD-PATH: [[TYPE_S2]] = !{!"_ZTS8StructS2", [[TYPE_SHORT]], i64 8, [[TYPE_INT]], i64 12}
+// OLD-PATH: [[TYPE_S2]] = !{!"_ZTS8StructS2", [[TYPE_S]], i64 0, [[TYPE_SHORT]], i64 8, [[TYPE_INT]], i64 12}
// OLD-PATH: [[TAG_C_b_a_f32]] = !{[[TYPE_C:!.*]], [[TYPE_INT]], i64 12}
// OLD-PATH: [[TYPE_C]] = !{!"_ZTS7StructC", [[TYPE_SHORT]], i64 0, [[TYPE_B]], i64 4, [[TYPE_INT]], i64 28}
// OLD-PATH: [[TAG_D_b_a_f32]] = !{[[TYPE_D:!.*]], [[TYPE_INT]], i64 12}
// OLD-PATH: [[TYPE_D]] = !{!"_ZTS7StructD", [[TYPE_SHORT]], i64 0, [[TYPE_B]], i64 4, [[TYPE_INT]], i64 28, [[TYPE_CHAR]], i64 32}
+// OLD-PATH: [[TAG_M1_f16_2]] = !{[[TYPE_M1:!.*]], [[TYPE_SHORT]], i64 12}
+// OLD-PATH: [[TYPE_M1]] = !{!"_ZTS8StructM1", [[TYPE_S]], i64 0, [[TYPE_T:!.*]], i64 8, [[TYPE_SHORT]], i64 12}
+// OLD_PATH: [[TYPE_T]] = !{!"_ZTS7StructT", [[TYPE_INT]], i64 0}
+// OLD-PATH: [[TAG_M2_f16_2]] = !{[[TYPE_M2:!.*]], [[TYPE_SHORT]], i64 20}
+// OLD-PATH: [[TYPE_M2]] = !{!"_ZTS8StructM2", [[TYPE_DYN:!.*]], i64 0, [[TYPE_S]], i64 12, [[TYPE_SHORT]], i64 20}
+// OLD_PATH: [[TYPE_DYN]] = !{!"_ZTS9StructDyn", [[TYPE_INT]], i64 8}
// NEW-PATH: [[TYPE_CHAR:!.*]] = !{!{{.*}}, i64 1, !"omnipotent char"}
// NEW-PATH: [[TAG_i32]] = !{[[TYPE_INT:!.*]], [[TYPE_INT]], i64 0, i64 4}
@@ -244,8 +293,14 @@ uint32_t g12(StructC *C, StructD *D, uint64_t count) {
// NEW-PATH: [[TYPE_S]] = !{[[TYPE_CHAR]], i64 8, !"_ZTS7StructS", [[TYPE_SHORT]], i64 0, i64 2, [[TYPE_INT]], i64 4, i64 4}
// NEW-PATH: [[TAG_S_f16]] = !{[[TYPE_S]], [[TYPE_SHORT]], i64 0, i64 2}
// NEW-PATH: [[TAG_S2_f32_2]] = !{[[TYPE_S2:!.*]], [[TYPE_INT]], i64 12, i64 4}
-// NEW-PATH: [[TYPE_S2]] = !{[[TYPE_CHAR]], i64 16, !"_ZTS8StructS2", [[TYPE_SHORT]], i64 8, i64 2, [[TYPE_INT]], i64 12, i64 4}
+// NEW-PATH: [[TYPE_S2]] = !{[[TYPE_CHAR]], i64 16, !"_ZTS8StructS2", [[TYPE_S]], i64 0, i64 8, [[TYPE_SHORT]], i64 8, i64 2, [[TYPE_INT]], i64 12, i64 4}
// NEW-PATH: [[TAG_C_b_a_f32]] = !{[[TYPE_C:!.*]], [[TYPE_INT]], i64 12, i64 4}
// NEW-PATH: [[TYPE_C]] = !{[[TYPE_CHAR]], i64 32, !"_ZTS7StructC", [[TYPE_SHORT]], i64 0, i64 2, [[TYPE_B]], i64 4, i64 24, [[TYPE_INT]], i64 28, i64 4}
// NEW-PATH: [[TAG_D_b_a_f32]] = !{[[TYPE_D:!.*]], [[TYPE_INT]], i64 12, i64 4}
// NEW-PATH: [[TYPE_D]] = !{[[TYPE_CHAR]], i64 36, !"_ZTS7StructD", [[TYPE_SHORT]], i64 0, i64 2, [[TYPE_B]], i64 4, i64 24, [[TYPE_INT]], i64 28, i64 4, [[TYPE_CHAR]], i64 32, i64 1}
+// NEW-PATH: [[TAG_M1_f16_2]] = !{[[TYPE_M1:!.*]], [[TYPE_SHORT]], i64 12, i64 2}
+// NEW-PATH: [[TYPE_M1]] = !{[[TYPE_CHAR]], i64 16, !"_ZTS8StructM1", [[TYPE_S]], i64 0, i64 8, [[TYPE_T:!.*]], i64 8, i64 4, [[TYPE_SHORT]], i64 12, i64 2}
+// NEW_PATH: [[TYPE_T]] = !{[[TYPE_CHAR]], i64 4, !"_ZTS7StructT", [[TYPE_INT]], i64 0, i64 4}
+// NEW-PATH: [[TAG_M2_f16_2]] = !{[[TYPE_M2:!.*]], [[TYPE_SHORT]], i64 20, i64 2}
+// NEW-PATH: [[TYPE_M2]] = !{[[TYPE_CHAR]], i64 24, !"_ZTS8StructM2", [[TYPE_DYN:!.*]], i64 0, i64 12, [[TYPE_S]], i64 12, i64 8, [[TYPE_SHORT]], i64 20, i64 2}
+// NEW_PATH: [[TYPE_DYN]] = !{[[TYPE_CHAR]], i64 12, !"_ZTS9StructDyn", [[TYPE_INT]], i64 8, i64 4}
diff --git a/clang/unittests/CodeGen/TBAAMetadataTest.cpp b/clang/unittests/CodeGen/TBAAMetadataTest.cpp
index 149a8e074b692..2919a35c8cedf 100644
--- a/clang/unittests/CodeGen/TBAAMetadataTest.cpp
+++ b/clang/unittests/CodeGen/TBAAMetadataTest.cpp
@@ -968,13 +968,10 @@ TEST(TBAAMetadataTest, BaseClass) {
MConstInt(0)),
MConstInt(0));
- auto ClassDerived = MMTuple(
- MMString("_ZTS7Derived"),
- MMTuple(
- MMString("short"),
- OmnipotentCharCXX,
- MConstInt(0)),
- MConstInt(4));
+ auto ClassDerived =
+ MMTuple(MMString("_ZTS7Derived"), ClassBase, MConstInt(0),
+ MMTuple(MMString("short"), OmnipotentCharCXX, MConstInt(0)),
+ MConstInt(4));
const Instruction *I = match(BB,
MInstruction(Instruction::Store,
@@ -1047,13 +1044,10 @@ TEST(TBAAMetadataTest, PolymorphicClass) {
MConstInt(0)),
MConstInt(Compiler.PtrSize));
- auto ClassDerived = MMTuple(
- MMString("_ZTS7Derived"),
- MMTuple(
- MMString("short"),
- OmnipotentCharCXX,
- MConstInt(0)),
- MConstInt(Compiler.PtrSize + 4));
+ auto ClassDerived =
+ MMTuple(MMString("_ZTS7Derived"), ClassBase, MConstInt(0),
+ MMTuple(MMString("short"), OmnipotentCharCXX, MConstInt(0)),
+ MConstInt(Compiler.PtrSize + 4));
const Instruction *I = match(BB,
MInstruction(Instruction::Store,
More information about the cfe-commits
mailing list