[PATCH] D96164: [lld-macho] add code signature for native arm64 macOS

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

lgtm, will like @cynecx sign off



================
Comment at: lld/MachO/SyntheticSections.cpp:882
+    fileName = fileName.drop_front(slashIndex + 1);
+  fileNameSize = fileName.size();
+  allHeadersSize = alignTo(fixedHeadersSize + fileNameSize + 1, WordSize);
----------------
FWIW, StringRef::size() is O(1), so storing it isn't really necessary. But if you did this because it looks more uniform, that's fine too


================
Comment at: lld/MachO/SyntheticSections.cpp:910-912
+  // The kernel creates a signature-verification cache entry at
+  // mmap(2) time, before the signature exists. Now that the
+  // signature is written, invalidate the bogus cache entry.
----------------
would be good to link to https://openradar.appspot.com/FB8914231


================
Comment at: lld/MachO/SyntheticSections.cpp:917
+void CodeSignatureSection::writeTo(uint8_t *buf) const {
+  using namespace llvm::support::endian;
+  using namespace llvm::MachO;
----------------
I think we already included this namespace at the top of the file


================
Comment at: lld/MachO/SyntheticSections.cpp:943
+  codeDirectory->pageSize = blockSizeShift;
+  codeDirectory->spare2 = 0;
+  codeDirectory->scatterOffset = 0;
----------------
JFYI, there are several other places in the code that assume that the `mmap`'ed buf is initialized to all zeroes. I couldn't find any docs that guarantee that behavior, but it seems to work in practice. That said it doesn't hurt to be explicit


================
Comment at: lld/MachO/Writer.cpp:790-792
+  uint8_t *codeEnd =
+      buffer->getBufferEnd() -
+      (codeSignatureSection ? codeSignatureSection->getSize() : 0);
----------------
is this necessary? I don't think hashing a few extra zero bytes makes a difference


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96164