Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2025

Conversation

Michael137
Copy link
Member

Reverts #144880

Caused TestObjCIvarsInBlocks.py to fail on macOS CI.

@Michael137 Michael137 requested a review from JDevlieghere as a code owner June 20, 2025 16:20
@Michael137 Michael137 merged commit 8775119 into main Jun 20, 2025
6 of 8 checks passed
@Michael137 Michael137 deleted the revert-144880-lldb/cxx-object-parameter-changes-3 branch June 20, 2025 16:21
@llvmbot llvmbot added the lldb label Jun 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

Reverts llvm/llvm-project#144880

Caused TestObjCIvarsInBlocks.py to fail on macOS CI.


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

2 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+20-8)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (+3-4)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3bec89cdf7469..4f79c8aa3f811 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -445,6 +445,15 @@ 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;
@@ -1107,7 +1116,7 @@ bool DWARFASTParserClang::ParseObjCMethod(
 std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod(
     const DWARFDIE &die, CompilerType clang_type,
     const ParsedDWARFTypeAttributes &attrs, const DWARFDIE &decl_ctx_die,
-    const DWARFDIE &object_parameter, bool &ignore_containing_context) {
+    bool is_static, bool &ignore_containing_context) {
   Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
   SymbolFileDWARF *dwarf = die.GetDWARF();
   assert(dwarf);
@@ -1191,9 +1200,6 @@ 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...
@@ -1219,7 +1225,9 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod(
     ClangASTMetadata metadata;
     metadata.SetUserID(die.GetID());
 
-    if (char const *object_pointer_name = object_parameter.GetName()) {
+    char const *object_pointer_name =
+        attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr;
+    if (object_pointer_name) {
       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));
@@ -1315,9 +1323,11 @@ 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,
-                           object_parameter, ignore_containing_context);
+            ParseCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static,
+                           ignore_containing_context);
         if (type_sp)
           return type_sp;
 
@@ -1412,7 +1422,9 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
           ClangASTMetadata metadata;
           metadata.SetUserID(die.GetID());
 
-          if (char const *object_pointer_name = object_parameter.GetName()) {
+          char const *object_pointer_name =
+              attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr;
+          if (object_pointer_name) {
             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 a90f55bcff948..111604ce4068a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -470,8 +470,7 @@ 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] object_parameter The DIE of this subprogram's object parameter.
-  ///                             May be an invalid DIE for C++ static methods.
+  /// \param[in] is_static Is true iff we're parsing a static method.
   ///
   /// \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.
@@ -486,8 +485,7 @@ 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,
-                 const lldb_private::plugin::dwarf::DWARFDIE &object_parameter,
-                 bool &ignore_containing_context);
+                 bool is_static, bool &ignore_containing_context);
 
   lldb::TypeSP ParseArrayType(const lldb_private::plugin::dwarf::DWARFDIE &die,
                               const ParsedDWARFTypeAttributes &attrs);
@@ -557,6 +555,7 @@ 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;

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Jun 21, 2025
llvm#145065)"

This reverts commit 8775119.

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.
Michael137 added a commit to Michael137/llvm-project that referenced this pull request Jun 21, 2025
llvm#145065)"

This reverts commit 8775119.

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.

(cherry picked from commit 2572a2f)
Michael137 added a commit to Michael137/llvm-project that referenced this pull request Jun 22, 2025
llvm#145065)"

This reverts commit 8775119.

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.

(cherry picked from commit 2572a2f)
Michael137 added a commit that referenced this pull request Jun 23, 2025
#145065)" (#145126)

This reverts commit 8775119.

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.
miguelcsx pushed a commit to miguelcsx/llvm-project that referenced this pull request Jun 23, 2025
llvm#145065)" (llvm#145126)

This reverts commit 8775119.

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.
Jaddyen pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 23, 2025
llvm#145065)" (llvm#145126)

This reverts commit 8775119.

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.
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
llvm#145065)" (llvm#145126)

This reverts commit 8775119.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants