[PATCH] D80855: [lld-macho] Support non-pcrel section relocs

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:196
+      uint32_t targetOffset;
+      if (rel.r_pcrel)
+        // The implicit addend for pcrel section relocations is the pcrel offset
----------------
smeenai wrote:
> Super nit: even though it's not technically needed, for instances like this where there's a comment within an `if` (even though there's only a single actual statement), I prefer putting braces around it (and the `else`), to prevent any possible confusion.
I prefer that too, just wasn't sure what the prevailing style was supposed to be :) 


================
Comment at: lld/test/MachO/relocations.s:60
+## This generates a non-pcrel section relocation
+  .quad L_.str
----------------
smeenai wrote:
> Are you missing the corresponding CHECK for this? It would also be nice to add a test with a non-zero offset.
> Are you missing the corresponding CHECK for this?

oh yeah good catch

> It would also be nice to add a test with a non-zero offset.

L._str is at a non-zero offset in its section, which I think is the important case to test, but yeah I guess it wouldn't hurt to add another reloc whose source is also at a non-zero offset


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80855