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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

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`



================
Comment at: lld/ELF/Driver.cpp:1074
+  for (opt::Arg *arg : args.filtered(OPT_z)) {
+    constexpr StringRef prefix = "-z dead-reloc-in-nonalloc=: ";
+    std::pair<StringRef, StringRef> option =
----------------
I'd move it after the first `continue`. Also, perhaps, `errPrefix` would be more clear name for this variable.


================
Comment at: lld/test/ELF/dead-reloc-in-nonalloc.s:9
+# RUN: llvm-objdump -s %t | FileCheck %s --check-prefixes=COMMON,AA
+# RUN: ld.lld --icf=all -z dead-reloc-in-nonalloc=.debug_info=2863311530 \
+# RUN:   -z dead-reloc-in-nonalloc=.not_debug=0xbbbbbbbb %t.o -o - | cmp %t -
----------------
Please add a comment saying that 2863311530 == 0xaaaaaaaa.


================
Comment at: lld/test/ELF/dead-reloc-in-nonalloc.s:35
+
+# RUN: not ld.lld -z dead-reloc-in-nonalloc= 2>&1 | FileCheck %s --check-prefix=USAGE
+# RUN: not ld.lld -z dead-reloc-in-nonalloc=a= 2>&1 | FileCheck %s --check-prefix=USAGE
----------------
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).


================
Comment at: lld/test/ELF/debug-dead-reloc.s:27
+# OVERRIDE:      Contents of section .debug_loc:
+# OVERRIDE-NEXT:  0000 2a000000 00000000 2a000000 00000000
+
----------------
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
...
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83264/new/

https://reviews.llvm.org/D83264