[PATCH] D132560: [lld-macho] Add initial support for chained fixups

[Daniel Bertalan via Phabricator via llvm-commits]

BertalanD added inline comments.


================
Comment at: lld/MachO/Driver.cpp:1478
+  if (config->makeChainedFixups && args.hasArg(OPT_no_pie))
+    error("-fixup_chains is incompatible with -no_pie");
+
----------------
thakis wrote:
> Emit a diag if makeChainedFixups is true but the deployment target (`config->platformInfo.minimum`) is too old to support them.
> 
> Below are no action required:
> 
> - Idea guying, not for this patch (and likely never): What does dyld do if a binary has both bind opcodes and chained fixups? Does it use the opcodes on older macOS (or iOS or…) versions but the fixups on newer versions?
> 
> - nit: I'd s/makeChainedFixups/emitFixupChains/ (to be closer to the flag name), but meh. Oh, you made it match the section, not the flag. Even more meh then, but s/make/emit/ in the variable still seems marginally better to me.
> Emit a diag if makeChainedFixups is true but the deployment target (config->platformInfo.minimum) is too old to support them.

Will do!

> Idea guying, not for this patch (and likely never): What does dyld do if a binary has both bind opcodes and chained fixups? Does it use the opcodes on older macOS (or iOS or…) versions but the fixups on newer versions?

That would be an awesome hack. We could *maybe* make chained fixups' new binding behavior (always eager, uses the GOT instead of LazyPointerSection) work with bind opcodes, but it still wouldn't work with an older dyld.  `LC_DYLD_CHAINED_FIXUPS` is defined as `0x34 | LC_REQ_DYLD`, which  means dyld will refuse to load the file with a "load command unknown" message if it does not know about it.

> nit: I'd s/makeChainedFixups/emitFixupChains/ (to be closer to the flag name), but meh. Oh, you made it match the section, not the flag. Even more meh then, but s/make/emit/ in the variable still seems marginally better to me.

`makeChainedFixups` it is then. The double "fixup chains" and "chained fixups" naming is not ideal, but oh well. Apple's user-facing documentation uses the latter, that's what we should do as well IMO.


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

https://reviews.llvm.org/D132560