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

[Daniel Rodríguez Troitiño via Phabricator via llvm-commits]

drodriguez added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:361
   LinkData FunctionStarts;
-  LinkData CodeSignature;
+  CodeSignatureInfo CodeSignature;
 
----------------
alexander-shaposhnikov wrote:
> alexander-shaposhnikov wrote:
> > drodriguez wrote:
> > > nuriamari wrote:
> > > > 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.
> > > @alexander-shaposhnikov: Can we talk again about this? It feels as if the rest of the `LinkData` blobs have their information hold in this `Object` struct. Most of them are simply the data itself, but for other things, like `IndirectSymbolTable`, it seems that they are defined here, and used in the `MachOReader`, the `MachOLayoutBuilder` and the `MachOWriter`. In the case of the `LC_CODE_SIGNATURE`, we don't have any use in `MachOReader` because the information is discarded, but it seems to me that this is the correct place. Additionally, to be usage from `MachOWriter` and `MachOLayoutBuilder`, it needs to end up as a public instance variable in `MachOLayoutBuilder`, making it the first public instance variable of that class.
> > > 
> > > In any case, it is a minimal thing, and I don't need it to be changed. I just wanted to point out the similarities with the code here.
> > the pipeline looks like this: parse the input (the output is Object) -> apply various transformations (to Object) -> write the final output (including the calculations of the new layout). Therefore, storing "offsets" in Object doesn't appear to be right, another indication why it's not right - they are not used (and should not be used (because they can be invalidated by some transformations as I pointed out in another comment)) before emitting the new object file.
> regarding public instance -  unless I'm missing something this won't happen.
> Currently these values are calculated in MachOLayoutBuilder::layoutTail,  it's perfectly fine for layoutTail to have a write access a member of the same class. External customers can use a getter that returns a const ref.
Understood. Thanks for explaining the concepts behind the code flow. This blob size and contents depending on the previous content is a little bit of a problem (specially the total size), and specially the size needed to be included in the `__LINKEDIT` information calculated during the layout.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111164