[lld] e6ad78f - [ELF] Don't resolve a relocation in .debug_line referencing an ICF folded symbol to the tombstone value

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2020-07-01T13:38:16-07:00
New Revision: e6ad78fe050392e6807593793f59e65b0e6dc433

URL: https://github.com/llvm/llvm-project/commit/e6ad78fe050392e6807593793f59e65b0e6dc433
DIFF: https://github.com/llvm/llvm-project/commit/e6ad78fe050392e6807593793f59e65b0e6dc433.diff

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

After 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 before 2020-07-01 (binutils-gdb a8caed5d7faa639a1e6769eba551d15d8ddd9510 "Recognize -1 as a tombstone value in .debug_line")
(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.

Reviewed By: dblaikie, jhenderson

Differential Revision: https://reviews.llvm.org/D82828

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 33bad9fbb0ef..8376ac4bfdf9 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -856,6 +856,7 @@ void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef<RelTy> rels) {
   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 @@ void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef<RelTy> rels) {
       // 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 would stop debugger users from setting breakpoints on
+      // the folded-in function, 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;

diff  --git a/lld/test/ELF/debug-dead-reloc-icf.s b/lld/test/ELF/debug-dead-reloc-icf.s
index 3230656f1fbc..716e245c12c9 100644
--- a/lld/test/ELF/debug-dead-reloc-icf.s
+++ b/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 @@ _start:
 .section .debug_info
   .quad .text+8
   .quad .text.1+8
+
+## .debug_line contributions associated with folded-in functions will describe
+## 
diff erent lines to the canonical function. Leaving a tombstone value would
+## prevent users from setting breakpoints on the folded-in functions.
+## Instead resolve the relocation to the folded .text.1 to .text
+.section .debug_line
+  .quad .text
+  .quad .text.1