[PATCH] D126956: [tbaa] Handle base classes in struct tbaa

[Bruno De Fraine via Phabricator via cfe-commits]

brunodf created this revision.
brunodf added reviewers: jeroen.dobbelaere, eli.friedman.
Herald added a subscriber: kosarev.
Herald added a project: All.
brunodf requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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).


Repository:
  rG LLVM Github Monorepo

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
@@ -222,7 +222,7 @@
 // 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}
@@ -244,7 +244,7 @@
 // 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}
Index: clang/lib/CodeGen/CodeGenTBAA.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenTBAA.cpp
+++ clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -336,6 +336,30 @@
     const RecordDecl *RD = TTy->getDecl()->getDefinition();
     const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
     SmallVector<llvm::MDBuilder::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.getTypeSizeInChars(BaseQTy).getQuantity();
+        Fields.push_back(
+            llvm::MDBuilder::TBAAStructField(Offset, Size, TypeNode));
+      }
+    }
     for (FieldDecl *Field : RD->fields()) {
       if (Field->isZeroSize(Context) || Field->isUnnamedBitfield())
         continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126956.433995.patch
Type: text/x-patch
Size: 4824 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220603/89f07875/attachment.bin>