https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/145126
>From 2572a2f224d8c3372fadc0edfa2af5f618765669 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 20 Jun 2025 18:09:54 +0100 Subject: [PATCH 1/3] Reland "[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes (#145065)" This reverts commit 877511920dcf36463e06746d626e8876583a6abd. This fixes the `TestObjCInBlockVars.py` LLDB API test. The issue was that `GetCXXObjectParameter` wouldn't deduce the object parameter of Objective-C method definitions correctly. In DWARF those don't have a `DW_AT_specification` (so no link back to a DeclContext that is a class type). The fix is to only check the validity of the DeclContext DIE *if* no `DW_AT_object_pointer` exists on the DIE. If `DW_AT_object_pointer` does exist, we should just always use that as the object_parameter. --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 28 ++++++------------- .../SymbolFile/DWARF/DWARFASTParserClang.h | 7 +++-- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 4f79c8aa3f811..3bec89cdf7469 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -445,15 +445,6 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) { name.SetCString(form_value.AsCString()); break; - case DW_AT_object_pointer: - // GetAttributes follows DW_AT_specification. - // DW_TAG_subprogram definitions and declarations may both - // have a DW_AT_object_pointer. Don't overwrite the one - // we parsed for the definition with the one from the declaration. - if (!object_pointer.IsValid()) - object_pointer = form_value.Reference(); - break; - case DW_AT_signature: signature = form_value; break; @@ -1116,7 +1107,7 @@ bool DWARFASTParserClang::ParseObjCMethod( std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( const DWARFDIE &die, CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs, const DWARFDIE &decl_ctx_die, - bool is_static, bool &ignore_containing_context) { + const DWARFDIE &object_parameter, bool &ignore_containing_context) { Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); SymbolFileDWARF *dwarf = die.GetDWARF(); assert(dwarf); @@ -1200,6 +1191,9 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( TypeSystemClang::GetDeclContextForType(class_opaque_type), die, attrs.name.GetCString()); + // In DWARF, a C++ method is static if it has no object parameter child. + const bool is_static = !object_parameter.IsValid(); + // We have a C++ member function with no children (this pointer!) and clang // will get mad if we try and make a function that isn't well formed in the // DWARF, so we will just skip it... @@ -1225,9 +1219,7 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( ClangASTMetadata metadata; metadata.SetUserID(die.GetID()); - char const *object_pointer_name = - attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr; - if (object_pointer_name) { + if (char const *object_pointer_name = object_parameter.GetName()) { metadata.SetObjectPtrName(object_pointer_name); LLDB_LOGF(log, "Setting object pointer name: %s on method object %p.\n", object_pointer_name, static_cast<void *>(cxx_method_decl)); @@ -1323,11 +1315,9 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, type_handled = ParseObjCMethod(*objc_method, die, clang_type, attrs, is_variadic); } else if (is_cxx_method) { - // In DWARF, a C++ method is static if it has no object parameter child. - const bool is_static = !object_parameter.IsValid(); auto [handled, type_sp] = - ParseCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static, - ignore_containing_context); + ParseCXXMethod(die, clang_type, attrs, decl_ctx_die, + object_parameter, ignore_containing_context); if (type_sp) return type_sp; @@ -1422,9 +1412,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, ClangASTMetadata metadata; metadata.SetUserID(die.GetID()); - char const *object_pointer_name = - attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr; - if (object_pointer_name) { + if (char const *object_pointer_name = object_parameter.GetName()) { metadata.SetObjectPtrName(object_pointer_name); LLDB_LOGF(log, "Setting object pointer name: %s on function " diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index 111604ce4068a..a90f55bcff948 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -470,7 +470,8 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { /// \param[in] decl_ctx_die The DIE representing the DeclContext of the C++ /// method being parsed. /// - /// \param[in] is_static Is true iff we're parsing a static method. + /// \param[in] object_parameter The DIE of this subprogram's object parameter. + /// May be an invalid DIE for C++ static methods. /// /// \param[out] ignore_containing_context Will get set to true if the caller /// should treat this C++ method as-if it was not a C++ method. @@ -485,7 +486,8 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { lldb_private::CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs, const lldb_private::plugin::dwarf::DWARFDIE &decl_ctx_die, - bool is_static, bool &ignore_containing_context); + const lldb_private::plugin::dwarf::DWARFDIE &object_parameter, + bool &ignore_containing_context); lldb::TypeSP ParseArrayType(const lldb_private::plugin::dwarf::DWARFDIE &die, const ParsedDWARFTypeAttributes &attrs); @@ -555,7 +557,6 @@ struct ParsedDWARFTypeAttributes { const char *mangled_name = nullptr; lldb_private::ConstString name; lldb_private::Declaration decl; - lldb_private::plugin::dwarf::DWARFDIE object_pointer; lldb_private::plugin::dwarf::DWARFFormValue abstract_origin; lldb_private::plugin::dwarf::DWARFFormValue containing_type; lldb_private::plugin::dwarf::DWARFFormValue signature; >From cd5412d1bc9d9ef11d4a81268b275052bcd4a0ec Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 20 Jun 2025 18:09:54 +0100 Subject: [PATCH 2/3] fixup! fix objective-c case; add unit-test; adjust test docs --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 7 +- .../DWARF/DWARFASTParserClangTests.cpp | 163 +++++++++++++++++- 2 files changed, 165 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index e4ecb82c5f1e1..d3912ad55a235 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -167,9 +167,6 @@ DWARFASTParserClang::GetObjectParameter(const DWARFDIE &subprogram, subprogram.Tag() == DW_TAG_inlined_subroutine || subprogram.Tag() == DW_TAG_subroutine_type); - if (!decl_ctx_die.IsStructUnionOrClass()) - return {}; - if (DWARFDIE object_parameter = subprogram.GetAttributeValueAsReferenceDIE(DW_AT_object_pointer)) return object_parameter; @@ -177,6 +174,10 @@ DWARFASTParserClang::GetObjectParameter(const DWARFDIE &subprogram, // If no DW_AT_object_pointer was specified, assume the implicit object // parameter is the first parameter to the function, is called "this" and is // artificial (which is what most compilers would generate). + + if (!decl_ctx_die.IsStructUnionOrClass()) + return {}; + auto children = subprogram.children(); auto it = llvm::find_if(children, [](const DWARFDIE &child) { return child.Tag() == DW_TAG_formal_parameter; diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp index f18e938dbc4c9..0c647ce5adc52 100644 --- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp @@ -742,8 +742,8 @@ TEST_F(DWARFASTParserClangTests, TestUniqueDWARFASTTypeMap_CppInsertMapFind) { ASSERT_EQ(type_sp, reparsed_type_sp); } -TEST_F(DWARFASTParserClangTests, TestParseDWARFAttributes_ObjectPointer) { - // This tests the behaviour of ParsedDWARFTypeAttributes +TEST_F(DWARFASTParserClangTests, TestObjectPointer) { + // This tests the behaviour of DWARFASTParserClang // for DW_TAG_subprogram definitions which have a DW_AT_object_pointer // *and* a DW_AT_specification that also has a DW_AT_object_pointer. // We don't want the declaration DW_AT_object_pointer to overwrite the @@ -916,6 +916,165 @@ TEST_F(DWARFASTParserClangTests, TestParseDWARFAttributes_ObjectPointer) { } } +TEST_F(DWARFASTParserClangTests, + TestObjectPointer_NoSpecificationOnDefinition) { + // This tests the behaviour of DWARFASTParserClang + // for DW_TAG_subprogram definitions which have a DW_AT_object_pointer + // but no DW_AT_specification that would link back to its declaration. + // This is how Objective-C class method definitions are emitted. + + const char *yamldata = R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_EXEC + Machine: EM_AARCH64 +DWARF: + debug_str: + - Context + - func + - this + debug_abbrev: + - ID: 0 + Table: + - Code: 0x1 + Tag: DW_TAG_compile_unit + Children: DW_CHILDREN_yes + Attributes: + - Attribute: DW_AT_language + Form: DW_FORM_data2 + - Code: 0x2 + Tag: DW_TAG_structure_type + Children: DW_CHILDREN_yes + Attributes: + - Attribute: DW_AT_name + Form: DW_FORM_strp + - Code: 0x3 + Tag: DW_TAG_subprogram + Children: DW_CHILDREN_yes + Attributes: + - Attribute: DW_AT_name + Form: DW_FORM_strp + - Attribute: DW_AT_declaration + Form: DW_FORM_flag_present + - Attribute: DW_AT_object_pointer + Form: DW_FORM_ref4 + - Attribute: DW_AT_artificial + Form: DW_FORM_flag_present + - Attribute: DW_AT_external + Form: DW_FORM_flag_present + - Code: 0x4 + Tag: DW_TAG_formal_parameter + Children: DW_CHILDREN_no + Attributes: + - Attribute: DW_AT_artificial + Form: DW_FORM_flag_present + - Code: 0x5 + Tag: DW_TAG_subprogram + Children: DW_CHILDREN_yes + Attributes: + - Attribute: DW_AT_object_pointer + Form: DW_FORM_ref4 + - Code: 0x6 + Tag: DW_TAG_formal_parameter + Children: DW_CHILDREN_no + Attributes: + - Attribute: DW_AT_name + Form: DW_FORM_strp + - Attribute: DW_AT_artificial + Form: DW_FORM_flag_present + debug_info: + - Version: 5 + UnitType: DW_UT_compile + AddrSize: 8 + Entries: + +# DW_TAG_compile_unit +# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus) + + - AbbrCode: 0x1 + Values: + - Value: 0x04 + +# DW_TAG_structure_type +# DW_AT_name [DW_FORM_strp] ("Context") + + - AbbrCode: 0x2 + Values: + - Value: 0x0 + +# DW_TAG_subprogram +# DW_AT_name [DW_FORM_strp] ("func") +# DW_AT_object_pointer [DW_FORM_ref4] + - AbbrCode: 0x3 + Values: + - Value: 0x8 + - Value: 0x1 + - Value: 0x1d + - Value: 0x1 + - Value: 0x1 + +# DW_TAG_formal_parameter +# DW_AT_artificial + - AbbrCode: 0x4 + Values: + - Value: 0x1 + + - AbbrCode: 0x0 + - AbbrCode: 0x0 + +# DW_TAG_subprogram +# DW_AT_object_pointer [DW_FORM_ref4] ("this") +# DW_AT_specification [DW_FORM_ref4] ("func") + - AbbrCode: 0x5 + Values: + - Value: 0x29 + +# DW_TAG_formal_parameter +# DW_AT_name [DW_FORM_strp] ("this") +# DW_AT_artificial + - AbbrCode: 0x6 + Values: + - Value: 0xd + - Value: 0x1 + + - AbbrCode: 0x0 + - AbbrCode: 0x0 +... +)"; + YAMLModuleTester t(yamldata); + + DWARFUnit *unit = t.GetDwarfUnit(); + ASSERT_NE(unit, nullptr); + const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE(); + ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit); + ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus); + DWARFDIE cu_die(unit, cu_entry); + + auto holder = std::make_unique<clang_utils::TypeSystemClangHolder>("ast"); + auto &ast_ctx = *holder->GetAST(); + DWARFASTParserClangStub ast_parser(ast_ctx); + + auto context_die = cu_die.GetFirstChild(); + ASSERT_TRUE(context_die.IsValid()); + ASSERT_EQ(context_die.Tag(), DW_TAG_structure_type); + + auto subprogram_definition = context_die.GetSibling(); + ASSERT_TRUE(subprogram_definition.IsValid()); + ASSERT_EQ(subprogram_definition.Tag(), DW_TAG_subprogram); + ASSERT_FALSE(subprogram_definition.GetAttributeValueAsOptionalUnsigned( + DW_AT_external)); + ASSERT_FALSE( + subprogram_definition.GetAttributeValueAsReferenceDIE(DW_AT_specification) + .IsValid()); + + auto param_die = subprogram_definition.GetFirstChild(); + ASSERT_TRUE(param_die.IsValid()); + EXPECT_EQ(param_die, + ast_parser.GetObjectParameter(subprogram_definition, {})); +} + TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ExplicitObjectParameter) { // Tests parsing of a C++ non-static member function with an explicit object // parameter that isn't called "this" and is not a pointer (but a CV-qualified >From 76167641243a374a3591f0a5c914b1968bdab26c Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Sun, 22 Jun 2025 23:30:15 +0100 Subject: [PATCH 3/3] fixup! fix yaml offsets --- lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp index 0c647ce5adc52..fa05cd174fc7b 100644 --- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp @@ -1026,10 +1026,9 @@ TEST_F(DWARFASTParserClangTests, # DW_TAG_subprogram # DW_AT_object_pointer [DW_FORM_ref4] ("this") -# DW_AT_specification [DW_FORM_ref4] ("func") - AbbrCode: 0x5 Values: - - Value: 0x29 + - Value: 0x25 # DW_TAG_formal_parameter # DW_AT_name [DW_FORM_strp] ("this") _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits