[PATCH] D118570: [lld-macho] Add support for -image_base, -static, -pagezero_size, and -version_load_command

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

Neat, thanks for implementing all these! As @smeenai said, we definitely need tests :)

Also, could you break up the diff to make review easier? I think each flag merits its own diff



================
Comment at: lld/MachO/Config.h:100
   Symbol *entry = nullptr;
   bool hasReexports = false;
   bool allLoad = false;
----------------
we haven't been super consistent about this, but it would be nice to keep the Configuration struct organized by field type. E.g. `addLoadVersion` could go here


================
Comment at: lld/MachO/Writer.cpp:49
 public:
-  Writer() : buffer(errorHandler().outputBuffer) {}
+  Writer() : buffer(errorHandler().outputBuffer), addr(config->imageBase) {}
 
----------------
from what I understand, `-image_base` should control the address of the mach header, but setting `addr` here means that we're setting the start address of `__PAGEZERO` instead, which doesn't seem right

Also, I wonder what happens when `-image_base` points to an address within the `__PAGEZERO` segment...


================
Comment at: lld/MachO/Writer.cpp:316
+    fatal("Unknown flavor " +
+          llvm::Twine::utohexstr(target->threadCommandFlavor));
+    return 0;
----------------
no need for `llvm::`


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

https://reviews.llvm.org/D118570