[PATCH] D88064: [lld-macho] handle option -headerpad_max_install_names

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Writer.cpp:371
+    size_t installNamesPadSize =
+        (dylibCount + ((config->outputType == MH_DYLIB) ? 1 : 0)) * 1024;
+    if (config->headerPad < installNamesPadSize)
----------------
nit: make 1024 a named constant

nit 2: I assume we are checking for `MH_DYLIB` here because the `LC_DYLIB_ID` command also contains an install name. Would be good to have a comment.

Also -- don't we need to allocate space for the `LC_REEXPORT_DYLIB` commands as well?


================
Comment at: lld/MachO/Writer.cpp:372-373
+        (dylibCount + ((config->outputType == MH_DYLIB) ? 1 : 0)) * 1024;
+    if (config->headerPad < installNamesPadSize)
+      config->headerPad = installNamesPadSize;
+  }
----------------
would be nice to have a test where the `-headerpad` argument is higher than the max install names size


================
Comment at: lld/test/MachO/headerpad.s:56
+# PADMAX1:      magic        cputype  cpusubtype  caps    filetype ncmds sizeofcmds               flags
+# PADMAX1-NEXT: MH_MAGIC_64  X86_64   ALL         LIB64   EXECUTE  10     [[#%u, CMDSIZE:]] {{.*}}
+# PADMAX1:      sectname __text
----------------
ultra nit: delete one space for alignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88064