[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");
+
----------------
BertalanD wrote:
> 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.
of course I meant `emitFixupChains`. note to self: don't respond to code review at 11 PM.


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

https://reviews.llvm.org/D132560