[PATCH] D57939: Refactor RelocVisitor and fix computation of SHT_RELA-typed relocation entries

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFRelocMap.h:24
+  object::RelocationResolver Resolver;
+  uint64_t S;
 };
----------------
dblaikie wrote:
> Could this have a more verbose/self-descriptive name?
Changed it to `SymbolValue`.

In ELF ABI, this is typically: "S Represents the value of the symbol whose index resides in the relocation entry"


================
Comment at: lib/DebugInfo/DWARF/DWARFDataExtractor.cpp:27
+    *SecNdx = E->SectionIndex;
+  return E->Resolver(E->Reloc, E->S, A).getValueOr(A);
 }
----------------
dblaikie wrote:
> Could RelocAddrEntry maybe have a "resolve" function that handles most of this? (eg: passes in E->Reloc and E->S, handles the "getValueOr" part too, probably - and/or maybe the "getValueOr" part could be handled by having a default/trivial Resolver that returns A? Less branching in the code required that way)
Changed it to `return E->Resolver(E->Reloc, E->SymbolValue, A);`

The check (whether Resolver can handle the relocation type) in DWARFContext.cpp is gross but efficient..


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57939