[dsymutil] Fix .debug_addr index calculation

[JDevlieghere]

Summary

The DW_OP_addrx operation encodes a zero-based index into the .debug_addr section. The index is relative to the DW_AT_addr_base attribute of the associated compilation unit. In order to compute the offset into the .debug_addr section and find the associated relocation, we need to add the add the add base offset and multiply the index with the address size. This patch fixes a bug in this computation, where the multiplication was omitted. This went unnoticed because for small test cases, the index for interesting addresses (such as the main subprogram) is often zero.

rdar://110881668

[avl]

LGTM, with addressing small code simplification suggestion.

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

1093

probably, just

return hasValidRelocationAt(ValidDebugAddrRelocs, StartOffset, EndOffset);

?

[JDevlieghere]

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

1093

Yup, thanks, I meant to unstage that part.

[JDevlieghere]

This patch fixes a 125 tests when running the LLDB test suite against DWARF 5 on Darwin. 🥳

[thakis]

Seems to break tests: http://45.33.8.238/linux/110001/step_12.txt

Please take a look and revert for now if it takes a while to fix.

[JDevlieghere]

In D153162#4429293, @thakis wrote:

Seems to break tests: http://45.33.8.238/linux/110001/step_12.txt

Please take a look and revert for now if it takes a while to fix.

Working on it, just a matter of updating the tests.

[aprantl]

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

1087

I find the name ByteSize misleading. Would you be okay with renaming it to AddrSize? Then it's consistent with how dwarfdump prints the .debug_addr header.

[JDevlieghere]

In D153162#4429314, @JDevlieghere wrote:

In D153162#4429293, @thakis wrote:

Seems to break tests: http://45.33.8.238/linux/110001/step_12.txt

Please take a look and revert for now if it takes a while to fix.

Working on it, just a matter of updating the tests.

Fixed in 8e1f820bb4ea.

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

1087

Done in 8e1f820bb4ea.

llvm/test/tools/dsymutil/ARM/dwarf5-addrx-0x0-last.test

+$ cat dwarf5-addrx-0x0-last.c
+#include <stdio.h>
+
+int main (int argc, char const *argv[])
+{
+  int pass_me = argc + 10;
+  printf("Foo\n");
+  printf("Bar\n");
+
+  return 0;
+}
+
+$ clang -gdwarf-5 dwarf5-addrx-0x0-last.c -c -o dwarf5-addrx-0x0-last.o
+$ clang dwarf5-addrx-0x0-last.o -o dwarf5-addrx-0x0-last.out
+
+# Sanity check: make sure main's low PC (0x0) requires an index computation
+# into the .debug_addr section.
+RUN: llvm-dwarfdump -debug-addr %p/../Inputs/private/tmp/dwarf5/dwarf5-addrx-0x0-last.o | FileCheck %s --check-prefix DEBUGADDR
+
+DEBUGADDR: Addrs: [
+DEBUGADDR: 0x0000000000000054
+DEBUGADDR: 0x0000000000000059
+DEBUGADDR: 0x0000000000000000
+DEBUGADDR: ]
+
+RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/dwarf5/dwarf5-addrx-0x0-last.out -o %t.dSYM 2>&1 | FileCheck %s --allow-empty
+RUN: llvm-dwarfdump --verify %t.dSYM 2>&1 | FileCheck %s
+RUN: llvm-dwarfdump --verbose -debug-info %t.dSYM | FileCheck %s --check-prefix DEBUGINFO
+RUN: llvm-dwarfdump --verbose -debug-line %t.dSYM | FileCheck %s --check-prefix DEBUGLINE
+
+CHECK-NOT: error:
+
+DEBUGINFO:   DW_TAG_subprogram
+DEBUGINFO:     DW_AT_low_pc [DW_FORM_addr]     (0x0000000100003f4c)
+DEBUGINFO:     DW_AT_high_pc [DW_FORM_data4]   (0x00000054)
+DEBUGINFO:     DW_AT_name [DW_FORM_strp]       ( .debug_str[0x0000011c] = "main")
+
+DEBUGLINE:  0x0000000100003f4c   4   0  {{.*}}  is_stmt
+DEBUGLINE:  0x0000000100003f6c   5  17  {{.*}}  is_stmt prologue_end
+DEBUGLINE:  0x0000000100003f70   5  22  {{.*}}
+DEBUGLINE:  0x0000000100003f74   5   7  {{.*}}
+DEBUGLINE:  0x0000000100003f78   6   3  {{.*}}  is_stmt
+DEBUGLINE:  0x0000000100003f84   7   3  {{.*}}  is_stmt
+DEBUGLINE:  0x0000000100003f94   9   3  {{.*}}  is_stmt epilogue_begin
+DEBUGLINE:  0x0000000100003fa0   9   3  {{.*}}  is_stmt end_sequence

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

std::optional<int64_t> DwarfLinkerForBinary::AddressManager<
   case dwarf::DW_FORM_addrx:
   case dwarf::DW_FORM_addrx1:
   case dwarf::DW_FORM_addrx2:
   case dwarf::DW_FORM_addrx3:
   case dwarf::DW_FORM_addrx4: {
     std::optional<DWARFFormValue> AddrValue = DIE.find(dwarf::DW_AT_low_pc);
     if (std::optional<uint64_t> AddrOffsetSectionBase =
             DIE.getDwarfUnit()->getAddrOffsetSectionBase()) {
-      uint64_t StartOffset = *AddrOffsetSectionBase + AddrValue->getRawUValue();
-      uint64_t EndOffset =
-          StartOffset + DIE.getDwarfUnit()->getAddressByteSize();
-      return hasValidRelocationAt(ValidDebugAddrRelocs, StartOffset, EndOffset);
+      // Addrx is a index into the debug_addr section, not an offset, so we need
+      // to multiply by byte size.
+      const uint64_t ByteSize = DIE.getDwarfUnit()->getAddressByteSize();
+      const uint64_t StartOffset =
+          *AddrOffsetSectionBase + (AddrValue->getRawUValue() * ByteSize);
+      const uint64_t EndOffset = StartOffset + ByteSize;
+      if (std::optional<int64_t> RelocAdjustment = hasValidRelocationAt(
+              ValidDebugAddrRelocs, StartOffset, EndOffset))
+        return RelocAdjustment;
+      return std::nullopt;
     }
 
     Linker.reportWarning("no base offset for address table", SrcFileName);
     return std::nullopt;
   }
   default:
     return std::nullopt;
   }