[PATCH] D133974: [objcopy] Fix order of Mach-O LINKEDIT pieces during layout

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

drodriguez created this revision.
drodriguez added reviewers: jhenderson, MaskRay.
Herald added subscribers: StephenFan, abrachet, hiraditya.
Herald added a reviewer: ributzka.
Herald added a project: All.
drodriguez requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The exports trie and the chained fixups where in the opposite order, and
function starts happenned before them, instead of after them.

Restore the correct order and rewrite the code to make it easier to move
around in the future if needed by reusing the `Offset` variable and
keeping both the `StartOf...` and the size of each piece together.

This was found out while trying to use the system strip in a binary
already stripped by LLVM and receiving errors around chained fixups when
we enabled those in the linker.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133974

Files:
  llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp


Index: llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
===================================================================
--- llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
+++ llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
@@ -235,37 +235,44 @@
          "Incorrect tail offset");
   Offset = std::max(Offset, HeaderSize + O.Header.SizeOfCmds);
 
-  // The order of LINKEDIT elements is as follows:
-  // rebase info, binding info, weak binding info, lazy binding info, export
-  // trie, data-in-code, symbol table, indirect symbol table, symbol table
-  // strings, code signature.
   uint64_t NListSize = Is64Bit ? sizeof(MachO::nlist_64) : sizeof(MachO::nlist);
   uint64_t StartOfLinkEdit = Offset;
-  uint64_t StartOfRebaseInfo = StartOfLinkEdit;
-  uint64_t StartOfBindingInfo = StartOfRebaseInfo + O.Rebases.Opcodes.size();
-  uint64_t StartOfWeakBindingInfo = StartOfBindingInfo + O.Binds.Opcodes.size();
+
+  // The order of LINKEDIT elements is as follows:
+  // rebase info, binding info, weak binding info, lazy binding info, export
+  // trie, chained fixups, dyld exports trie, function starts, data-in-code,
+  // symbol table, indirect symbol table, symbol table strings,
+  // and code signature.
+  auto linkEditUpdateOffset = [&Offset](uint32_t size) {
+    uint64_t previousOffset = Offset;
+    Offset += size;
+    return previousOffset;
+  };
+
+  uint64_t StartOfRebaseInfo = linkEditUpdateOffset(O.Rebases.Opcodes.size());
+  uint64_t StartOfBindingInfo = linkEditUpdateOffset(O.Binds.Opcodes.size());
+  uint64_t StartOfWeakBindingInfo =
+      linkEditUpdateOffset(O.WeakBinds.Opcodes.size());
   uint64_t StartOfLazyBindingInfo =
-      StartOfWeakBindingInfo + O.WeakBinds.Opcodes.size();
-  uint64_t StartOfExportTrie =
-      StartOfLazyBindingInfo + O.LazyBinds.Opcodes.size();
-  uint64_t StartOfFunctionStarts = StartOfExportTrie + O.Exports.Trie.size();
-  uint64_t StartOfDyldExportsTrie =
-      StartOfFunctionStarts + O.FunctionStarts.Data.size();
+      linkEditUpdateOffset(O.LazyBinds.Opcodes.size());
+  uint64_t StartOfExportTrie = linkEditUpdateOffset(O.Exports.Trie.size());
   uint64_t StartOfChainedFixups =
-      StartOfDyldExportsTrie + O.ExportsTrie.Data.size();
-  uint64_t StartOfDataInCode =
-      StartOfChainedFixups + O.ChainedFixups.Data.size();
+      linkEditUpdateOffset(O.ChainedFixups.Data.size());
+  uint64_t StartOfDyldExportsTrie =
+      linkEditUpdateOffset(O.ExportsTrie.Data.size());
+  uint64_t StartOfFunctionStarts =
+      linkEditUpdateOffset(O.FunctionStarts.Data.size());
+  uint64_t StartOfDataInCode = linkEditUpdateOffset(O.DataInCode.Data.size());
   uint64_t StartOfLinkerOptimizationHint =
-      StartOfDataInCode + O.DataInCode.Data.size();
+      linkEditUpdateOffset(O.LinkerOptimizationHint.Data.size());
   uint64_t StartOfSymbols =
-      StartOfLinkerOptimizationHint + O.LinkerOptimizationHint.Data.size();
-  uint64_t StartOfIndirectSymbols =
-      StartOfSymbols + NListSize * O.SymTable.Symbols.size();
+      linkEditUpdateOffset(NListSize * O.SymTable.Symbols.size());
+  uint64_t StartOfIndirectSymbols = linkEditUpdateOffset(
+      sizeof(uint32_t) * O.IndirectSymTable.Symbols.size());
   uint64_t StartOfSymbolStrings =
-      StartOfIndirectSymbols +
-      sizeof(uint32_t) * O.IndirectSymTable.Symbols.size();
-  uint64_t StartOfCodeSignature =
-      StartOfSymbolStrings + StrTableBuilder.getSize();
+      linkEditUpdateOffset(StrTableBuilder.getSize());
+
+  uint64_t StartOfCodeSignature = Offset;
   uint32_t CodeSignatureSize = 0;
   if (O.CodeSignatureCommandIndex) {
     StartOfCodeSignature = alignTo(StartOfCodeSignature, 16);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133974.460501.patch
Type: text/x-patch
Size: 3647 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220915/0a22b9ee/attachment.bin>