rupprecht created this revision.
rupprecht added reviewers: labath, teemperor.
Herald added a reviewer: shafik.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This reverts commit 3bf96b0329be554c67282b0d7d8da6a864b9e38f
<https://reviews.llvm.org/rG3bf96b0329be554c67282b0d7d8da6a864b9e38f>. It
causes crashes as reported in PR52257 and a few other places. A reproducer is
bundled with this commit to verify any fix forward.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113449
Files:
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp
Index: lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp
===================================================================
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/PR52257.cpp
@@ -0,0 +1,21 @@
+// Verify that LLDB doesn't crash. See llvm.org/PR52257.
+
+// RUN: %clangxx_host -g -c %s -o %t.o
+// RUN: %lldb -b -o "print b" %t.o | FileCheck %s
+
+// CHECK: (lldb) print b
+// CHECK: (B) $0 = {
+// CHECK: tag_set_ = nullptr
+// CHECK: }
+
+template <typename> struct pair {};
+struct A {
+ using iterator = pair<char *>;
+ pair<char *> a_[];
+};
+struct B {
+ using iterator = A::iterator;
+ iterator begin();
+ A *tag_set_;
+};
+B b;
Index: lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
===================================================================
--- lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
+++ /dev/null
@@ -1,23 +0,0 @@
-struct Outer {
- typedef int HookToOuter;
- // When importing this type, we have to import all of it before adding it
- // via the FieldDecl to 'Outer'. If we don't do this, then Clang will
- // while adding 'NestedClassMember' ask for the full type of 'NestedClass'
- // which then will start pulling in the 'RefToOuter' member. That member
- // will import the typedef above and add it to 'Outer'. And adding a
- // Decl to a DeclContext that is currently already in the process of adding
- // another Decl will cause an inconsistent lookup.
- struct NestedClass {
- HookToOuter RefToOuter;
- int SomeMember;
- } NestedClassMember;
-};
-
-// We query the members of base classes of a type by doing a lookup via Clang.
-// As this tests is trying to find a borked lookup, we need a base class here
-// to make our 'GetChildMemberWithName' use the Clang lookup.
-struct In : Outer {};
-
-In test_var;
-
-int main() { return test_var.NestedClassMember.SomeMember; }
Index: lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
===================================================================
--- lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
+++ /dev/null
@@ -1,16 +0,0 @@
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestCase(TestBase):
-
- mydir = TestBase.compute_mydir(__file__)
-
- def test(self):
- self.build()
- self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
- test_var = self.expect_expr("test_var", result_type="In")
- nested_member = test_var.GetChildMemberWithName('NestedClassMember')
- self.assertEqual("Outer::NestedClass",
- nested_member.GetType().GetName())
Index: lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
===================================================================
--- lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-CXX_SOURCES := main.cpp
-
-include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -479,7 +479,10 @@
decl->getDeclKindName(), ast_dump);
}
- CopyDecl(decl);
+ Decl *copied_decl = CopyDecl(decl);
+
+ if (!copied_decl)
+ continue;
// FIXME: We should add the copied decl to the 'decls' list. This would
// add the copied Decl into the DeclContext and make sure that we
@@ -489,6 +492,12 @@
// lookup issues later on.
// We can't just add them for now as the ASTImporter already added the
// decl into the DeclContext and this would add it twice.
+
+ if (FieldDecl *copied_field = dyn_cast<FieldDecl>(copied_decl)) {
+ QualType copied_field_type = copied_field->getType();
+
+ m_ast_importer_sp->RequireCompleteType(copied_field_type);
+ }
} else {
SkippedDecls = true;
}
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -888,37 +888,6 @@
LLDB_LOG(log, "[ClangASTImporter] Complete definition not found");
}
- // Disable the minimal import for fields that have record types. There is
- // no point in minimally importing the record behind their type as Clang
- // will anyway request their definition when the FieldDecl is added to the
- // RecordDecl (as Clang will query the FieldDecl's type for things such
- // as a deleted constexpr destructor).
- // By importing the type ahead of time we avoid some corner cases where
- // the FieldDecl's record is importing in the middle of Clang's
- // `DeclContext::addDecl` logic.
- if (clang::FieldDecl *fd = dyn_cast<FieldDecl>(From)) {
- // This is only necessary because we do the 'minimal import'. Remove this
- // once LLDB stopped using that mode.
- assert(isMinimalImport() && "Only necessary for minimal import");
- QualType field_type = fd->getType();
- if (field_type->isRecordType()) {
- // First get the underlying record and minimally import it.
- clang::TagDecl *record_decl = field_type->getAsTagDecl();
- llvm::Expected<Decl *> imported = Import(record_decl);
- if (!imported)
- return imported.takeError();
- // Check how/if the import got redirected to a different AST. Now
- // import the definition of what was actually imported. If there is no
- // origin then that means the record was imported by just picking a
- // compatible type in the target AST (in which case there is no more
- // importing to do).
- if (clang::Decl *origin = m_master.GetDeclOrigin(*imported).decl) {
- if (llvm::Error def_err = ImportDefinition(record_decl))
- return std::move(def_err);
- }
- }
- }
-
return ASTImporter::ImportImpl(From);
}
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
