From 5cf09dd22008514f36f12e670f29111c67d612e3 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 230c83943c222..1b6904cc82b45 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 c79128bc8f39e..80e633979ca14 100644 --- a/clang/lib/Sema/SemaExprMember.cpp +++ b/clang/lib/Sema/SemaExprMember.cpp @@ -715,6 +715,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 142ddcc2c21a9..f6968bb89e978 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.starts_with("_$"); } +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 4e95ec4c97996..10d640c154c29 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -1337,6 +1337,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 d8d519693f102..9402542f6b563 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 3dfbf6a04fcd0..2d5e045013300 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2346,8 +2346,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 41c4fca9614b5680f2ff9a24225dcfde5fe7e9e2 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 022b9436ee8ea..73226f6a53556 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."""