[PATCH] D82828: [ELF] Don't resolve a relocation in .debug_line referencing an ICF folded symbol to the tombstone value

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: avl, dblaikie, grimar, jhenderson, probinson, psmith.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

After D81784 <https://reviews.llvm.org/D81784>, we resolve a relocation in .debug_* referencing an ICF folded
section symbol to a tombstone value.

Doing this for .debug_line has a problem (https://reviews.llvm.org/D81784#2116925 ):
.debug_line may describe folded lines as having addresses UINT64_MAX or
some wraparound small addresses.

  int foo(int x) {
    return x; // line 2
  }
  
  int bar(int x) {
    return x; // line 6
  }



  Address            Line   Column File   ISA Discriminator Flags
  ------------------ ------ ------ ------ --- ------------- -------------
  0x00000000002016c0      1      0      1   0             0  is_stmt
  0x00000000002016c7      2      9      1   0             0  is_stmt
  prologue_end
  0x00000000002016ca      2      2      1   0             0
  0x00000000002016cc      2      2      1   0             0  end_sequence
  // UINT64_MAX and wraparound small addresses
  0xffffffffffffffff      5      0      1   0             0  is_stmt
  0x0000000000000006      6      9      1   0             0  is_stmt
  prologue_end
  0x0000000000000009      6      2      1   0             0
  0x000000000000000b      6      2      1   0             0  end_sequence
  0x00000000002016d0      9      0      1   0             0  is_stmt
  0x00000000002016df     10      6      1   0             0  is_stmt prologue_end
  0x00000000002016e6     11     11      1   0             0  is_stmt
  ...

These entries can confuse debuggers:

gdb (can't continue due to a breakpoint in an invalid region of memory):

  Warning:
  Cannot insert breakpoint 1.
  Cannot access memory at address 0x6

lldb (breakpoint has no effect):

  (lldb) b 6
  Breakpoint 1: no locations (pending).
  WARNING:  Unable to resolve breakpoint to any actual locations.

This patch special cases .debug_line to not use the tombstone value,
restoring the previous behavior: .debug_line will have entries with the
same addresses (ICF) but different line numbers. A breakpoint on line 2
or 6 will trigger on both functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82828

Files:
  lld/ELF/InputSection.cpp
  lld/test/ELF/debug-dead-reloc-icf.s


Index: lld/test/ELF/debug-dead-reloc-icf.s
===================================================================
--- lld/test/ELF/debug-dead-reloc-icf.s
+++ lld/test/ELF/debug-dead-reloc-icf.s
@@ -10,6 +10,9 @@
 
 # CHECK:      Contents of section .debug_info:
 # CHECK-NEXT:  0000 {{[0-9a-f]+}}000 00000000 ffffffff ffffffff
+# CHECK:      Contents of section .debug_line:
+# CHECK-NEXT:  0000 [[ADDR:[0-9a-f]+]] 00000000
+# CHECK-SAME:                                   [[ADDR]] 00000000
 
 .globl _start
 _start:
@@ -22,3 +25,11 @@
 .section .debug_info
   .quad .text+8
   .quad .text.1+8
+
+# .debug_line contribution associated to a folded function can describe
+# different lines from the canonical function. Leaving a tombstone value can
+# cause addresses like UINT64_MAX and some wraparound small addresses which will
+# confuse debuggers. Resolve the relocation to the folded .text.1 to .text
+.section .debug_line
+  .quad .text
+  .quad .text.1
Index: lld/ELF/InputSection.cpp
===================================================================
--- lld/ELF/InputSection.cpp
+++ lld/ELF/InputSection.cpp
@@ -856,6 +856,7 @@
   const bool isDebug = isDebugSection(*this);
   const bool isDebugLocOrRanges =
       isDebug && (name == ".debug_loc" || name == ".debug_ranges");
+  const bool isDebugLine = isDebug && name == ".debug_line";
 
   for (const RelTy &rel : rels) {
     RelType type = rel.getType(config->isMips64EL);
@@ -926,12 +927,15 @@
       // If the referenced symbol is discarded (made Undefined), or the
       // section defining the referenced symbol is garbage collected,
       // sym.getOutputSection() is nullptr. `ds->section->repl != ds->section`
-      // catches the ICF folded case.
+      // catches the ICF folded case. However, resolving a relocation in
+      // .debug_line to -1 can leave addresses like -1 and wraparound small
+      // addresses which can confuse debuggers. So exclude .debug_line.
       //
       // For pre-DWARF-v5 .debug_loc and .debug_ranges, -1 is a reserved value
       // (base address selection entry), so -2 is used.
       auto *ds = dyn_cast<Defined>(&sym);
-      if (!sym.getOutputSection() || (ds && ds->section->repl != ds->section)) {
+      if (!sym.getOutputSection() ||
+          (ds && ds->section->repl != ds->section && !isDebugLine)) {
         target->relocateNoSym(bufLoc, type,
                               isDebugLocOrRanges ? UINT64_MAX - 1 : UINT64_MAX);
         continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82828.274296.patch
Type: text/x-patch
Size: 2473 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200630/0fe38a27/attachment-0001.bin>