jankratochvil updated this revision to Diff 347475.
jankratochvil added a comment.
Update of the testcase but it still does not reproduce the LLDB problem.
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,78 @@
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 {};
+typedef DWrapper<void> DW;
+struct B {
+ DW spd;
+} b;
+struct E {
+ B &b_ref = b;
+ static DW f() { return {}; }
+} e;
+ )",
+ Lang_CXX11);
+
+ auto *FromE = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("E")));
+
+#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 E;
+ )",
+ Lang_CXX11);
+ auto *ToE = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("E")));
+
+#if 1
+ // 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(FromE)->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(FromE, Lang_CXX11);
+ // With this line commented out the testcase PASSes despite it should not.
+// findFromTU(FromE)->Importer->MapImported(FromE, ToE);
+ findFromTU(FromE)->Importer->Imported(FromE, ToE);
+ llvm::Error Err = findFromTU(FromE)->Importer->ImportDefinition(FromE);
+ EXPECT_FALSE((bool)Err);
+ consumeError(std::move(Err));
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
@@ -6431,5 +6503,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits