[lldb][DWARFASTParserClang] Make C++ method parsing aware of explicit object parameters #124096
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… object parameters LLDB deduces the CV-qualifiers and storage class of a C++ method from the object parameter. Currently it assumes that parameter is implicit (and is a pointer type with the name "this"). This isn't true anymore in C++23 with explicit object parameters. To support those we can simply check the `DW_AT_object_pointer` of the subprogram DIE (works for both declarations and definitions) when searching for the object parameter. We can also remove the check for `eEncodingIsPointerUID`, because in C++ an artificial parameter called `this` is only ever the implicit object parameter (at least for all the major compilers).
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesLLDB deduces the CV-qualifiers and storage class of a C++ method from the object parameter. Currently it assumes that parameter is implicit (and is a pointer type with the name "this"). This isn't true anymore in C++23 with explicit object parameters. To support those we can simply check the We can also remove the check for Full diff: https://github.com/llvm/llvm-project/pull/124096.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f54b7fc9cdad24..682ee6d287bf5c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -173,7 +173,9 @@ GetCXXObjectParameter(const DWARFDIE &subprogram,
if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
return {};
- // FIXME: if subprogram has a explicit DW_AT_object_pointer, use it.
+ if (DWARFDIE object_parameter =
+ subprogram.GetAttributeValueAsReferenceDIE(DW_AT_object_pointer))
+ return object_parameter;
// 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
@@ -215,11 +217,6 @@ static unsigned GetCXXMethodCVQuals(const DWARFDIE &subprogram,
return 0;
uint32_t encoding_mask = this_type->GetEncodingMask();
-
- // FIXME: explicit object parameters need not to be pointers
- if (!(encoding_mask & (1u << Type::eEncodingIsPointerUID)))
- return 0;
-
unsigned cv_quals = 0;
if (encoding_mask & (1u << Type::eEncodingIsConstUID))
cv_quals |= clang::Qualifiers::Const;
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
index b31f56aa372d58..9c0300be08a78a 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -902,3 +902,181 @@ TEST_F(DWARFASTParserClangTests, TestParseDWARFAttributes_ObjectPointer) {
EXPECT_TRUE(attrs.object_pointer.IsValid());
EXPECT_EQ(attrs.object_pointer, param_die);
}
+
+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
+ // rvalue reference instead).
+
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_str:
+ - Context
+ - func
+ - mySelf
+ 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_external
+ Form: DW_FORM_flag_present
+ - Code: 0x4
+ Tag: DW_TAG_formal_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Code: 0x5
+ Tag: DW_TAG_rvalue_reference_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Code: 0x6
+ Tag: DW_TAG_const_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Code: 0x7
+ Tag: DW_TAG_volatile_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ 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
+
+# DW_TAG_formal_parameter
+# DW_AT_name [DW_FORM_strp] ("mySelf")
+# DW_AT_type [DW_FORM_ref4] (const volatile Context &&)
+ - AbbrCode: 0x4
+ Values:
+ - Value: 0xd
+ - Value: 0x28
+
+ - AbbrCode: 0x0
+ - AbbrCode: 0x0
+
+# DW_TAG_rvalue_reference_type
+# DW_AT_type [DW_FORM_ref4] ("const volatile Context")
+
+ - AbbrCode: 0x5
+ Values:
+ - Value: 0x2d
+
+# DW_TAG_const_type
+# DW_AT_type [DW_FORM_ref4] ("volatile Context")
+
+ - AbbrCode: 0x6
+ Values:
+ - Value: 0x32
+
+# DW_TAG_volatile_type
+# DW_AT_type [DW_FORM_ref4] ("Context")
+
+ - AbbrCode: 0x7
+ Values:
+ - Value: 0xf
+
+ - 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 ts_or_err =
+ cu_die.GetDWARF()->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
+ auto *parser =
+ static_cast<DWARFASTParserClang *>((*ts_or_err)->GetDWARFParser());
+
+ auto context_die = cu_die.GetFirstChild();
+ ASSERT_TRUE(context_die.IsValid());
+ ASSERT_EQ(context_die.Tag(), DW_TAG_structure_type);
+
+ SymbolContext sc;
+ bool new_type;
+ auto context_type_sp = parser->ParseTypeFromDWARF(sc, context_die, &new_type);
+ ASSERT_NE(context_type_sp, nullptr);
+
+ ASSERT_TRUE(
+ parser->CompleteTypeFromDWARF(context_die, context_type_sp.get(),
+ context_type_sp->GetForwardCompilerType()));
+
+ auto *record_decl = llvm::dyn_cast_or_null<clang::CXXRecordDecl>(
+ ClangUtil::GetAsTagDecl(context_type_sp->GetForwardCompilerType()));
+ ASSERT_NE(record_decl, nullptr);
+
+ auto method_it = record_decl->method_begin();
+ ASSERT_NE(method_it, record_decl->method_end());
+
+ // Check that we didn't parse the function as static.
+ EXPECT_FALSE(method_it->isStatic());
+
+ // Check that method qualifiers were correctly set.
+ EXPECT_EQ(method_it->getMethodQualifiers(),
+ clang::Qualifiers::fromCVRMask(clang::Qualifiers::Const |
+ clang::Qualifiers::Volatile));
+}
|
labath
approved these changes
Jan 23, 2025
Yea there's still a couple of issues with it. Particularly, expression evaluation in such methods currently doesn't work. But I suspect that isn't DWARF parser related, but due to assumptions in other parts of the expression evaluator. One particularly nasty looking blocker is that Still need to investigate exactly what's going on though |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/14712 Here is the relevant piece of the build log for the reference |
Michael137
added a commit
that referenced
this pull request
Jan 23, 2025
…explicit object parameters" (#124100) Reverts #124096 Broke linux CI: ``` Note: This is test shard 7 of 42. [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from DWARFASTParserClangTests [ RUN ] DWARFASTParserClangTests.TestParseSubroutine_ExplicitObjectParameter Expected<T> must be checked before access or destruction. Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed). Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 SymbolFileDWARFTests 0x0000560271ee5ba7 1 SymbolFileDWARFTests 0x0000560271ee3a2c 2 SymbolFileDWARFTests 0x0000560271ee63ea 3 libc.so.6 0x00007f3e54e5b050 4 libc.so.6 0x00007f3e54ea9e2c 5 libc.so.6 0x00007f3e54e5afb2 gsignal + 18 6 libc.so.6 0x00007f3e54e45472 abort + 211 7 SymbolFileDWARFTests 0x0000560271e79d51 8 SymbolFileDWARFTests 0x0000560271e724f7 9 SymbolFileDWARFTests 0x0000560271f39e2c 10 SymbolFileDWARFTests 0x0000560271f3b368 11 SymbolFileDWARFTests 0x0000560271f3c053 12 SymbolFileDWARFTests 0x0000560271f4cf67 13 SymbolFileDWARFTests 0x0000560271f4c18a 14 SymbolFileDWARFTests 0x0000560271f2561c 15 libc.so.6 0x00007f3e54e4624a 16 libc.so.6 0x00007f3e54e46305 __libc_start_main + 133 17 SymbolFileDWARFTests 0x0000560271e65161 ```
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/3894 Here is the relevant piece of the build log for the reference |
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 23, 2025
…g aware of explicit object parameters" (#124100) Reverts llvm/llvm-project#124096 Broke linux CI: ``` Note: This is test shard 7 of 42. [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from DWARFASTParserClangTests [ RUN ] DWARFASTParserClangTests.TestParseSubroutine_ExplicitObjectParameter Expected<T> must be checked before access or destruction. Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed). Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 SymbolFileDWARFTests 0x0000560271ee5ba7 1 SymbolFileDWARFTests 0x0000560271ee3a2c 2 SymbolFileDWARFTests 0x0000560271ee63ea 3 libc.so.6 0x00007f3e54e5b050 4 libc.so.6 0x00007f3e54ea9e2c 5 libc.so.6 0x00007f3e54e5afb2 gsignal + 18 6 libc.so.6 0x00007f3e54e45472 abort + 211 7 SymbolFileDWARFTests 0x0000560271e79d51 8 SymbolFileDWARFTests 0x0000560271e724f7 9 SymbolFileDWARFTests 0x0000560271f39e2c 10 SymbolFileDWARFTests 0x0000560271f3b368 11 SymbolFileDWARFTests 0x0000560271f3c053 12 SymbolFileDWARFTests 0x0000560271f4cf67 13 SymbolFileDWARFTests 0x0000560271f4c18a 14 SymbolFileDWARFTests 0x0000560271f2561c 15 libc.so.6 0x00007f3e54e4624a 16 libc.so.6 0x00007f3e54e46305 __libc_start_main + 133 17 SymbolFileDWARFTests 0x0000560271e65161 ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LLDB deduces the CV-qualifiers and storage class of a C++ method from the object parameter. Currently it assumes that parameter is implicit (and is a pointer type with the name "this"). This isn't true anymore in C++23 with explicit object parameters. To support those we can simply check the
DW_AT_object_pointerof the subprogram DIE (works for both declarations and definitions) when searching for the object parameter.We can also remove the check for
eEncodingIsPointerUID, because in C++ an artificial parameter calledthisis only ever the implicit object parameter (at least for all the major compilers).