brunodf updated this revision to Diff 441017.
brunodf added a comment.
Herald added a subscriber: mgrang.

New version that fixes the problem with base classes not appearing in order of 
increasing offset.

This issue was discovered in the multistage build, but the test case has now 
been extended for this situation.

Also changed to obtain the the size of the base subobject with `getDataSize()` 
since other subobjects may be allocated in its tail padding.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126956/new/

https://reviews.llvm.org/D126956

Files:
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/test/CodeGen/tbaa-class.cpp
  clang/unittests/CodeGen/TBAAMetadataTest.cpp

Index: clang/unittests/CodeGen/TBAAMetadataTest.cpp
===================================================================
--- clang/unittests/CodeGen/TBAAMetadataTest.cpp
+++ clang/unittests/CodeGen/TBAAMetadataTest.cpp
@@ -968,13 +968,10 @@
       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 @@
       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,
Index: clang/test/CodeGen/tbaa-class.cpp
===================================================================
--- clang/test/CodeGen/tbaa-class.cpp
+++ clang/test/CodeGen/tbaa-class.cpp
@@ -51,6 +51,25 @@
    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 @@
   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 @@
 // 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 @@
 // 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}
Index: clang/lib/CodeGen/CodeGenTBAA.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenTBAA.cpp
+++ clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -335,7 +335,40 @@
   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 differ from declaration order. In particular, Itanium ABI will
+      // allocate a primary base first.
+      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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to