martong created this revision.
martong added reviewers: shafik, teemperor, jingham, clayborg, a_sidorin.
Herald added subscribers: lldb-commits, cfe-commits, gamesh411, Szelethus,
dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added projects: clang, LLDB.
With LLDB we use localUncachedLookup(), however, that fails to find
Decls when a transparent context is involved and the given DC has
external lexical storage. The solution is to use noload_lookup, which
works well with transparent contexts. But, we cannot use only the
noload_lookup since the slow case of localUncachedLookup is still needed
in some other cases.
These other 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().
We must avoid loading external decls during the import becuase
ExternalASTSource is implemented with ASTImporter, so external loads
during import results in uncontrolled and faulty import.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D61333
Files:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
lldb/include/lldb/Utility/Logging.h
lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Symbol/ClangASTImporter.cpp
lldb/source/Utility/Logging.cpp
Index: lldb/source/Utility/Logging.cpp
===================================================================
--- lldb/source/Utility/Logging.cpp
+++ lldb/source/Utility/Logging.cpp
@@ -17,6 +17,7 @@
static constexpr Log::Category g_categories[] = {
{{"api"}, {"log API calls and return values"}, LIBLLDB_LOG_API},
+ {{"ast"}, {"log AST"}, LIBLLDB_LOG_AST},
{{"break"}, {"log breakpoints"}, LIBLLDB_LOG_BREAKPOINTS},
{{"commands"}, {"log command argument parsing"}, LIBLLDB_LOG_COMMANDS},
{{"comm"}, {"log communication activities"}, LIBLLDB_LOG_COMMUNICATION},
Index: lldb/source/Symbol/ClangASTImporter.cpp
===================================================================
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -918,6 +918,28 @@
if (clang::TagDecl *to_tag = dyn_cast<clang::TagDecl>(to)) {
if (clang::TagDecl *from_tag = dyn_cast<clang::TagDecl>(from)) {
to_tag->setCompleteDefinition(from_tag->isCompleteDefinition());
+
+ Log *log_ast(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_AST));
+ if (log_ast) {
+ std::string name_string;
+ if (NamedDecl *from_named_decl = dyn_cast<clang::NamedDecl>(from)) {
+ llvm::raw_string_ostream name_stream(name_string);
+ from_named_decl->printName(name_stream);
+ name_stream.flush();
+ }
+ log_ast->Printf("==== [ClangASTImporter][TUDecl: %p] Imported "
+ "(%sDecl*)%p, named %s (from "
+ "(Decl*)%p)",
+ static_cast<void *>(to->getTranslationUnitDecl()),
+ from->getDeclKindName(), static_cast<void *>(to),
+ name_string.c_str(), static_cast<void *>(from));
+
+ // Log the AST of the TU.
+ std::string ast_string;
+ llvm::raw_string_ostream ast_stream(ast_string);
+ to->getTranslationUnitDecl()->dump(ast_stream);
+ log_ast->Printf("%s", ast_string.c_str());
+ }
}
}
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -637,18 +637,6 @@
m_ast_importer_sp->RequireCompleteType(copied_field_type);
}
-
- DeclContext *decl_context_non_const =
- const_cast<DeclContext *>(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);
}
}
Index: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
===================================================================
--- lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
+++ 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;
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
===================================================================
--- /dev/null
+++ 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;
+}
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
===================================================================
--- /dev/null
+++ 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.')
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
Index: lldb/include/lldb/Utility/Logging.h
===================================================================
--- lldb/include/lldb/Utility/Logging.h
+++ lldb/include/lldb/Utility/Logging.h
@@ -42,6 +42,7 @@
#define LIBLLDB_LOG_LANGUAGE (1u << 28)
#define LIBLLDB_LOG_DATAFORMATTERS (1u << 29)
#define LIBLLDB_LOG_DEMANGLE (1u << 30)
+#define LIBLLDB_LOG_AST (1u << 31)
#define LIBLLDB_LOG_ALL (UINT32_MAX)
#define LIBLLDB_LOG_DEFAULT \
(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD | LIBLLDB_LOG_DYNAMIC_LOADER | \
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -480,7 +480,7 @@
}) == FromTUs.end());
ArgVector Args = getArgVectorForLanguage(Lang);
- FromTUs.emplace_back(SrcCode, FileName, Args);
+ FromTUs.emplace_back(SrcCode, FileName, Args, Creator);
TU &Tu = FromTUs.back();
return Tu.TUDecl;
@@ -5626,6 +5626,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<CXXRecordDecl>().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<CXXRecordDecl>().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, );
@@ -5669,5 +5713,8 @@
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
+ DefaultTestValuesForRunOptions, );
+
} // end namespace ast_matchers
} // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1813,6 +1813,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, se
+ // 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();
}
@@ -7641,10 +7648,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;
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
