From ff8fdb915a6cea3bcc3057dd2ee718d0844fdfe9 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 19 Mar 2024 15:05:13 +0000 Subject: [PATCH 1/2] [lldb][Type Completion] Load C++ methods lazily from DWARF Note, this is a no-op when the LLDB setting `plugin.typesystem.clang.experimental-redecl-completion` is not set (which is currently the default). So this patch has no affect unless the user explicitly opts into it. The type-completion rework (aka redecl-completion) implemented in https://github.com/apple/llvm-project/pull/8222 comes with a large performance penalty, since we now eagerly complete `RecordType`s. Completing a `RecordType` previously unconditionally resolved all member function of said type. With redecl-completion completion, however, this meant we were now pulling in many more definitions than needed. Without redecl-completion, this isn't a problem, since importing method parameters is cheap (they are imported minimally), so we wouldn't notice that we always resolved all member functions. This patch tries to load methods lazily when in redecl-completion mode. We do this by introducing a new `ExternalASTSource::FindExternalVisibleMethods` API which Clang parses a member access expression. The callback into LLDB will do a `FindFunctions` call for all methods with the method name we're looking for, filters out any mismatches, and lets Clang continue with it's parsing. We still load following methods eagerly: 1. virtual functions: currently overrides are resolved in `CompleteRecordType` 2. operators: currently I couldn't find a point at which Clang can call into LLDB here to facilitate lazy loading 3. ctors/dtors: same reason as (2) In our benchmark harness, we saw this patch bring down redecl-completion expression evaluation on-par with top-of-tree expression evaluation. --- clang/include/clang/AST/ExternalASTSource.h | 5 + clang/lib/Sema/SemaExprMember.cpp | 7 + .../ExpressionParser/Clang/ClangASTSource.cpp | 121 ++++++++++++++++++ .../ExpressionParser/Clang/ClangASTSource.h | 17 +++ .../Clang/ClangExpressionDeclMap.cpp | 7 + .../Clang/ClangExpressionDeclMap.h | 2 + .../Clang/ClangExternalASTSourceCallbacks.h | 5 + .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 30 ++++- .../TestCppFunctionRefQualifiers.py | 16 +-- 9 files changed, 200 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h index 8e573965b0a33..fdb02ff84f97a 100644 --- a/clang/include/clang/AST/ExternalASTSource.h +++ b/clang/include/clang/AST/ExternalASTSource.h @@ -150,6 +150,11 @@ class ExternalASTSource : public RefCountedBase { virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name); + virtual bool FindExternalVisibleMethodsByName(const DeclContext *DC, + DeclarationName Name) { + return false; + } + /// Ensures that the table of all visible declarations inside this /// context is up to date. /// diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp index 3d14ca3859bb6..5609e56bb810d 100644 --- a/clang/lib/Sema/SemaExprMember.cpp +++ b/clang/lib/Sema/SemaExprMember.cpp @@ -685,6 +685,13 @@ static bool LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R, } } + if (ExternalASTSource *Source = + DC->getParentASTContext().getExternalSource()) { + if (auto LookupName = R.getLookupName()) { + Source->FindExternalVisibleMethodsByName(DC, LookupName); + } + } + // The record definition is complete, now look up the member. SemaRef.LookupQualifiedName(R, DC, SS); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp index 9f8cc243e5174..5a8544fdeae71 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -11,6 +11,7 @@ #include "ClangDeclVendor.h" #include "ClangModulesDeclVendor.h" +#include "NameSearchContext.h" #include "lldb/Core/Module.h" #include "lldb/Core/ModuleList.h" #include "lldb/Symbol/CompilerDeclContext.h" @@ -21,6 +22,7 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" #include "clang/Basic/SourceManager.h" #include "Plugins/ExpressionParser/Clang/ClangUtil.h" @@ -615,6 +617,125 @@ bool ClangASTSource::IgnoreName(const ConstString name, name_string_ref.startswith("_$"); } +bool ClangASTSource::FindExternalVisibleMethodsByName( + const clang::DeclContext *DC, clang::DeclarationName Name) { + if (!TypeSystemClang::UseRedeclCompletion()) + return true; + + SmallVector decls; + NameSearchContext context(*m_clang_ast_context, decls, Name, DC); + FindExternalVisibleMethods(context); + + return true; +} + +void ClangASTSource::FindExternalVisibleMethods(NameSearchContext &context) { + assert(m_ast_context); + + const ConstString name(context.m_decl_name.getAsString().c_str()); + CompilerDeclContext namespace_decl; + FindExternalVisibleMethods(context, lldb::ModuleSP(), namespace_decl); +} + +bool ClangASTSource::CompilerDeclContextsMatch( + CompilerDeclContext candidate_decl_ctx, DeclContext const *context, + TypeSystemClang &ts) { + auto CDC1 = candidate_decl_ctx.GetTypeSystem()->DeclContextGetCompilerContext( + candidate_decl_ctx.GetOpaqueDeclContext()); + + // Member functions have at least 2 entries (1 + // for method name, 1 for parent class) + assert(CDC1.size() > 1); + + // drop last entry (which is function name) + CDC1.pop_back(); + + const auto CDC2 = ts.DeclContextGetCompilerContext( + const_cast(context)); + + // Quick by-name check of the entire context hierarchy. + if (CDC1 == CDC2) + return true; + + // Otherwise, check whether the 'candidate_decl_ctx' is a base class of + // 'context'. + auto const *candidate_context = + (static_cast( + candidate_decl_ctx.GetOpaqueDeclContext())) + ->getParent(); + + auto const *candidate_cxx_record = + dyn_cast(candidate_context); + if (!candidate_cxx_record) + return false; + + auto const *cxx_record = dyn_cast(context); + if (!cxx_record) + return false; + + // cxx_record comes from user expression AST. So we need to get origin + // to compare against candidate_context. + auto orig = GetDeclOrigin(cxx_record); + if (!orig.Valid()) + return false; + + if (llvm::cast(orig.decl)->isDerivedFrom(candidate_cxx_record)) + return true; + + return false; +} + +void ClangASTSource::FindExternalVisibleMethods( + NameSearchContext &context, lldb::ModuleSP module_sp, + CompilerDeclContext &namespace_decl) { + assert(m_ast_context); + + SymbolContextList sc_list; + const ConstString name(context.m_decl_name.getAsString().c_str()); + if (!m_target) + return; + + if (context.m_found_type) + return; + + ModuleFunctionSearchOptions function_options; + function_options.include_inlines = false; + function_options.include_symbols = false; + m_target->GetImages().FindFunctions(name, lldb::eFunctionNameTypeMethod, + function_options, sc_list); + + auto num_matches = sc_list.GetSize(); + if (num_matches == 0) + return; + + for (const SymbolContext &sym_ctx : sc_list) { + assert(sym_ctx.function); + CompilerDeclContext decl_ctx = sym_ctx.function->GetDeclContext(); + if (!decl_ctx) + continue; + + assert(decl_ctx.IsClassMethod()); + + if (!CompilerDeclContextsMatch(decl_ctx, context.m_decl_context, + context.m_clang_ts)) + continue; + + clang::CXXMethodDecl *src_method = llvm::cast( + static_cast(decl_ctx.GetOpaqueDeclContext())); + Decl *copied_decl = CopyDecl(src_method); + + if (!copied_decl) + continue; + + CXXMethodDecl *copied_method_decl = dyn_cast(copied_decl); + + if (!copied_method_decl) + continue; + + context.AddNamedDecl(copied_method_decl); + } +} + void ClangASTSource::FindExternalVisibleDecls( NameSearchContext &context, lldb::ModuleSP module_sp, CompilerDeclContext &namespace_decl) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h index 6ae282196cfeb..b37cc0a60b3dc 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h @@ -87,6 +87,9 @@ class ClangASTSource : public ImporterBackedASTSource, bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, clang::DeclarationName Name) override; + bool FindExternalVisibleMethodsByName(const clang::DeclContext *DC, + clang::DeclarationName Name) override; + /// Enumerate all Decls in a given lexical context. /// /// \param[in] DC @@ -197,6 +200,7 @@ class ClangASTSource : public ImporterBackedASTSource, /// \param[in] context /// The NameSearchContext to use when filing results. virtual void FindExternalVisibleDecls(NameSearchContext &context); + virtual void FindExternalVisibleMethods(NameSearchContext &context); clang::Sema *getSema(); @@ -219,6 +223,12 @@ class ClangASTSource : public ImporterBackedASTSource, return m_original.FindExternalVisibleDeclsByName(DC, Name); } + bool + FindExternalVisibleMethodsByName(const clang::DeclContext *DC, + clang::DeclarationName Name) override { + return m_original.FindExternalVisibleMethodsByName(DC, Name); + } + void FindExternalLexicalDecls( const clang::DeclContext *DC, llvm::function_ref IsKindWeWant, @@ -288,6 +298,9 @@ class ClangASTSource : public ImporterBackedASTSource, void FindExternalVisibleDecls(NameSearchContext &context, lldb::ModuleSP module, CompilerDeclContext &namespace_decl); + void FindExternalVisibleMethods(NameSearchContext &context, + lldb::ModuleSP module, + CompilerDeclContext &namespace_decl); /// Find all Objective-C methods matching a given selector. /// @@ -364,6 +377,10 @@ class ClangASTSource : public ImporterBackedASTSource, NameSearchContext &context, DeclFromUser &origin_iface_decl); + bool CompilerDeclContextsMatch(CompilerDeclContext candidate_decl_ctx, + clang::DeclContext const *context, + TypeSystemClang &ts); + protected: bool FindObjCMethodDeclsWithOrigin( NameSearchContext &context, diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 77c761b9bf9ac..0f79fc7934d76 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1341,6 +1341,13 @@ void ClangExpressionDeclMap::LookupFunction( } } +void ClangExpressionDeclMap::FindExternalVisibleMethods( + NameSearchContext &context) { + assert(m_ast_context); + + ClangASTSource::FindExternalVisibleMethods(context); +} + void ClangExpressionDeclMap::FindExternalVisibleDecls( NameSearchContext &context, lldb::ModuleSP module_sp, const CompilerDeclContext &namespace_decl) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h index 9430ab5b0464b..61a6496d8c7b6 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h @@ -18,6 +18,7 @@ #include "ClangASTSource.h" #include "ClangExpressionVariable.h" +#include "Plugins/ExpressionParser/Clang/NameSearchContext.h" #include "lldb/Core/Value.h" #include "lldb/Expression/Materializer.h" #include "lldb/Symbol/SymbolContext.h" @@ -266,6 +267,7 @@ class ClangExpressionDeclMap : public ClangASTSource { /// \param[in] context /// The NameSearchContext that can construct Decls for this name. void FindExternalVisibleDecls(NameSearchContext &context) override; + void FindExternalVisibleMethods(NameSearchContext &context) override; /// Find all entities matching a given name in a given module/namespace, /// using a NameSearchContext to make Decls for them. diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h index c2ac0e2cd441d..ab429bb7c9307 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h @@ -37,6 +37,11 @@ class ClangExternalASTSourceCallbacks : public ImporterBackedASTSource { bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, clang::DeclarationName Name) override; + bool FindExternalVisibleMethodsByName(const clang::DeclContext *DC, + clang::DeclarationName Name) override { + return false; + } + void CompleteType(clang::TagDecl *tag_decl) override; void CompleteType(clang::ObjCInterfaceDecl *objc_decl) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 88d4b48d0499d..1f1f0ecbaf67e 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2246,8 +2246,34 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die, default_accessibility, layout_info); // Now parse any methods if there were any... - for (const DWARFDIE &die : member_function_dies) - dwarf->ResolveType(die); + for (const DWARFDIE &mem : member_function_dies) { + if (!TypeSystemClang::UseRedeclCompletion() || + type_is_objc_object_or_interface) { + dwarf->ResolveType(mem); + continue; + } + + ConstString mem_name(mem.GetName()); + ConstString die_name(die.GetName()); + const bool is_ctor = mem_name == die_name; + const bool is_virtual_method = + mem.GetAttributeValueAsUnsigned( + DW_AT_virtuality, DW_VIRTUALITY_none) > DW_VIRTUALITY_none; + const bool is_operator = mem_name.GetStringRef().starts_with("operator"); + const bool is_static_method = + mem.GetFirstChild().GetAttributeValueAsUnsigned(DW_AT_artificial, + 0) == 0; + + // FIXME: With RedeclCompletion, we currently don't have a good + // way to call `FindExternalVisibleMethods` from Clang + // for static functions, constructors or operators. + // So resolve them now. + // + // We want to resolve virtual methods now too because + // we set the method overrides below. + if (is_ctor || is_operator || is_virtual_method || is_static_method) + dwarf->ResolveType(mem); + } if (type_is_objc_object_or_interface) { ConstString class_name(clang_type.GetTypeName()); diff --git a/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py b/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py index 5ab5ae12a4e55..c4707c53ea2f0 100644 --- a/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py +++ b/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py @@ -34,11 +34,11 @@ def test(self): self.expect_expr("Foo{}.func()", result_type="int64_t", result_value="3") self.filecheck("target modules dump ast", __file__) - # CHECK: |-CXXMethodDecl {{.*}} func 'uint32_t () const &' - # CHECK-NEXT: | `-AsmLabelAttr {{.*}} - # CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'int64_t () const &&' - # CHECK-NEXT: | `-AsmLabelAttr {{.*}} - # CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'uint32_t () &' - # CHECK-NEXT: | `-AsmLabelAttr {{.*}} - # CHECK-NEXT: `-CXXMethodDecl {{.*}} func 'int64_t () &&' - # CHECK-NEXT: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'uint32_t () const &' + # CHECK-DAG: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'int64_t () const &&' + # CHECK-DAG: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'uint32_t () &' + # CHECK-DAG: `-AsmLabelAttr {{.*}} + # CHECK-DAG: CXXMethodDecl {{.*}} func 'int64_t () &&' + # CHECK-DAG: `-AsmLabelAttr {{.*}} From 4397f63f7675d09af6a2c04d5abce4dc2cbe9083 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 18 Apr 2024 17:35:44 +0100 Subject: [PATCH 2/2] [lldb][test][redecl-completion] XFAIL currently unsupported tests w/ lazy method loading --- .../completion/TestExprCompletion.py | 1 + .../context-object/TestContextObject.py | 28 ++++++++++++++----- .../empty-module/TestEmptyStdModule.py | 1 + .../expression/pr35310/TestExprsBug35310.py | 1 + .../class_members/TestSBTypeClassMembers.py | 1 + 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lldb/test/API/commands/expression/completion/TestExprCompletion.py b/lldb/test/API/commands/expression/completion/TestExprCompletion.py index 27a9e1d8a00f7..22327f5e006c7 100644 --- a/lldb/test/API/commands/expression/completion/TestExprCompletion.py +++ b/lldb/test/API/commands/expression/completion/TestExprCompletion.py @@ -13,6 +13,7 @@ class CommandLineExprCompletionTestCase(TestBase): NO_DEBUG_INFO_TESTCASE = True + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test_expr_completion(self): self.build() self.main_source = "main.cpp" diff --git a/lldb/test/API/commands/expression/context-object/TestContextObject.py b/lldb/test/API/commands/expression/context-object/TestContextObject.py index 1ed629a42c1ee..a9606f7b84097 100644 --- a/lldb/test/API/commands/expression/context-object/TestContextObject.py +++ b/lldb/test/API/commands/expression/context-object/TestContextObject.py @@ -5,9 +5,29 @@ import lldb import lldbsuite.test.lldbutil as lldbutil from lldbsuite.test.lldbtest import * - +from lldbsuite.test.decorators import * class ContextObjectTestCase(TestBase): + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) + def test_context_object_eval_function(self): + """Tests expression evaluation of functions in context of an object.""" + self.build() + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "// Break here", self.main_source_spec + ) + frame = thread.GetFrameAtIndex(0) + for obj in "cpp_struct", "cpp_struct_ref": + obj_val = frame.FindVariable(obj) + self.assertTrue(obj_val.IsValid()) + + # Test functions evaluation + value = obj_val.EvaluateExpression("function()") + self.assertTrue(value.IsValid()) + self.assertSuccess(value.GetError()) + self.assertEqual(value.GetValueAsSigned(), 2222) + + def test_context_object(self): """Tests expression evaluation in context of an object.""" self.build() @@ -35,12 +55,6 @@ def test_context_object(self): self.assertSuccess(value.GetError()) self.assertEqual(value.GetValueAsSigned(), 1111) - # Test functions evaluation - value = obj_val.EvaluateExpression("function()") - self.assertTrue(value.IsValid()) - self.assertSuccess(value.GetError()) - self.assertEqual(value.GetValueAsSigned(), 2222) - # Test that we retrieve the right global value = obj_val.EvaluateExpression("global.field") self.assertTrue(value.IsValid()) diff --git a/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py b/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py index 913d964578918..22f5aa2825f7d 100644 --- a/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py +++ b/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py @@ -15,6 +15,7 @@ class ImportStdModule(TestBase): @add_test_categories(["libc++"]) @skipIfRemote @skipIf(compiler=no_match("clang")) + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test(self): self.build() diff --git a/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py b/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py index ea609367bc5b0..ee7429cb6be1f 100644 --- a/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py +++ b/lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py @@ -12,6 +12,7 @@ def setUp(self): self.main_source = "main.cpp" self.main_source_spec = lldb.SBFileSpec(self.main_source) + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) def test_issue35310(self): """Test invoking functions with non-standard linkage names. diff --git a/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py b/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py index 5ad2a073be585..445008968a140 100644 --- a/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py +++ b/lldb/test/API/python_api/class_members/TestSBTypeClassMembers.py @@ -19,6 +19,7 @@ def setUp(self): self.source = "main.mm" self.line = line_number(self.source, "// set breakpoint here") + @expectedFailureAll(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true')) @skipUnlessDarwin def test(self): """Test SBType APIs to fetch member function types."""