[PATCH] D82899: [ELF] Resolve R_DTPREL in .debug_* referencing discarded symbols to -1

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: avl, davide, dblaikie, grimar, jhenderson, phosek, probinson, psmith.
Herald added subscribers: llvm-commits, s.egerton, PkmX, atanasyan, simoncook, kristof.beyls, arichardson, sdardis, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

The location of a TLS variable is encoded as a DW_OP_const4u/DW_OP_const8u
followed by a DW_OP_push_tls_address (or DW_OP_GNU_push_tls_address if tuned for
gdb https://sourceware.org/bugzilla/show_bug.cgi?id=11616 ).

This change follows up to D81784 <https://reviews.llvm.org/D81784> and makes relocations types generalized as
R_DTPREL (e.g. R_X86_64_DTPOFF{32,64}, R_PPC64_DTPREL64) use -1 as the
tombstone value as well. This works for both TLS Variant I and Variant II
architectures.

- arm: .long tls(tlsldo)
- mips64: .dtpreldword tls+32768
- ppc64: .quad tls at DTPREL+0x8000
- riscv: neither GCC nor clang has implemented DW_AT_location. It is likely .long/.quad tls at dtprel+0x800
- x86-32: .long tls at DTPOFF
- x86-64: .quad tls at DTPOFF

tls has a non-negative st_value, so such relocations never resolve to -1
in a normal (not discarded) case.

  // clang -fuse-ld=lld -g -ffunction-sections a.c -Wl,--gc-sections
  // foo and tls will be discarded by --gc-sections.
  // DW_AT_location [DW_FORM_exprloc] (DW_OP_const8u 0xffffffffffffffff, DW_OP_GNU_push_tls_address)
  thread_local int tls;
  int foo() { return ++tls; }
  int main() {}

Also, drop logic added in D26201 <https://reviews.llvm.org/D26201> intended to address PR30793. It added a test
(gc-debuginfo-tls.s) using a non-SHF_ALLOC section and a local symbol, which
does not reflect the intended scenario: a relocation in a SHF_ALLOC section
referencing a discarded non-local symbol. For such a non .debug_* section, just
emit an error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82899

Files:
  lld/ELF/InputSection.cpp
  lld/test/ELF/debug-dead-reloc-tls.s
  lld/test/ELF/gc-debuginfo-tls.s


Index: lld/test/ELF/gc-debuginfo-tls.s
===================================================================
--- lld/test/ELF/gc-debuginfo-tls.s
+++ lld/test/ELF/gc-debuginfo-tls.s
@@ -1,10 +1,11 @@
 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
-# RUN: ld.lld %t.o --gc-sections -shared -o %t1
+# RUN: not ld.lld %t.o --gc-sections -shared -o %t1 2>&1 | FileCheck %s --check-prefix=ERR
 # RUN: ld.lld %t.o -shared -o %t2
-# RUN: llvm-readobj --symbols %t1 | FileCheck %s --check-prefix=GC
 # RUN: llvm-readobj --symbols %t2 | FileCheck %s --check-prefix=NOGC
 
+# ERR: error: {{.*}}.o has an STT_TLS symbol but doesn't have an SHF_TL
+
 # NOGC:      Symbol {
 # NOGC:        Name: patatino
 # NOGC-NEXT:   Value: 0x0
@@ -15,8 +16,6 @@
 # NOGC-NEXT:   Section: .tbss
 # NOGC-NEXT: }
 
-# GC-NOT: tbss
-
 .section .tbss,"awT", at nobits
 patatino:
   .long 0
Index: lld/test/ELF/debug-dead-reloc-tls.s
===================================================================
--- /dev/null
+++ lld/test/ELF/debug-dead-reloc-tls.s
@@ -0,0 +1,27 @@
+# REQUIRES: x86
+## Test we resolve relocations referencing TLS symbols in .debug_* sections to
+## a tombstone value if the referenced TLS symbol is discarded.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: ld.lld --gc-sections %t.o -o %t
+# RUN: llvm-objdump -s %t | FileCheck %s
+
+# CHECK:      Contents of section .debug_info:
+# CHECK-NEXT:  0000 ffffffff ffffffff ffffffff ffffffff
+
+.globl _start
+_start:
+  ret
+
+.section .tbss,"awT"
+.globl global
+local:
+global:
+  .quad 0
+
+.section .debug_info
+## On ppc64, st_value 0 is supposed to point to 0x8000 bytes past the start of
+## the dynamic TLS vector. References usually have an addend of 0x8000.
+## MIPS is similar. RISC-V uses 0x800. Many other architectures don't use an offset.
+  .quad local+0x8000
+  .quad global+0x8000
Index: lld/ELF/InputSection.cpp
===================================================================
--- lld/ELF/InputSection.cpp
+++ lld/ELF/InputSection.cpp
@@ -906,12 +906,7 @@
       continue;
     }
 
-    if (sym.isTls() && !Out::tlsPhdr) {
-      target->relocateNoSym(bufLoc, type, 0);
-      continue;
-    }
-
-    if (isDebug && type == target->symbolicRel) {
+    if (isDebug && (type == target->symbolicRel || expr == R_DTPREL)) {
       // Resolve relocations in .debug_* referencing (discarded symbols or ICF
       // folded section symbols) to a tombstone value. Resolving to addend is
       // unsatisfactory because the result address range may collide with a
@@ -923,6 +918,10 @@
       // to resolve an address attribute (which may have a non-zero addend) to
       // -1+addend (wrap around to a low address).
       //
+      // R_DTPREL typed relocations represent an offset into the dynamic thread
+      // vector. The computed value is st_value plus a non-negative offset.
+      // Negative values are invalid, so -1 can be used as the tombstone value.
+      //
       // 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`


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82899.274558.patch
Type: text/x-patch
Size: 3202 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200630/c76e7d3f/attachment.bin>