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

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Very cool. I haven't really looked at it yet. (I'll play with it once the prerequisites are landed; fewer `patch` invocations then :) )



================
Comment at: lld/MachO/Driver.cpp:1478
+  if (config->makeChainedFixups && args.hasArg(OPT_no_pie))
+    error("-fixup_chains is incompatible with -no_pie");
+
----------------
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.


================
Comment at: lld/MachO/SyntheticSections.h:664
+// first fixup of each page; the rest can be found by walking the chain using
+// the offset that is embedded in each entry.
+//
----------------
Maybe mention that the motivation is that text pages can be relocated lazily at page-in time, and that they don't become dirty and can be discarded and paged in again as needed. That is, the benefits are:

- somewhat faster startup (no need to do all relocs at app startup)
- more memory-efficient, since text pages can be discarded

That motivates why the fixups are per-page and then chained (since that's how they're accessed at page-in time).


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

https://reviews.llvm.org/D132560