diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp index 66a87ba924dba2..3edbc4ab98c06e 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp @@ -443,7 +443,9 @@ void ASTResultSynthesizer::CommitPersistentDecls() { return; auto *persistent_vars = llvm::cast(state); - TypeSystemClang *scratch_ctx = ScratchTypeSystemClang::GetForTarget(m_target); + + TypeSystemClang *scratch_ctx = ScratchTypeSystemClang::GetForTarget( + m_target, m_ast_context->getLangOpts()); for (clang::NamedDecl *decl : m_decls) { StringRef name = decl->getName(); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp index cf34d9359f1182..79ee565a3fddbe 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -71,20 +71,22 @@ ClangASTSource::~ClangASTSource() { if (!m_target) return; - // We are in the process of destruction, don't create clang ast context on - // demand by passing false to - // Target::GetScratchTypeSystemClang(create_on_demand). - TypeSystemClang *scratch_clang_ast_context = - ScratchTypeSystemClang::GetForTarget(*m_target, false); - if (!scratch_clang_ast_context) - return; + // Unregister the current ASTContext as a source for all scratch + // ASTContexts in the ClangASTImporter. Without this the scratch AST might + // query the deleted ASTContext for additional type information. + // We unregister from *all* scratch ASTContexts in case a type got exported + // to a scratch AST that isn't the best fitting scratch ASTContext. + TypeSystemClang *scratch_ast = ScratchTypeSystemClang::GetForTarget( + *m_target, ScratchTypeSystemClang::DefaultAST, false); - clang::ASTContext &scratch_ast_context = - scratch_clang_ast_context->getASTContext(); + if (!scratch_ast) + return; - if (m_ast_context != &scratch_ast_context && m_ast_importer_sp) - m_ast_importer_sp->ForgetSource(&scratch_ast_context, m_ast_context); + ScratchTypeSystemClang *default_scratch_ast = + llvm::cast(scratch_ast); + // Unregister from the default scratch AST (and all sub-ASTs). + default_scratch_ast->ForgetSource(m_ast_context, *m_ast_importer_sp); } void ClangASTSource::StartTranslationUnit(ASTConsumer *Consumer) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index c455e08bc327b6..852ce3bbd3dba3 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -184,7 +184,7 @@ ClangExpressionDeclMap::TargetInfo ClangExpressionDeclMap::GetTargetInfo() { TypeFromUser ClangExpressionDeclMap::DeportType(TypeSystemClang &target, TypeSystemClang &source, TypeFromParser parser_type) { - assert(&target == ScratchTypeSystemClang::GetForTarget(*m_target)); + assert(&target == GetScratchContext(*m_target)); assert((TypeSystem *)&source == parser_type.GetTypeSystem()); assert(&source.getASTContext() == m_ast_context); @@ -222,7 +222,7 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl, if (target == nullptr) return false; - auto *clang_ast_context = ScratchTypeSystemClang::GetForTarget(*target); + auto *clang_ast_context = GetScratchContext(*target); if (!clang_ast_context) return false; @@ -260,7 +260,7 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl, if (target == nullptr) return false; - TypeSystemClang *context = ScratchTypeSystemClang::GetForTarget(*target); + TypeSystemClang *context = GetScratchContext(*target); if (!context) return false; @@ -1638,8 +1638,7 @@ void ClangExpressionDeclMap::AddOneGenericVariable(NameSearchContext &context, if (target == nullptr) return; - TypeSystemClang *scratch_ast_context = - ScratchTypeSystemClang::GetForTarget(*target); + TypeSystemClang *scratch_ast_context = GetScratchContext(*target); if (!scratch_ast_context) return; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h index 0c81d46c6c528c..a9cd5d166b9d8d 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h @@ -380,6 +380,11 @@ class ClangExpressionDeclMap : public ClangASTSource { /// Deallocate struct variables void DisableStructVars() { m_struct_vars.reset(); } + TypeSystemClang *GetScratchContext(Target &target) { + return ScratchTypeSystemClang::GetForTarget(target, + m_ast_context->getLangOpts()); + } + /// Get this parser's ID for use in extracting parser- and JIT-specific data /// from persistent variables. uint64_t GetParserID() { return (uint64_t) this; } diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 463ff2fedce9a1..f46b145da66c42 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -9558,13 +9558,42 @@ TypeSystemClang::DeclContextGetTypeSystemClang(const CompilerDeclContext &dc) { return nullptr; } +namespace { +/// A specialized scratch AST used within ScratchTypeSystemClang. +/// These are the ASTs backing the different IsolatedASTKinds. They behave +/// like a normal ScratchTypeSystemClang but they don't own their own +/// persistent storage or target reference. +class SpecializedScratchAST : public TypeSystemClang { +public: + /// \param name The display name of the TypeSystemClang instance. + /// \param triple The triple used for the TypeSystemClang instance. + /// \param ast_source The ClangASTSource that should be used to complete + /// type information. + SpecializedScratchAST(llvm::StringRef name, llvm::Triple triple, + std::unique_ptr ast_source) + : TypeSystemClang(name, triple), + m_scratch_ast_source_up(std::move(ast_source)) { + // Setup the ClangASTSource to complete this AST. + m_scratch_ast_source_up->InstallASTContext(*this); + llvm::IntrusiveRefCntPtr proxy_ast_source( + m_scratch_ast_source_up->CreateProxy()); + SetExternalSource(proxy_ast_source); + } + + /// The ExternalASTSource that performs lookups and completes types. + std::unique_ptr m_scratch_ast_source_up; +}; +} // namespace + +char ScratchTypeSystemClang::ID; +const llvm::NoneType ScratchTypeSystemClang::DefaultAST = llvm::None; + ScratchTypeSystemClang::ScratchTypeSystemClang(Target &target, llvm::Triple triple) - : TypeSystemClang("scratch ASTContext", triple), + : TypeSystemClang("scratch ASTContext", triple), m_triple(triple), m_target_wp(target.shared_from_this()), m_persistent_variables(new ClangPersistentVariables) { - m_scratch_ast_source_up = std::make_unique( - target.shared_from_this(), m_persistent_variables->GetClangASTImporter()); + m_scratch_ast_source_up = CreateASTSource(); m_scratch_ast_source_up->InstallASTContext(*this); llvm::IntrusiveRefCntPtr proxy_ast_source( m_scratch_ast_source_up->CreateProxy()); @@ -9576,8 +9605,10 @@ void ScratchTypeSystemClang::Finalize() { m_scratch_ast_source_up.reset(); } -TypeSystemClang *ScratchTypeSystemClang::GetForTarget(Target &target, - bool create_on_demand) { +TypeSystemClang * +ScratchTypeSystemClang::GetForTarget(Target &target, + llvm::Optional ast_kind, + bool create_on_demand) { auto type_system_or_err = target.GetScratchTypeSystemForLanguage( lldb::eLanguageTypeC, create_on_demand); if (auto err = type_system_or_err.takeError()) { @@ -9585,7 +9616,13 @@ TypeSystemClang *ScratchTypeSystemClang::GetForTarget(Target &target, std::move(err), "Couldn't get scratch TypeSystemClang"); return nullptr; } - return llvm::dyn_cast(&type_system_or_err.get()); + ScratchTypeSystemClang &scratch_ast = + llvm::cast(type_system_or_err.get()); + // If no dedicated sub-AST was requested, just return the main AST. + if (ast_kind == DefaultAST) + return &scratch_ast; + // Search the sub-ASTs. + return &scratch_ast.GetIsolatedAST(*ast_kind); } UserExpression *ScratchTypeSystemClang::GetUserExpression( @@ -9630,3 +9667,41 @@ PersistentExpressionState * ScratchTypeSystemClang::GetPersistentExpressionState() { return m_persistent_variables.get(); } + +void ScratchTypeSystemClang::ForgetSource(ASTContext *src_ctx, + ClangASTImporter &importer) { + // Remove it as a source from the main AST. + importer.ForgetSource(&getASTContext(), src_ctx); + // Remove it as a source from all created sub-ASTs. + for (const auto &a : m_isolated_asts) + importer.ForgetSource(&a.second->getASTContext(), src_ctx); +} + +std::unique_ptr ScratchTypeSystemClang::CreateASTSource() { + return std::make_unique( + m_target_wp.lock()->shared_from_this(), + m_persistent_variables->GetClangASTImporter()); +} + +static llvm::StringRef +GetSpecializedASTName(ScratchTypeSystemClang::IsolatedASTKind feature) { + switch (feature) { + case ScratchTypeSystemClang::IsolatedASTKind::CppModules: + return "scratch ASTContext for C++ module types"; + } + llvm_unreachable("Unimplemented ASTFeature kind?"); +} + +TypeSystemClang &ScratchTypeSystemClang::GetIsolatedAST( + ScratchTypeSystemClang::IsolatedASTKind feature) { + auto found_ast = m_isolated_asts.find(feature); + if (found_ast != m_isolated_asts.end()) + return *found_ast->second; + + // Couldn't find the requested sub-AST, so create it now. + std::unique_ptr new_ast; + new_ast.reset(new SpecializedScratchAST(GetSpecializedASTName(feature), + m_triple, CreateASTSource())); + m_isolated_asts[feature] = std::move(new_ast); + return *m_isolated_asts[feature]; +} diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 8d2c0f045c140c..f17fc622ede4d2 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -1110,6 +1110,9 @@ class TypeSystemClang : public TypeSystem { /// The TypeSystemClang instance used for the scratch ASTContext in a /// lldb::Target. class ScratchTypeSystemClang : public TypeSystemClang { + /// LLVM RTTI support + static char ID; + public: ScratchTypeSystemClang(Target &target, llvm::Triple triple); @@ -1117,15 +1120,59 @@ class ScratchTypeSystemClang : public TypeSystemClang { void Finalize() override; + /// The different kinds of isolated ASTs within the scratch TypeSystem. + /// + /// These ASTs are isolated from the main scratch AST and are each + /// dedicated to a special language option/feature that makes the contained + /// AST nodes incompatible with other AST nodes. + enum class IsolatedASTKind { + /// The isolated AST for declarations/types from expressions that imported + /// type information from a C++ module. The templates from a C++ module + /// often conflict with the templates we generate from debug information, + /// so we put these types in their own AST. + CppModules + }; + + /// Alias for requesting the default scratch TypeSystemClang in GetForTarget. + // This isn't constexpr as gtest/llvm::Optional comparison logic is trying + // to get the address of this for pretty-printing. + static const llvm::NoneType DefaultAST; + + /// Infers the appropriate sub-AST from Clang's LangOptions. + static llvm::Optional + InferIsolatedASTKindFromLangOpts(const clang::LangOptions &l) { + // If modules are activated we want the dedicated C++ module AST. + // See IsolatedASTKind::CppModules for more info. + if (l.Modules) + return IsolatedASTKind::CppModules; + return DefaultAST; + } + /// Returns the scratch TypeSystemClang for the given target. /// \param target The Target which scratch TypeSystemClang should be returned. + /// \param ast_kind Allows requesting a specific sub-AST instead of the + /// default scratch AST. See also `IsolatedASTKind`. /// \param create_on_demand If the scratch TypeSystemClang instance can be /// created by this call if it doesn't exist yet. If it doesn't exist yet and /// this parameter is false, this function returns a nullptr. /// \return The scratch type system of the target or a nullptr in case an /// error occurred. + static TypeSystemClang * + GetForTarget(Target &target, + llvm::Optional ast_kind = DefaultAST, + bool create_on_demand = true); + + /// Returns the scratch TypeSystemClang for the given target. The returned + /// TypeSystemClang will be the scratch AST or a sub-AST, depending on which + /// fits best to the passed LangOptions. + /// \param target The Target which scratch TypeSystemClang should be returned. + /// \param lang_opts The LangOptions of a clang ASTContext that the caller + /// wants to export type information from. This is used to + /// find the best matching sub-AST that will be returned. static TypeSystemClang *GetForTarget(Target &target, - bool create_on_demand = true); + const clang::LangOptions &lang_opts) { + return GetForTarget(target, InferIsolatedASTKindFromLangOpts(lang_opts)); + } UserExpression * GetUserExpression(llvm::StringRef expr, llvm::StringRef prefix, @@ -1143,7 +1190,28 @@ class ScratchTypeSystemClang : public TypeSystemClang { CreateUtilityFunction(std::string text, std::string name) override; PersistentExpressionState *GetPersistentExpressionState() override; + + /// Unregisters the given ASTContext as a source from the scratch AST (and + /// all sub-ASTs). + /// \see ClangASTImporter::ForgetSource + void ForgetSource(clang::ASTContext *src_ctx, ClangASTImporter &importer); + + // llvm casting support + bool isA(const void *ClassID) const override { + return ClassID == &ID || TypeSystemClang::isA(ClassID); + } + static bool classof(const TypeSystem *ts) { return ts->isA(&ID); } + private: + std::unique_ptr CreateASTSource(); + /// Returns the requested sub-AST. + /// Will lazily create the sub-AST if it hasn't been created before. + TypeSystemClang &GetIsolatedAST(IsolatedASTKind feature); + + /// The target triple. + /// This was potentially adjusted and might not be identical to the triple + /// of `m_target_wp`. + llvm::Triple m_triple; lldb::TargetWP m_target_wp; /// The persistent variables associated with this process for the expression /// parser. @@ -1151,6 +1219,12 @@ class ScratchTypeSystemClang : public TypeSystemClang { /// The ExternalASTSource that performs lookups and completes minimally /// imported types. std::unique_ptr m_scratch_ast_source_up; + + /// Map from IsolatedASTKind to their actual TypeSystemClang instance. + /// This map is lazily filled with sub-ASTs and should be accessed via + /// `GetSubAST` (which lazily fills this map). + std::unordered_map> + m_isolated_asts; }; } // namespace lldb_private diff --git a/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile b/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile new file mode 100644 index 00000000000000..f938f7428468ab --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/Makefile @@ -0,0 +1,3 @@ +USE_LIBCPP := 1 +CXX_SOURCES := main.cpp +include Makefile.rules diff --git a/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py b/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py new file mode 100644 index 00000000000000..0296b736bf78b3 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py @@ -0,0 +1,88 @@ +""" +Test that LLDB is separating C++ module types and debug information types +in the scratch AST. +""" + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + @add_test_categories(["libc++"]) + @skipIf(compiler=no_match("clang")) + def test(self): + """ + This test is creating ValueObjects with both C++ module and debug + info types for std::vector. We can't merge these types into + the same AST, so for this test to pass LLDB should split the types + up depending on whether they come from a module or not. + """ + self.build() + + lldbutil.run_to_source_breakpoint(self, + "// Set break point at this line.", + lldb.SBFileSpec("main.cpp")) + + children = [ + ValueCheck(value="3"), + ValueCheck(value="1"), + ValueCheck(value="2"), + ] + + # The debug info vector type doesn't know about the default template + # arguments, so they will be printed. + debug_info_vector_type = "std::vector >" + + # The C++ module vector knows its default template arguments and will + # suppress them. + module_vector_type = "std::vector" + + # First muddy the scratch AST with non-C++ module types. + self.expect_expr("a", result_type=debug_info_vector_type, + result_children=children) + self.expect_expr("dbg_info_vec", + result_type="std::vector >", + result_children=[ + ValueCheck(type="DbgInfoClass", children=[ + ValueCheck(name="ints", type=debug_info_vector_type, children=[ + ValueCheck(value="1") + ]) + ]) + ]) + + # Enable the C++ module import and get the module vector type. + self.runCmd("settings set target.import-std-module true") + self.expect_expr("a", result_type=module_vector_type, + result_children=children) + + # Test mixed debug info/module types + self.expect_expr("dbg_info_vec", + result_type="std::vector", + result_children=[ + ValueCheck(type="DbgInfoClass", children=[ + ValueCheck(name="ints", type=module_vector_type, children=[ + ValueCheck(value="1") + ]) + ]) + ]) + + + # Turn off the C++ module import and use debug info types again. + self.runCmd("settings set target.import-std-module false") + self.expect_expr("a", result_type=debug_info_vector_type, + result_children=children) + + # Test the types that were previoiusly mixed debug info/module types. + self.expect_expr("dbg_info_vec", + result_type="std::vector >", + result_children=[ + ValueCheck(type="DbgInfoClass", children=[ + ValueCheck(name="ints", type=debug_info_vector_type, children=[ + ValueCheck(value="1") + ]) + ]) + ]) diff --git a/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp b/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp new file mode 100644 index 00000000000000..1087ad832acd74 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/non-module-type-separation/main.cpp @@ -0,0 +1,17 @@ +#include + +struct DbgInfoClass { + std::vector ints; +}; + +int main(int argc, char **argv) { + std::vector a = {3, 1, 2}; + + // Create a std::vector of a class from debug info with one element. + std::vector dbg_info_vec; + dbg_info_vec.resize(1); + // And that class has a std::vector of integers that comes from the C++ + // module. + dbg_info_vec.back().ints.push_back(1); + return 0; // Set break point at this line. +} diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp index c1bbc552662f3f..0348e734d2400e 100644 --- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -730,3 +730,15 @@ TEST_F(TestTypeSystemClang, AddMethodToObjCObjectType) { EXPECT_FALSE(method->isDirectMethod()); EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo"); } + +TEST(TestScratchTypeSystemClang, InferSubASTFromLangOpts) { + LangOptions lang_opts; + EXPECT_EQ( + ScratchTypeSystemClang::DefaultAST, + ScratchTypeSystemClang::InferIsolatedASTKindFromLangOpts(lang_opts)); + + lang_opts.Modules = true; + EXPECT_EQ( + ScratchTypeSystemClang::IsolatedASTKind::CppModules, + ScratchTypeSystemClang::InferIsolatedASTKindFromLangOpts(lang_opts)); +}