jankratochvil updated this revision to Diff 347315.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101236

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/Shell/Expr/no_unique_address.cpp
  lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
===================================================================
--- lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/bitfields.cpp
@@ -44,18 +44,18 @@
 // CHECK: TranslationUnitDecl {{.*}}
 // CHECK: |-CXXRecordDecl {{.*}} struct Struct definition
 // CHECK: | |-FieldDecl {{.*}} A 'int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 5
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 5
 // CHECK: | |-FieldDecl {{.*}} B 'int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 7
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 7
 // CHECK: | |-FieldDecl {{.*}} C 'unsigned int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 3
 // CHECK: | |-FieldDecl {{.*}} D 'unsigned int'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 15
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 15
 // CHECK: | |-FieldDecl {{.*}} E 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 1
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 1
 // CHECK: | |-FieldDecl {{.*}} F 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 2
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 2
 // CHECK: | |-FieldDecl {{.*}} G 'char'
-// CHECK: | | `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: | | {{.}}-IntegerLiteral {{.*}} 'int' 3
 // CHECK: | `-FieldDecl {{.*}} H 'char'
-// CHECK: |   `-IntegerLiteral {{.*}} 'int' 3
+// CHECK: |   {{.}}-IntegerLiteral {{.*}} 'int' 3
Index: lldb/test/Shell/Expr/no_unique_address.cpp
===================================================================
--- /dev/null
+++ lldb/test/Shell/Expr/no_unique_address.cpp
@@ -0,0 +1,15 @@
+// Test LLDB does not assert on miscalculated field offsets.
+
+// RUN: %clang_host %s -g -c -o %t.o
+// RUN: %lldb %t.o -o "p c.c" -o exit | FileCheck %s
+
+// CHECK: (lldb) p c.c
+// CHECK-NEXT: (char) $0 = '\0'
+
+struct A {};
+struct B {
+  [[no_unique_address]] A a;
+};
+struct C : B {
+  char c;
+} c;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7196,6 +7196,7 @@
     if (bit_width)
       field->setBitWidth(bit_width);
     SetMemberOwningModule(field, record_decl);
+    field->addAttr(NoUniqueAddressAttr::CreateImplicit(ast->getASTContext()));
 
     if (name.empty()) {
       // Determine whether this field corresponds to an anonymous struct or
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6357,6 +6357,92 @@
             ToD->getTemplateSpecializationKind());
 }
 
+struct LLDBMinimalImport : ASTImporterOptionSpecificTestBase {
+  LLDBMinimalImport() {
+    Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+                 ASTContext &FromContext, FileManager &FromFileManager,
+                 bool MinimalImport,
+                 const std::shared_ptr<ASTImporterSharedState> &SharedState) {
+      auto retval = new ASTImporter(ToContext, ToFileManager, FromContext,
+                                    FromFileManager, true /*MinimalImport*/,
+                                    // We use the regular lookup.
+                                    /*SharedState=*/nullptr);
+      retval->setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
+      return retval;
+    };
+  }
+};
+
+TEST_P(LLDBMinimalImport, LLDBImportNoUniqueAddress) {
+  TranslationUnitDecl *FromTU = getTuDecl(
+      R"(
+// lldb/test/API/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
+template <typename T> struct DWrapper {};
+
+struct D {};
+
+namespace NS {
+typedef DWrapper<D> DW;
+}
+
+struct B {
+  NS::DW spd;
+  int a = 0;
+};
+
+struct E {
+  E(B &b) : b_ref(b) {}
+  NS::DW f() { return {}; };
+  void g() {
+    return; //%self.expect("p b_ref", substrs=['(B) $0 =', '(spd = NS::DW', 'a = 0)'])
+  }
+
+  B &b_ref;
+};
+B b;
+E e(b);
+      )",
+      Lang_CXX11);
+
+  auto *FromB = FirstDeclMatcher<CXXRecordDecl>().match(
+      FromTU, cxxRecordDecl(hasName("B")));
+
+#if 0
+  // Set up a stub external storage.
+  FromTU->setHasExternalLexicalStorage(true);
+  // Set up DeclContextBits.HasLazyExternalLexicalLookups to true.
+  FromTU->setMustBuildLookupTable();
+  struct TestExternalASTSource : ExternalASTSource {};
+  FromTU->getASTContext().setExternalSource(new TestExternalASTSource());
+#endif
+
+  TranslationUnitDecl *ToTU = getToTuDecl(
+      R"(
+struct B;
+      )",
+      Lang_CXX11);
+  auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
+      ToTU, cxxRecordDecl(hasName("B")));
+
+#if 0
+  // Set up a stub external storage.
+  ToTU->setHasExternalLexicalStorage(true);
+  // Set up DeclContextBits.HasLazyExternalLexicalLookups to true.
+  ToTU->setMustBuildLookupTable();
+  struct TestExternalASTSource : ExternalASTSource {};
+  ToTU->getASTContext().setExternalSource(new TestExternalASTSource());
+#endif
+
+  // With this line commented out findFromTU(FromB)->Importer == nullptr
+  // With this line uncommented: clang/lib/AST/ASTImporter.cpp:9383: clang::Decl *clang::ASTImporter::MapImported(clang::Decl *, clang::Decl *): Assertion `(Pos == ImportedDecls.end() || Pos->second == To) && "Try to import an already imported Decl"' failed.
+  auto *ImportedB = Import(FromB, Lang_CXX11);
+  findFromTU(FromB)->Importer->MapImported(FromB, ToB);
+  findFromTU(FromB)->Importer->Imported(FromB, ToB);
+  llvm::Error Err = findFromTU(FromB)->Importer->ImportDefinition(FromB);
+  EXPECT_FALSE((bool)Err);
+  consumeError(std::move(Err));
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 
@@ -6431,5 +6517,8 @@
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportWithExternalSource,
                          DefaultTestValuesForRunOptions);
 
+INSTANTIATE_TEST_SUITE_P(ParameterizedTests, LLDBMinimalImport,
+                         DefaultTestValuesForRunOptions);
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3675,6 +3675,30 @@
                               D->getInClassInitStyle()))
     return ToField;
 
+  // FieldDecl::isZeroSize may need to check these.
+  if (const Attr *FromAttr = D->getAttr<NoUniqueAddressAttr>()) {
+    if (auto ToAttrOrErr = Importer.Import(FromAttr))
+      ToField->addAttr(*ToAttrOrErr);
+    else
+      return ToAttrOrErr.takeError();
+#if 0
+    RecordType *FromRT =
+        const_cast<RecordType *>(D->getType()->getAs<RecordType>());
+    // Is this field a struct? FieldDecl::isZeroSize will need its definition.
+    if (FromRT) {
+      RecordDecl *FromRD = FromRT->getDecl();
+      RecordType *ToRT =
+          const_cast<RecordType *>(ToField->getType()->getAs<RecordType>());
+      assert(ToRT && "RecordType import has different type");
+      RecordDecl *ToRD = ToRT->getDecl();
+      if (FromRD->isCompleteDefinition() && !ToRD->isCompleteDefinition()) {
+        if (Error Err = ImportDefinition(FromRD, ToRD, IDK_Basic))
+          return std::move(Err);
+      }
+    }
+#endif
+  }
+
   ToField->setAccess(D->getAccess());
   ToField->setLexicalDeclContext(LexicalDC);
   if (ToInitializer)
@@ -8451,6 +8475,8 @@
   // Make sure that ImportImpl registered the imported decl.
   assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
 
+  // There may have been NoUniqueAddressAttr for FieldDecl::isZeroSize.
+  ToD->dropAttrs();
   if (FromD->hasAttrs())
     for (const Attr *FromAttr : FromD->getAttrs()) {
       auto ToAttrOrErr = Import(FromAttr);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to