[ELF] Add -z dead-reloc-in-nonalloc=<section_glob>=<value>

[MaskRay]

Summary

... to customize the tombstone value we use for an absolute relocation
referencing a discarded symbol. This can be used as a workaround when
some debug processing tool has trouble with current -1 tombstone value
(https://bugs.chromium.org/p/chromium/issues/detail?id=1102223#c11 )

For example, to get the current built-in rules (not considering the .debug_line special case for ICF):

-z dead-reloc-in-nonalloc='.debug_*=0xffffffffffffffff'
-z dead-reloc-in-nonalloc=.debug_loc=0xfffffffffffffffe
-z dead-reloc-in-nonalloc=.debug_ranges=0xfffffffffffffffe

To get GNU ld (as of binutils 2.35)'s behavior:

-z dead-reloc-in-nonalloc='*=0'
-z dead-reloc-in-nonalloc=.debug_ranges=1

This option has other use cases. For example, if we want to check
whether a non-SHF_ALLOC section, say, .foo has dead relocations.
With this patch, we can run a regular LLD and run another with a special
-z dead-reloc-in-nonalloc=, then compare their output.

[MaskRay]

Add more tests

[psmith]

I think an option makes sense. I have a small reservation about making users order their matches on the command line. We should make it more explicit in the help and the manual, or try and sort the matches using a heuristic order of specificity, for example no wildcards is more specific than wildcards, longer text is more specific than shorter. Other than that I think it looks OK.

lld/ELF/Options.td

127

I think it will be worth mentioning that the there can be multiple occurrences and that the last takes precedence.
"Resolve a relocation from a matched non-SHF_ALLOC section to a discarded symbol to the specified value. Accepts wildcards, in the event of a section matching more than one instance of this option, the last instance on the command-line takes precedence."

lld/docs/ld.lld.1

626 ↗

(On Diff #275868)

Similar to the comment above. Although as the man-page we can elaborate a bit.
"Resolve a relocation from a matched non-SHF_ALLOC section to a discarded symbol to the specified value. Accepts wildcards, in the event of a section matching more than one instance of this option, the last instance on the command-line takes precedence. An order of least specific
to most specific match is recommended."

[jhenderson]

Aside from the inline comment, this looks reasonable, but I haven't looked in detail due to time constraints.

lld/test/ELF/dead-reloc-in-nonalloc.s

32–33 ↗

(On Diff #275868)

I'd be slightly tempted to swap the two arguments around, to show that it is definitely last, and not most-specific, that wins.

[grimar]

Probably there are other names rather than "dead-reloc-in-nonalloc" which we might want to consider?

-z dead-noalloc-reloc-val
-z tombstone-reloc
-z resolve-dead-reloc

lld/ELF/Driver.cpp

1074

I'd move it after the first continue. Also, perhaps, errPrefix would be more clear name for this variable.

lld/test/ELF/dead-reloc-in-nonalloc.s

9 ↗

(On Diff #275868)

Please add a comment saying that 2863311530 == 0xaaaaaaaa.

35 ↗

(On Diff #275868)

I'd leave a single common comment saying you're going to check all possible invalid cases now.
(to separate these tests from above ones).

lld/test/ELF/debug-dead-reloc.s

27 ↗

(On Diff #275868)

What about printing other sections content too?
(seems there is no other test showing that when override the tombstone value for a debug section, the other ones
remain unaffected)

Probably it worth to combine CHECK and OVERRIDE:

# CHECK:           Contents of section .debug_loc:
# NOOVERRIDE-NEXT: 0000 feffffff ffffffff feffffff ffffffff
# OVERRIDE-NEXT:   0000 2a000000 00000000 2a000000 00000000
...

[MaskRay]

Address comments

lld/ELF/Options.td

127

Thanks for the suggestion.

My fault, we don't actually have a help for -z options, so I can only leave the message in ld.lld.1
We probably should figure out a way to print -z options in --help.

I pick -z because nonalloc is an ELF specific concept.

lld/test/ELF/debug-dead-reloc.s

27 ↗

(On Diff #275868)

I think the value testing more section contents is low. This is just an auxiliary test showing that --gc-sections is similar to ICF.

(Moreover, NOOVERRIDE is longer than CHECK and I would need to re-align CHECK-NEXT: above.)

[thakis]

Thanks! Should the release notes grow an entry mentioning the behavior change and this flag?

[MaskRay]

In D83264#2137849, @thakis wrote:

Thanks! Should the release notes grow an entry mentioning the behavior change and this flag?

Good idea. Will send a review afterwards.

[thakis]

Cool. Could you check this in, please?

[grimar]

In D83264#2135482, @grimar wrote:

Probably there are other names rather than "dead-reloc-in-nonalloc" which we might want to consider?

-z dead-noalloc-reloc-val
-z tombstone-reloc
-z resolve-dead-reloc

This remained unanswered.

[MaskRay]

In D83264#2140954, @grimar wrote:

In D83264#2135482, @grimar wrote:

Probably there are other names rather than "dead-reloc-in-nonalloc" which we might want to consider?

-z dead-noalloc-reloc-val
-z tombstone-reloc
-z resolve-dead-reloc

This remained unanswered.

Dead relocations in SHF_ALLOC sections are errors. relocateNonAlloc does not handle them, so -z tombstone-reloc and -z resolve-dead-reloc are excluded.

For -z dead-nonalloc-reloc-val The adjective nonalloc appears to describe reloc, which is not precise. nonalloc describes the section.

I still consider -z dead-reloc-in-nonalloc the best.

lld/ELF/Config.h

struct Configuration {
   bool armJ1J2BranchEncoding = false;
   bool asNeeded = false;
   bool bsymbolic;
   bool bsymbolicFunctions;
   bool callGraphProfileSort;
   bool checkSections;
   bool compressDebugSections;
   bool cref;
+  std::vector<std::pair<llvm::GlobPattern, uint64_t>> deadNonAllocReloc;
   bool defineCommon;
   bool demangle = true;
   bool dependentLibraries;
   bool disableVerify;
   bool ehFrameHdr;
   bool emitLLVM;
   bool emitRelocs;
   bool enableNewDtags;

lld/ELF/Driver.cpp

return s == "combreloc" || s == "copyreloc" || s == "defs" ||
          s == "nocombreloc" || s == "nocopyreloc" || s == "nodefaultlib" ||
          s == "nodelete" || s == "nodlopen" || s == "noexecstack" ||
          s == "nognustack" || s == "nokeep-text-section-prefix" ||
          s == "norelro" || s == "noseparate-code" || s == "notext" ||
          s == "now" || s == "origin" || s == "pac-plt" || s == "rel" ||
          s == "rela" || s == "relro" || s == "retpolineplt" ||
          s == "rodynamic" || s == "shstk" || s == "text" || s == "undefs" ||
          s == "wxneeded" || s.startswith("common-page-size=") ||
+         s.startswith("dead-reloc-in-nonalloc=") ||
          s.startswith("max-page-size=") || s.startswith("stack-size=") ||
          s.startswith("start-stop-visibility=");
 }
 
 // Report an error for an unknown -z option.
 static void checkZOptions(opt::InputArgList &args) {
   for (auto *arg : args.filtered(OPT_z))
     if (!isKnownZFlag(arg->getValue()))
static void readConfigs(opt::InputArgList &args) {
   config->zRodynamic = hasZOption(args, "rodynamic");
   config->zSeparate = getZSeparate(args);
   config->zShstk = hasZOption(args, "shstk");
   config->zStackSize = args::getZOptionValue(args, OPT_z, "stack-size", 0);
   config->zStartStopVisibility = getZStartStopVisibility(args);
   config->zText = getZFlag(args, "text", "notext", true);
   config->zWxneeded = hasZOption(args, "wxneeded");
 
+  for (opt::Arg *arg : args.filtered(OPT_z)) {
+    std::pair<StringRef, StringRef> option =
+        StringRef(arg->getValue()).split('=');
+    if (option.first != "dead-reloc-in-nonalloc")
+      continue;
+    std::pair<StringRef, StringRef> kv = option.second.split('=');
+    if (kv.second.empty()) {
+      error(arg->getSpelling() + ": expected <section_glob>=<value>");
+      continue;
+    }
+    uint64_t v;
+    if (!to_integer(kv.second, v))
+      error(arg->getSpelling() + ": expected an integer, but got '" +
+            kv.second + "'");
+    else if (Expected<GlobPattern> pat = GlobPattern::create(kv.first))
+      config->deadNonAllocReloc.emplace_back(std::move(*pat), v);
+    else
+      error(arg->getSpelling() + ": " + toString(pat.takeError()));
+  }
+
   // Parse LTO options.
   if (auto *arg = args.getLastArg(OPT_plugin_opt_mcpu_eq))
     parseClangOption(saver.save("-mcpu=" + StringRef(arg->getValue())),
                      arg->getSpelling());
 
   for (opt::Arg *arg : args.filtered(OPT_plugin_opt_eq_minus))
     parseClangOption(std::string("-") + arg->getValue(), arg->getSpelling());
 

lld/ELF/InputSection.cpp

 // function as a performance optimization.
 template <class ELFT, class RelTy>
 void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef<RelTy> rels) {
   const unsigned bits = sizeof(typename ELFT::uint) * 8;
   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->deadNonAllocReloc))
+    if (patAndValue.first.match(this->name)) {
+      tombstone = patAndValue.second;
+      break;
+    }
 
   for (const RelTy &rel : rels) {
     RelType type = rel.getType(config->isMips64EL);
 
     // GCC 8.0 or earlier have a bug that they emit R_386_GOTPC relocations
     // against _GLOBAL_OFFSET_TABLE_ for .debug_info. The bug has been fixed
     // in 2017 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82630), but we
     // need to keep this bug-compatible code for a while.
if (expr != R_ABS && expr != R_DTPREL && expr != R_RISCV_ADD) {
       // address 0. For bug-compatibilty, we accept them with warnings. We
       // know Steel Bank Common Lisp as of 2018 have this bug.
       warn(msg);
       target->relocateNoSym(bufLoc, type,
                             SignExtend64<bits>(sym.getVA(addend - offset)));
       continue;
     }
 
-    if (isDebug && (type == target->symbolicRel || expr == R_DTPREL)) {
+    if (tombstone ||
+        (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
       // valid range of low address, or leave multiple CUs claiming ownership of
       // the same range of code, which may confuse consumers.
       //
       // To address the problems, we use -1 as a tombstone value for most
       // .debug_* sections. We have to ignore the addend because we don't want
if (tombstone ||
       // .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 && !isDebugLine)) {
-        target->relocateNoSym(bufLoc, type,
-                              isDebugLocOrRanges ? UINT64_MAX - 1 : UINT64_MAX);
+        // If -z dead-reloc-in-nonalloc= is specified, respect it.
+        const uint64_t value =
+            tombstone ? SignExtend64<bits>(*tombstone)
+                      : (isDebugLocOrRanges ? UINT64_MAX - 1 : UINT64_MAX);
+        target->relocateNoSym(bufLoc, type, value);
         continue;
       }
     }
     target->relocateNoSym(bufLoc, type, SignExtend64<bits>(sym.getVA(addend)));
   }
 }
 
 // This is used when '-r' is given.

lld/ELF/Options.td

 def color_diagnostics_eq: J<"color-diagnostics=">,
   HelpText<"Use colors in diagnostics">,
   MetaVarName<"[auto,always,never]">;
 
 defm cref: B<"cref",
     "Output cross reference table",
     "Do not output cross reference table">;
 
+defm dead_nonalloc_reloc_value : EEq<"dead-nonalloc-reloc-value",
+  "Resolve a relocation from a matched non-SHF_ALLOC section to a discarded "
+  "symbol to the specified value">,
+  MetaVarName<"<section_glob>=<value>">;
+
 defm define_common: B<"define-common",
     "Assign space to common symbols",
     "Do not assign space to common symbols">;
 
 defm demangle: B<"demangle",
     "Demangle symbol names (default)",
     "Do not demangle symbol names">;
 

lld/test/ELF/debug-dead-reloc-icf.s

 # REQUIRES: x86
 ## Test we resolve symbolic relocations in .debug_* sections to a tombstone
 ## value if the referenced section symbol is folded into another section by ICF.
 ## Otherwise, we would leave entries in multiple CUs claiming ownership of the
 ## same range of code, which can confuse consumers.
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
 # RUN: ld.lld --icf=all %t.o -o %t
 # RUN: llvm-objdump -s %t | FileCheck %s
 
 # 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
 
+## -z dead-reloc-in-nonalloc can override the decision.
+# RUN: ld.lld --icf=all -z dead-reloc-in-nonalloc=.debug_info=3 %t.o -o %t1
+# RUN: llvm-objdump -s %t1 | FileCheck %s --check-prefix=OVERRIDE
+
+# OVERRIDE:      Contents of section .debug_info:
+# OVERRIDE-NEXT:  0000 {{[0-9a-f]+}}000 00000000 03000000 00000000
+# OVERRIDE:      Contents of section .debug_line:
+# OVERRIDE-NEXT:  0000 [[ADDR:[0-9a-f]+]] 00000000
+# OVERRIDE-SAME:                                   [[ADDR]] 00000000
+
 .globl _start
 _start:
   ret
 
 ## .text.1 will be folded by ICF.
 .section .text.1,"ax"
   ret