[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/Target.h:90
+    rel.type = type;
+    relocate(loc, rel, val);
+  }
----------------
MaskRay wrote:
> grimar wrote:
> > It is probably looks a bit strange to have such piece here.
> > It does not look to be generic code because initializes only single field.
> > And it seems used only once.
> `relocateOne` is very common.
> 
> ```
> % rg relocateOne lld | wc -l
> 72
> ```
> 
> The intention is to thread `Symbol` through relocation handling (context: https://github.com/ClangBuiltLinux/linux/issues/773)
> 
> For many `relocateOne`, there is probably not a good `Symbol *` to thread through. We can use `nullptr`, but that will requires changes in the call sites. I think it is just simpler to keep the utility here.
I think that there are quite a few callsites that are in files unaffected by this diff or in the context, for example Thunks, PLT sequences. As I understand it they are a way of using the relocateOne logic to avoid duplicating some potentially nasty encoding logic.

This does raise an interesting question though as we don't always have a symbol when doing such a relocation. For example we can make a Relocation, like relocateOne above but we may not be able to fake a symbol in all contexts.

This would mean that this patch would work for rel->type as there would always be a type, but we'd have to be careful about testing that a symbol exists before using it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73254