[PATCH] D89099: [lld][ELF] Rewrite ICF-folded references rather than tombstoning them

[Petr Hosek via Phabricator via llvm-commits]

phosek created this revision.
phosek added reviewers: mcgrathr, MaskRay, dblaikie, jhenderson, probinson.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.
phosek requested review of this revision.

This modifies the logic introduced in D82828 <https://reviews.llvm.org/D82828> to address the case that
was brought up in PR47150.

Currently, we use tombstone value for all discarded references in
.debug_line section, but we rewrite ICF references in other debug
sections. This introduces inconsistency between .debug_line and
.debug_info data which is a problem for consumers such as GSYM.

This change modifies the behavior to be more consistent: we always
rewrite the reference for ICF folded case across all debug sections,
and we always use the tombstone value for all other cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89099

Files:
  lld/ELF/InputSection.cpp
  lld/test/ELF/dead-reloc-in-nonalloc.s
  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
@@ -9,7 +9,8 @@
 # RUN: llvm-objdump -s %t | FileCheck %s
 
 # CHECK:      Contents of section .debug_info:
-# CHECK-NEXT:  0000 {{[0-9a-f]+}}000 00000000 00000000 00000000
+# CHECK-NEXT:  0000 [[ADDR:[0-9a-f]+]] 00000000
+# CHECK-SAME:                                   [[ADDR]] 00000000
 # CHECK:      Contents of section .debug_line:
 # CHECK-NEXT:  0000 [[ADDR:[0-9a-f]+]] 00000000
 # CHECK-SAME:                                   [[ADDR]] 00000000
Index: lld/test/ELF/dead-reloc-in-nonalloc.s
===================================================================
--- lld/test/ELF/dead-reloc-in-nonalloc.s
+++ lld/test/ELF/dead-reloc-in-nonalloc.s
@@ -11,10 +11,11 @@
 # RUN:   -z dead-reloc-in-nonalloc=.not_debug=0xbbbbbbbb %t.o -o - | cmp %t -
 
 # COMMON:      Contents of section .debug_addr:
-# COMMON-NEXT:  0000 [[ADDR:[0-9a-f]+]] 00000000 00000000 00000000
+# COMMON-NEXT:  0000 [[ADDR:[0-9a-f]+]] 00000000
+# COMMON-SAME:                                   [[ADDR]] 00000000
 
 # AA:          Contents of section .debug_info:
-# AA-NEXT:      0000 [[ADDR]] 00000000 aaaaaaaa 00000000
+# AA-NEXT:      0000 [[ADDR]] 00000000 [[ADDR]] 00000000
 # AA:          Contents of section .not_debug:
 # AA-NEXT:      0000 bbbbbbbb
 
@@ -23,7 +24,8 @@
 # RUN: llvm-objdump -s %tzero | FileCheck %s --check-prefixes=COMMON,ZERO
 
 # ZERO:        Contents of section .debug_info:
-# ZERO-NEXT:    0000 {{[0-9a-f]+}}000 00000000 00000000 00000000
+# ZERO-NEXT:    0000 [[ADDR:[0-9a-f]+]] 00000000
+# ZERO-SAME:                                     [[ADDR]] 00000000
 
 ## Glob works.
 # RUN: ld.lld --icf=all -z dead-reloc-in-nonalloc='.debug_i*=0xaaaaaaaa' \
Index: lld/ELF/InputSection.cpp
===================================================================
--- lld/ELF/InputSection.cpp
+++ lld/ELF/InputSection.cpp
@@ -864,7 +864,6 @@
   const bool isDebug = isDebugSection(*this);
   const bool isDebugLocOrRanges =
       isDebug && (name == ".debug_loc" || name == ".debug_ranges");
-  const bool isDebugLine = isDebug && name == ".debug_line";
   Optional<uint64_t> tombstone;
   for (const auto &patAndValue : llvm::reverse(config->deadRelocInNonAlloc))
     if (patAndValue.first.match(this->name)) {
@@ -942,9 +941,8 @@
       // 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. 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.
+      // catches the ICF folded case where using tombstone value is undesirable
+      // because it would prevent detecting this case on the consumer side.
       //
       // For pre-DWARF-v5 .debug_loc and .debug_ranges, -1 is a reserved value
       // (base address selection entry), use 1 (which is used by GNU ld for
@@ -954,7 +952,7 @@
       // value. Enable -1 in a future release.
       auto *ds = dyn_cast<Defined>(&sym);
       if (!sym.getOutputSection() ||
-          (ds && ds->section->repl != ds->section && !isDebugLine)) {
+          (ds && ds->section->repl != ds->section && !isDebug)) {
         // If -z dead-reloc-in-nonalloc= is specified, respect it.
         const uint64_t value = tombstone ? SignExtend64<bits>(*tombstone)
                                          : (isDebugLocOrRanges ? 1 : 0);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89099.297135.patch
Type: text/x-patch
Size: 3684 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201009/348b9192/attachment.bin>