[lldb][DWARFASTParserClang] Don't overwrite DW_AT_object_pointer of definition with that of a declaration (#123089)

In #122742 we will start
attaching DW_AT_object_pointer to method declarations (in addition to
definitions).

Currently when LLDB parses a `DW_TAG_subprogram` definition, it will
parse all the attributes of the declaration as well. If we have
`DW_AT_object_pointer` on both, then we would overwrite the more
specific attribute that we got from the defintion with the one from the
specification. This is problematic because LLDB relies on getting the
`DW_AT_name` from the `DW_AT_object_pointer`, which doesn't exist on the
specification.

Note GCC does attach `DW_AT_object_pointer` on declarations *and*
definitions already (see https://godbolt.org/z/G1GvddY48), so there's
definitely some expressions that will fail for GCC compiled binaries.
This patch will fix those cases (e.g., I would expect `TestConstThis.py`
to fail with GCC).

Author: Michael137

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

@@ -377,7 +377,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) {
       break;
 
     case DW_AT_object_pointer:
-      object_pointer = form_value.Reference();
+      // 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:

lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

@@ -741,3 +741,164 @@ TEST_F(DWARFASTParserClangTests, TestUniqueDWARFASTTypeMap_CppInsertMapFind) {
 
   ASSERT_EQ(type_sp, reparsed_type_sp);
 }
+
+TEST_F(DWARFASTParserClangTests, TestParseDWARFAttributes_ObjectPointer) {
+  // This tests the behaviour of ParsedDWARFTypeAttributes
+  // 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
+  // one from the more specific definition's.
+
+  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
+            - Attribute:       DW_AT_specification
+              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
+            - Value:           0x14
+
+#       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));
+
+  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);
+}