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

Tue Jun 9 18:48:24 PDT 2020

smeenai requested changes to this revision.
smeenai added a comment.
This revision now requires changes to proceed.

The logic looks good; putting back in your queue for the comment about the test. (Please request review again if I'm just misunderstanding the test.)

================
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
----------------
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.

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80855