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

[Nuri Amari via Phabricator via llvm-commits]

nuriamari added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp:455
+  uint32_t TextSegmentFileSize = 0;
+  if (O.TextSegmentCommandIndex) {
+    const LoadCommand &TextSegmentLoadCommand =
----------------
alexander-shaposhnikov wrote:
> 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)
1/ Behaviour is the same as far as I can tell, checks for __TEXT segment [[ https://github.com/llvm/llvm-project/blob/main/lld/MachO/SyntheticSections.cpp#L1237 | here ]] and if the segment isn't found, [[ https://github.com/llvm/llvm-project/blob/main/lld/MachO/OutputSegment.cpp#L178 | creates an "empty" OutputSegment ]], which has [[ https://github.com/llvm/llvm-project/blob/main/lld/MachO/OutputSegment.h#L51 | fileoff and filesize values of 0 ]]. 

2/ No other usages, I will move the helpers here. I don't think getting stale values is an issue since the calls are made after the `MachOLayoutBuilder` [[ https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp#L272-L288 | updates the values ]]. 


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:361
   LinkData FunctionStarts;
-  LinkData CodeSignature;
+  CodeSignatureInfo CodeSignature;
 
----------------
alexander-shaposhnikov wrote:
> maybe it'd be better to make it a part of MachOLayoutBuilder ?
> (since it appears to be used only there)
It is also used in the `MachOWriter`, but I suppose the `Writer` can reach into the `LayoutBuilder` for that information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111164