[PATCH] D111164: Regenerate LC_CODE_SIGNATURE during llvm-objcopy operations

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexander-shaposhnikov added a comment.

I've added some in-line comments, but the high level approach looks reasonable to me, thanks for working on this!



================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:289
   uint64_t LinkEditSize =
-      (StartOfCodeSignature + O.CodeSignature.Data.size()) - StartOfLinkEdit;
+      StartOfCodeSignature + CodeSignatureSectionSize - StartOfLinkEdit;
 
----------------
nit: maybe drop "Section" from the name ?, i.e. CodeSignatureSection -> CodeSignature, CodeSignatureSectionSize -> CodeSignatueSize (it's a bit shorter + it's not a section per se).


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp:455
+  uint32_t TextSegmentFileSize = 0;
+  if (O.TextSegmentCommandIndex) {
+    const LoadCommand &TextSegmentLoadCommand =
----------------
1/ just in case - what happens if there is no __TEXT in the binary - what does `codesign` do in this case - does it still create something or bail out ?
2/ is there any other place where getSegmentFileOffset / getSegmentFileSize are used ? 
(if no - then probably it'd be better not to store them and just calculate here or have a helper function or method that returns them) (they also can change during some binary transformations e.g. adding / removing load commands) (this might not be observable on small tests because of the alignment)


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:361
   LinkData FunctionStarts;
-  LinkData CodeSignature;
+  CodeSignatureInfo CodeSignature;
 
----------------
maybe it'd be better to make it a part of MachOLayoutBuilder ?
(since it appears to be used only there)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111164