[PATCH] D81784: [ELF] Resolve relocations in .debug_* referencing (discarded symbols or ICF folded section symbols) to tombstone values

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/InputSection.cpp:857
+  bool isDebugLocOrRanges =
+      isDebug && (name == ".debug_loc" || name == ".debug_ranges");
 
----------------
use `const bool` for these 2?


================
Comment at: lld/ELF/InputSection.cpp:912
+    }
+    if (isDebug && type == target->symbolicRel) {
+      // Resolve relocations in .debug_* referencing (discarded symbols or ICF
----------------
Missing an empty line before `if`.


================
Comment at: lld/ELF/InputSection.cpp:935
+                              isDebugLocOrRanges ? UINT64_MAX - 1 : UINT64_MAX);
+        continue;
+      }
----------------
It looks a bit unideal to me, because `auto *ds = dyn_cast<Defined>(&sym)` is always called,
though it is not always needed.
The same happens with `isDebugLocOrRanges ? UINT64_MAX - 1 : UINT64_MAX` condition, which 
can be caluclated once.
Also `isDebugSection()` just do an excessive job it seems, because it checks the ".debug_" prefix.
And `sym.getOutputSection()` checks that `sym` is Defined internally, so this check is done twice too.

What about doing the following?


```
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 uint64_t Tombstone = (name == ".debug_loc" || name == ".debug_ranges")
                                 ? (UINT64_MAX - 1)
                                 : UINT64_MAX;
  ....

    if (sym.isTls() && !Out::tlsPhdr) {
      target->relocateNoSym(bufLoc, type, 0);
      continue;
    }

    if (!isDebug || type != target->symbolicRel) {
      target->relocateNoSym(bufLoc, type,
                            SignExtend64<bits>(sym.getVA(addend)));
      continue;
    }

    auto *ds = dyn_cast<Defined>(&sym);
    if (!ds || !ds->section->repl || !ds->section->repl->getOutputSection() ||
        ds->section->repl != ds->section)
      target->relocateNoSym(bufLoc, type, Tombstone);
    else
      target->relocateNoSym(bufLoc, type,
                            SignExtend64<bits>(sym.getVA(addend)));
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81784