[PATCH] D73254: [ELF] Rename relocateOne() to relocate() and pass `Relocation` to it

[Peter Smith via Phabricator via llvm-commits]

peter.smith added inline comments.


================
Comment at: lld/ELF/AArch64ErrataFix.cpp:424
   uint64_t p = patchSym->getVA() + 4;
-  target->relocateOne(buf + 4, R_AARCH64_JUMP26, s - p);
+  target->relocateWithoutSym(buf + 4, R_AARCH64_JUMP26, s - p);
 }
----------------
grimar wrote:
> Is `relocateNoSym` sounds better?
I'm happy with either, relocateNoSym is a bit shorter.


================
Comment at: lld/ELF/Target.h:88
+  void relocateWithoutSym(uint8_t *loc, RelType type, uint64_t val) const {
+    Relocation rel{};
+    rel.type = type;
----------------
grimar wrote:
> This creates an `R_ABS` relocation, what is probably not ideal.
> 
> What do you think about the following?
> ```
>   void relocateWithoutSym(uint8_t *loc, RelType type, uint64_t val) const {
>     relocate(loc, {R_NONE, type, 0, 0, nullptr}, val);
>   }
> ```
I agree, that R_NONE is better, even if we're unlikely to use it. As an alternative we could move R_NONE to the top of RelExpr and set it explicitly to 0, but I'm happy with the suggestion here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73254