[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes #144880

Michael137 · 2025-06-19T11:31:39Z

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 #144879

… 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.

…l 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 llvm#144876

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 llvm#144879

llvmbot · 2025-06-19T11:32:09Z

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

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 #144879

Full diff: https://github.com/llvm/llvm-project/pull/144880.diff

3 Files Affected:

(modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+17-28)
(modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (+8-3)
(modified) lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp (+26-12)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 620501b304e63..d1aef8becfd95 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -163,14 +163,15 @@ 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) {
+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 ||
          subprogram.Tag() == DW_TAG_subroutine_type);
 
-  if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+  if (!decl_ctx_die.IsStructUnionOrClass())
     return {};
 
   if (DWARFDIE object_parameter =
@@ -444,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;
@@ -1115,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);
@@ -1199,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...
@@ -1224,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));
@@ -1304,8 +1297,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
@@ -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 "
@@ -2411,12 +2399,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);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 3994726aa6b3e..a90f55bcff948 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.
   /// @{
@@ -466,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.
@@ -481,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);
@@ -551,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;
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) {

github-actions · 2025-06-19T11:33:55Z

✅ With the latest revision this PR passed the C/C++ code formatter.

JDevlieghere · 2025-06-20T15:43:55Z

@Michael137 Looks like this causes TestObjCIvarsInBlocks.py to fail on AS: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/28039/

#144880)" This reverts commit b017b4c.

…s" (#145065) Reverts #144880 Caused `TestObjCIvarsInBlocks.py` to fail on macOS CI.

Michael137 · 2025-06-20T16:21:20Z

@Michael137 Looks like this causes TestObjCIvarsInBlocks.py to fail on AS: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/28039/

Thanks for pinging. Reverted for now.

…RFAttributes" (#145065) Reverts llvm/llvm-project#144880 Caused `TestObjCIvarsInBlocks.py` to fail on macOS CI.

Michael137 added 3 commits June 19, 2025 12:14

Michael137 requested a review from labath June 19, 2025 11:31

Michael137 requested a review from JDevlieghere as a code owner June 19, 2025 11:31

llvmbot added the lldb label Jun 19, 2025

Michael137 added 4 commits June 19, 2025 12:45

fixup! fix is_static

d6e02ae

fixup! clang-format

d232941

Merge branch 'main' into lldb/cxx-object-parameter-changes-3

0f66fc9

Merge branch 'main' into lldb/cxx-object-parameter-changes-3

d887e82

labath approved these changes Jun 20, 2025

View reviewed changes

Michael137 merged commit b017b4c into llvm:main Jun 20, 2025
7 checks passed

Michael137 deleted the lldb/cxx-object-parameter-changes-3 branch June 20, 2025 15:01

Michael137 added a commit that referenced this pull request Jun 20, 2025

Revert "[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes (

34db3b6

#144880)" This reverts commit b017b4c.

Michael137 mentioned this pull request Jun 20, 2025

Revert "[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes" #145065

Merged

Michael137 added a commit that referenced this pull request Jun 20, 2025

Revert "[lldb][DWARF] Remove object_pointer from ParsedDWARFAttribute…

8775119

…s" (#145065) Reverts #144880 Caused `TestObjCIvarsInBlocks.py` to fail on macOS CI.

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 20, 2025

Automerge: Revert "[lldb][DWARF] Remove object_pointer from ParsedDWA…

8abf6f5

…RFAttributes" (#145065) Reverts llvm/llvm-project#144880 Caused `TestObjCIvarsInBlocks.py` to fail on macOS CI.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes #144880

[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes #144880

Uh oh!

Michael137 commented Jun 19, 2025

Uh oh!

llvmbot commented Jun 19, 2025

Uh oh!

github-actions bot commented Jun 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

JDevlieghere commented Jun 20, 2025

Uh oh!

Michael137 commented Jun 20, 2025

Uh oh!

Uh oh!

[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes #144880

[lldb][DWARF] Remove object_pointer from ParsedDWARFAttributes #144880

Uh oh!

Conversation

Michael137 commented Jun 19, 2025

Uh oh!

llvmbot commented Jun 19, 2025

Uh oh!

github-actions bot commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JDevlieghere commented Jun 20, 2025

Uh oh!

Michael137 commented Jun 20, 2025

Uh oh!

Uh oh!

github-actions bot commented Jun 19, 2025 •

edited

Loading