[Lldb-commits] [lldb] [lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes (PR #144880)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Thu Jun 19 04:45:24 PDT 2025
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/144880
>From bda770fa0bd47fc0ac64189f5e25a9b820051d8c Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 19 Jun 2025 12:11:32 +0100
Subject: [PATCH 1/4] [lldb][DWARFASTParserClang] GetCXXObjectParameter to take
DeclContext DIE parameter
I'm trying to call `GetCXXObjectParameter` from unit-tests in a
follow-up patch and taking a `DWARFDIE` instead of `clang::DeclContext`
makes that much simpler. These should be equivalent, since all we're
trying to check is that the parent context is a record type.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 620501b304e63..7fc1d70898d1d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -163,14 +163,14 @@ static bool TagIsRecordType(dw_tag_t tag) {
/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
/// C++ declaration context for class methods, assume no object
/// parameter exists for the given \c subprogram.
-static DWARFDIE
-GetCXXObjectParameter(const DWARFDIE &subprogram,
- const clang::DeclContext &containing_decl_ctx) {
+static DWARFDIE GetCXXObjectParameter(const DWARFDIE &subprogram,
+ const DWARFDIE &decl_ctx_die) {
+ assert(subprogram);
assert(subprogram.Tag() == DW_TAG_subprogram ||
subprogram.Tag() == DW_TAG_inlined_subroutine ||
subprogram.Tag() == DW_TAG_subroutine_type);
- if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+ if (!decl_ctx_die.IsStructUnionOrClass())
return {};
if (DWARFDIE object_parameter =
@@ -1304,8 +1304,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
clang::CallingConv calling_convention =
ConvertDWARFCallingConventionToClang(attrs);
- const DWARFDIE object_parameter =
- GetCXXObjectParameter(die, *containing_decl_ctx);
+ const DWARFDIE object_parameter = GetCXXObjectParameter(die, decl_ctx_die);
// clang_type will get the function prototype clang type after this
// call
@@ -2411,12 +2410,13 @@ DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
sstr << decl_ctx.GetQualifiedName();
+ DWARFDIE decl_ctx_die;
clang::DeclContext *containing_decl_ctx =
- GetClangDeclContextContainingDIE(die, nullptr);
+ GetClangDeclContextContainingDIE(die, &decl_ctx_die);
assert(containing_decl_ctx);
- const unsigned cv_quals = GetCXXMethodCVQuals(
- die, GetCXXObjectParameter(die, *containing_decl_ctx));
+ const unsigned cv_quals =
+ GetCXXMethodCVQuals(die, GetCXXObjectParameter(die, decl_ctx_die));
ParseChildParameters(containing_decl_ctx, die, is_variadic,
has_template_params, param_types, param_names);
>From de7b06c9afadcf950b96e8a45d7df19dd6a590f9 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 19 Jun 2025 12:22:30 +0100
Subject: [PATCH 2/4] [lldb][DWARFASTParserClang] Make GetCXXObjectParameter
public and call it from unit-tests
My goal is to remove the `object_pointer` member on
`ParsedDWARFTypeAttributes` since it's duplicating information that we
retrieve with `GetCXXObjectParameter` anyway. To continue having
coverage for the `DW_AT_object_pointer` code-paths, instead of checking the
`attrs.object_pointer` I'm now calling `GetCXXObjectParameter` directly.
We could find some very roundabout way to go via the Clang AST to check
that the object parameter was parsed correctly, but that quickly became
quite painful.
Depends on https://github.com/llvm/llvm-project/pull/144876
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 ++-
.../SymbolFile/DWARF/DWARFASTParserClang.h | 4 ++
.../DWARF/DWARFASTParserClangTests.cpp | 38 +++++++++++++------
3 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 7fc1d70898d1d..4f79c8aa3f811 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -163,8 +163,9 @@ static bool TagIsRecordType(dw_tag_t tag) {
/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
/// C++ declaration context for class methods, assume no object
/// parameter exists for the given \c subprogram.
-static DWARFDIE GetCXXObjectParameter(const DWARFDIE &subprogram,
- const DWARFDIE &decl_ctx_die) {
+DWARFDIE
+DWARFASTParserClang::GetCXXObjectParameter(const DWARFDIE &subprogram,
+ const DWARFDIE &decl_ctx_die) {
assert(subprogram);
assert(subprogram.Tag() == DW_TAG_subprogram ||
subprogram.Tag() == DW_TAG_inlined_subroutine ||
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 3994726aa6b3e..111604ce4068a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -112,6 +112,10 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
void MapDeclDIEToDefDIE(const lldb_private::plugin::dwarf::DWARFDIE &decl_die,
const lldb_private::plugin::dwarf::DWARFDIE &def_die);
+ lldb_private::plugin::dwarf::DWARFDIE GetCXXObjectParameter(
+ const lldb_private::plugin::dwarf::DWARFDIE &subprogram,
+ const lldb_private::plugin::dwarf::DWARFDIE &decl_ctx_die);
+
protected:
/// Protected typedefs and members.
/// @{
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
index 6c77736113da3..2d4b79fed4a55 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -889,18 +889,32 @@ TEST_F(DWARFASTParserClangTests, TestParseDWARFAttributes_ObjectPointer) {
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));
-
- auto param_die = subprogram_definition.GetFirstChild();
- ASSERT_TRUE(param_die.IsValid());
-
- ParsedDWARFTypeAttributes attrs(subprogram_definition);
- EXPECT_TRUE(attrs.object_pointer.IsValid());
- EXPECT_EQ(attrs.object_pointer, param_die);
+ {
+ auto decl_die = context_die.GetFirstChild();
+ ASSERT_TRUE(decl_die.IsValid());
+ ASSERT_EQ(decl_die.Tag(), DW_TAG_subprogram);
+ ASSERT_TRUE(decl_die.GetAttributeValueAsOptionalUnsigned(DW_AT_external));
+
+ auto param_die = decl_die.GetFirstChild();
+ ASSERT_TRUE(param_die.IsValid());
+
+ EXPECT_EQ(param_die,
+ ast_parser.GetCXXObjectParameter(decl_die, context_die));
+ }
+
+ {
+ 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));
+
+ auto param_die = subprogram_definition.GetFirstChild();
+ ASSERT_TRUE(param_die.IsValid());
+
+ EXPECT_EQ(param_die, ast_parser.GetCXXObjectParameter(subprogram_definition,
+ context_die));
+ }
}
TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ExplicitObjectParameter) {
>From ac92192b0950a8df592c8bc8df414a6db1c743fd Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 19 Jun 2025 12:29:25 +0100
Subject: [PATCH 3/4] [lldb][DWARF] Remove object_pointer from
ParsedDWARFAttributes
We can just always use `GetCXXObjectParameter` instead. We've only used
this attribute to set the object parameter name on ClangASTMetadata,
which doesn't seem like good enough justification to keep it around.
Depends on https://github.com/llvm/llvm-project/pull/144879
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 26 +++++--------------
.../SymbolFile/DWARF/DWARFASTParserClang.h | 7 ++---
2 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4f79c8aa3f811..d1aef8becfd95 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,10 +1315,8 @@ 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,
+ 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 d6e02aefd5864f94111f5365c086aa56a5503553 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 19 Jun 2025 12:45:11 +0100
Subject: [PATCH 4/4] fixup! fix is_static
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index d1aef8becfd95..06c78ed524eb4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1192,7 +1192,7 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod(
attrs.name.GetCString());
// In DWARF, a C++ method is static if it has no object parameter child.
- const bool is_static = object_parameter.IsValid();
+ 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
More information about the lldb-commits
mailing list