diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1823,6 +1823,13 @@ if (Error Err = ImportDeclContext(From, /*ForceImport=*/true)) return Err; + // There are cases in LLDB when we first import a class without its members. + // The class will have DefinitionData, but no members. Then, + // importDefinition is called from LLDB, which tries to get the members, so + // when we get here, the class already has the DefinitionData set, so we must + // unset the CompleteDefinition here to be able to complete again the + // definition. + To->setCompleteDefinition(false); To->completeDefinition(); return Error::success(); } @@ -7700,10 +7707,20 @@ LookupTable->lookup(ReDC, Name); return FoundDeclsTy(LookupResult.begin(), LookupResult.end()); } else { - // FIXME Can we remove this kind of lookup? - // Or lldb really needs this C/C++ lookup? - FoundDeclsTy Result; - ReDC->localUncachedLookup(Name, Result); + DeclContext::lookup_result NoloadLookupResult = ReDC->noload_lookup(Name); + FoundDeclsTy Result(NoloadLookupResult.begin(), NoloadLookupResult.end()); + // We must search by the slow case of localUncachedLookup because that is + // working even if there is no LookupPtr for the DC. We could use + // DC::buildLookup() to create the LookupPtr, but that would load external + // decls again, we must avoid that case. + // Also, even if we had the LookupPtr, we must find Decls which are not + // in the LookupPtr, so we need the slow case. + // These cases are handled in ASTImporterLookupTable, but we cannot use + // that with LLDB since that traverses through the AST which initiates the + // load of external decls again via DC::decls(). And again, we must avoid + // loading external decls during the import. + if (Result.empty()) + ReDC->localUncachedLookup(Name, Result); return Result; } } diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -4583,6 +4583,50 @@ EXPECT_EQ(Imported->getPreviousDecl(), Friend); } +struct LLDBLookupTest : ASTImporterOptionSpecificTestBase { + LLDBLookupTest() { + Creator = [](ASTContext &ToContext, FileManager &ToFileManager, + ASTContext &FromContext, FileManager &FromFileManager, + bool MinimalImport, ASTImporterLookupTable *LookupTable) { + return new ASTImporter(ToContext, ToFileManager, FromContext, + FromFileManager, MinimalImport, + // We use the regular lookup. + /*LookupTable=*/nullptr); + }; + } +}; + +TEST_P(LLDBLookupTest, ImporterShouldFindInTransparentContext) { + TranslationUnitDecl *ToTU = getToTuDecl( + R"( + extern "C" { + class X{}; + }; + )", + Lang_CXX); + auto *ToX = FirstDeclMatcher().match( + ToTU, cxxRecordDecl(hasName("X"))); + + // 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()); + + Decl *FromTU = getTuDecl( + R"( + class X; + )", + Lang_CXX); + auto *FromX = FirstDeclMatcher().match( + FromTU, cxxRecordDecl(hasName("X"))); + auto *ImportedX = Import(FromX, Lang_CXX); + // The lookup must find the existing class definition in the LinkageSpecDecl. + // Then the importer renders the existing and the new decl into one chain. + EXPECT_EQ(ImportedX->getCanonicalDecl(), ToX->getCanonicalDecl()); +} + INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest, DefaultTestValuesForRunOptions, ); @@ -4626,5 +4670,8 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest, + DefaultTestValuesForRunOptions, ); + } // end namespace ast_matchers } // end namespace clang diff --git a/lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile b/lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile new file mode 100644 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile @@ -0,0 +1,5 @@ +LEVEL = ../../../make + +C_SOURCES := main.c + +include $(LEVEL)/Makefile.rules diff --git a/lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py b/lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py new file mode 100644 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py @@ -0,0 +1,77 @@ +"""Test that importing modules in C works as expected.""" + +from __future__ import print_function + + +from distutils.version import StrictVersion +import os +import time +import platform + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class CModulesTestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + @skipIfFreeBSD + @skipIfLinux + @skipIfWindows + @skipIfNetBSD + @skipIf(macos_version=["<", "10.12"]) + def test_expr(self): + self.build() + exe = self.getBuildArtifact("a.out") + self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + + lldbutil.run_break_set_by_file_and_line( + self, "main.c", self.line, num_expected_locations=1, loc_exact=True) + + self.runCmd("run", RUN_SUCCEEDED) + + # The stop reason of the thread should be breakpoint. + self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT, + substrs=['stopped', + 'stop reason = breakpoint']) + + # The breakpoint should have a hit count of 1. + self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE, + substrs=[' resolved, hit count = 1']) + + # Enable logging of the imported AST. + log_file = os.path.join(self.getBuildDir(), "lldb-ast-log.txt") + self.runCmd("log enable lldb ast -f '%s'" % log_file) + + self.expect( + "expr -l objc++ -- @import Darwin; 3", + VARIABLES_DISPLAYED_CORRECTLY, + substrs=[ + "int", + "3"]) + + # This expr command imports __sFILE with definition + # (FILE is a typedef to __sFILE.) + self.expect( + "expr *fopen(\"/dev/zero\", \"w\")", + VARIABLES_DISPLAYED_CORRECTLY, + substrs=[ + "FILE", + "_close"] + ) + + # Check that the AST log contains exactly one definition of __sFILE. + f = open(log_file) + log_lines = f.readlines() + f.close() + os.remove(log_file) + self.assertEqual(" ".join(log_lines).count("struct __sFILE definition"), 1) + + def setUp(self): + # Call super's setUp(). + TestBase.setUp(self) + # Find the line number to break inside main(). + self.line = line_number('main.c', '// Set breakpoint 0 here.') diff --git a/lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c b/lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c new file mode 100644 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c @@ -0,0 +1,5 @@ +int main() +{ + int a = 0; // Set breakpoint 0 here. + return 0; +} diff --git a/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c b/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c --- a/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c +++ b/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c @@ -5,11 +5,11 @@ typedef struct { int a; int b; -} FILE; +} MYFILE; int main() { - FILE *myFile = malloc(sizeof(FILE)); + MYFILE *myFile = malloc(sizeof(MYFILE)); myFile->a = 5; myFile->b = 9; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -609,6 +609,8 @@ if (!original_decl_context) return; + // Indicates whether we skipped any Decls of the original DeclContext. + bool SkippedDecls = false; for (TagDecl::decl_iterator iter = original_decl_context->decls_begin(); iter != original_decl_context->decls_end(); ++iter) { Decl *decl = *iter; @@ -637,21 +639,22 @@ m_ast_importer_sp->RequireCompleteType(copied_field_type); } - - DeclContext *decl_context_non_const = - const_cast(decl_context); - - if (copied_decl->getDeclContext() != decl_context) { - if (copied_decl->getDeclContext()->containsDecl(copied_decl)) - copied_decl->getDeclContext()->removeDecl(copied_decl); - copied_decl->setDeclContext(decl_context_non_const); - } - - if (!decl_context_non_const->containsDecl(copied_decl)) - decl_context_non_const->addDeclInternal(copied_decl); + } else { + SkippedDecls = true; } } + // CopyDecl may build a lookup table which may set up ExternalLexicalStorage + // to false. However, since we skipped some of the external Decls we must + // set it back! + if (SkippedDecls) { + decl_context->setHasExternalLexicalStorage(true); + // This sets HasLazyExternalLexicalLookups to true. By setting this bit we + // ensure that the lookup table is rebuilt, which means the external source + // is consulted again when a clang::DeclContext::lookup is called. + const_cast(decl_context)->setMustBuildLookupTable(); + } + return; }