[MachO] Support exports trie in both LC_DYLD_INFO and LC_DYLD_EXPORTS_TRIE

[drodriguez]

Summary

The exports trie used to be pointed by the information in LC_DYLD_INFO,
but when chained fixups are present, the exports trie is pointed by
LC_DYLD_EXPORTS_TRIE instead.

Modify the Object library to give access to the information pointed by
each of the load commands, and to fallback from one into the other when
the exports are requested.

Modify ObjectYAML to support dumping the export trie when pointed by
LC_DYLD_EXPORTS_TRIE and to parse the existence of a export trie also
when the load command is present.

This is a split of D134250 with improvements on top.

[drodriguez]

Can someone have a look at this one? Thanks!

[beanz]

llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp

255 ↗

(On Diff #462600)

Hypothetically, without this assert does the code otherwise function to generate the export trie twice?

The reason I'm asking, is that one of the original goals for the mach-o yaml tooling was to be able to construct invalid object files for testing purposes.

[drodriguez]

llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp

255 ↗

(On Diff #462600)

I think without this assert the export trie information will end up in two places in the binary.

If later a tool like MachOReader from ObjCopy, MachOObjectFile of Object, or macho2yaml tries to read that information, they all prefer the one in the DYLD_INFO vs. the one in EXPORTS_TRIE.

If I understood things correctly what determines whether one or the other exist should be the presence of chained fixups. With chained fixups, the DYLD_INFO does not need to be present, but then you need a place to put the exports trie, so the extra LC is used. Since the chained fixup API is marked as non-final and not to be relied on, I did not want to depend on it too heavily, so I used the presence of the EXPORTS_TRIE as a proxy.

I can remove the assert, but I think it is a nice help that something is being fishy.

[drodriguez]

I am going to try to rewrite the diffs on top of this one to avoid the dependency, but if someone has some opinion about how to do this properly or what can I improve to make it better, I will appreciate it.

[alexander-shaposhnikov]

@drodriguez - if it's not too hard i'd probably split this diff into two. 1. changes to ObjectYAML (it would be good to have a test that invokes obj2yaml and yaml2obj) 2. changes to ObjCopy

[drodriguez]

I will try to split them. Not breaking anything with two separate changes might be complicated.

[drodriguez]

Isolate ObjectYAML and libObject changes from ObjCopy changes. Add test for ObjectYAML changes.

[alexander-shaposhnikov]

LG (with minor nit: for consistency with other tests it'd be good to rename export_trie-lc_dyld_info_only.yaml -> export_trie_lc_dyld_info_only.yaml, export_trie-lc_dyld_exports_trie.yaml -> export_trie_lc_dyld_exports_trie.yaml)

[drodriguez]

Rename test cases

llvm/include/llvm/Object/MachO.h

public:
   MachO::symtab_command getSymtabLoadCommand() const;
   MachO::dysymtab_command getDysymtabLoadCommand() const;
   MachO::linkedit_data_command getDataInCodeLoadCommand() const;
   MachO::linkedit_data_command getLinkOptHintsLoadCommand() const;
   ArrayRef<uint8_t> getDyldInfoRebaseOpcodes() const;
   ArrayRef<uint8_t> getDyldInfoBindOpcodes() const;
   ArrayRef<uint8_t> getDyldInfoWeakBindOpcodes() const;
   ArrayRef<uint8_t> getDyldInfoLazyBindOpcodes() const;
+  ArrayRef<uint8_t> getDyldInfoExportsTrie() const;
+
   /// If the optional is None, no header was found, but the object was
   /// well-formed.
   Expected<Optional<MachO::dyld_chained_fixups_header>>
   getChainedFixupsHeader() const;
   Expected<std::vector<ChainedFixupTarget>> getDyldChainedFixupTargets() const;
 
   // Note: This is a limited, temporary API, which will be removed when Apple
   // upstreams their implementation. Please do not rely on this.
   Expected<Optional<MachO::linkedit_data_command>>
   getChainedFixupsLoadCommand() const;
   // Returns the number of sections listed in dyld_chained_starts_in_image, and
   // a ChainedFixupsSegment for each segment that has fixups.
   Expected<std::pair<size_t, std::vector<ChainedFixupsSegment>>>
   getChainedFixupsSegments() const;
+  ArrayRef<uint8_t> getDyldExportsTrie() const;
 
-  ArrayRef<uint8_t> getDyldInfoExportsTrie() const;
   SmallVector<uint64_t> getFunctionStarts() const;
   ArrayRef<uint8_t> getUuid() const;
 
   StringRef getStringTableData() const;
   bool is64Bit() const;
   void ReadULEB128s(uint64_t Index, SmallVectorImpl<uint64_t> &Out) const;
 
   static StringRef guessLibraryShortName(StringRef Name, bool &isFramework,
private:
   std::unique_ptr<BindRebaseSegInfo> BindRebaseSectionTable;
   const char *SymtabLoadCmd = nullptr;
   const char *DysymtabLoadCmd = nullptr;
   const char *DataInCodeLoadCmd = nullptr;
   const char *LinkOptHintsLoadCmd = nullptr;
   const char *DyldInfoLoadCmd = nullptr;
   const char *FuncStartsLoadCmd = nullptr;
   const char *DyldChainedFixupsLoadCmd = nullptr;
+  const char *DyldExportsTrieLoadCmd = nullptr;
   const char *UuidLoadCmd = nullptr;
   bool HasPageZeroSegment = false;
 };
 
 /// DiceRef
 inline DiceRef::DiceRef(DataRefImpl DiceP, const ObjectFile *Owner)
   : DicePimpl(DiceP) , OwningObject(Owner) {}
 

llvm/lib/Object/MachOObjectFile.cpp

if (Load.C.cmd == MachO::LC_SYMTAB) {
       if ((Err = checkDyldInfoCommand(*this, Load, I, &DyldInfoLoadCmd,
                                       "LC_DYLD_INFO_ONLY", Elements)))
         return;
     } else if (Load.C.cmd == MachO::LC_DYLD_CHAINED_FIXUPS) {
       if ((Err = checkLinkeditDataCommand(
                *this, Load, I, &DyldChainedFixupsLoadCmd,
                "LC_DYLD_CHAINED_FIXUPS", Elements, "chained fixups")))
         return;
+    } else if (Load.C.cmd == MachO::LC_DYLD_EXPORTS_TRIE) {
+      if ((Err = checkLinkeditDataCommand(
+               *this, Load, I, &DyldExportsTrieLoadCmd, "LC_DYLD_EXPORTS_TRIE",
+               Elements, "exports trie")))
+        return;
     } else if (Load.C.cmd == MachO::LC_UUID) {
       if (Load.C.cmdsize != sizeof(MachO::uuid_command)) {
         Err = malformedError("LC_UUID command " + Twine(I) + " has incorrect "
                              "cmdsize");
         return;
       }
       if (UuidLoadCmd) {
         Err = malformedError("more than one LC_UUID command");
MachOObjectFile::exports(Error &E, ArrayRef<uint8_t> Trie,
 
   ExportEntry Finish(&E, O, Trie);
   Finish.moveToEnd();
 
   return make_range(export_iterator(Start), export_iterator(Finish));
 }
 
 iterator_range<export_iterator> MachOObjectFile::exports(Error &Err) const {
-  return exports(Err, getDyldInfoExportsTrie(), this);
+  ArrayRef<uint8_t> Trie;
+  if (DyldInfoLoadCmd)
+    Trie = getDyldInfoExportsTrie();
+  else if (DyldExportsTrieLoadCmd)
+    Trie = getDyldExportsTrie();
+
+  return exports(Err, Trie, this);
 }
 
 MachOAbstractFixupEntry::MachOAbstractFixupEntry(Error *E,
                                                  const MachOObjectFile *O)
     : E(E), O(O) {
   // Cache the vmaddress of __TEXT
   for (const auto &Command : O->load_commands()) {
     if (Command.C.cmd == MachO::LC_SEGMENT) {
ArrayRef<uint8_t> MachOObjectFile::getDyldInfoLazyBindOpcodes() const {
   if (!DyldInfoOrErr)
     return None;
   MachO::dyld_info_command DyldInfo = DyldInfoOrErr.get();
   const uint8_t *Ptr =
       reinterpret_cast<const uint8_t *>(getPtr(*this, DyldInfo.lazy_bind_off));
   return makeArrayRef(Ptr, DyldInfo.lazy_bind_size);
 }
 
+ArrayRef<uint8_t> MachOObjectFile::getDyldInfoExportsTrie() const {
+  if (!DyldInfoLoadCmd)
+    return None;
+
+  auto DyldInfoOrErr =
+      getStructOrErr<MachO::dyld_info_command>(*this, DyldInfoLoadCmd);
+  if (!DyldInfoOrErr)
+    return None;
+  MachO::dyld_info_command DyldInfo = DyldInfoOrErr.get();
+  const uint8_t *Ptr =
+      reinterpret_cast<const uint8_t *>(getPtr(*this, DyldInfo.export_off));
+  return makeArrayRef(Ptr, DyldInfo.export_size);
+}
+
 Expected<Optional<MachO::linkedit_data_command>>
 MachOObjectFile::getChainedFixupsLoadCommand() const {
   // Load the dyld chained fixups load command.
   if (!DyldChainedFixupsLoadCmd)
     return llvm::None;
   auto DyldChainedFixupsOrErr = getStructOrErr<MachO::linkedit_data_command>(
       *this, DyldChainedFixupsLoadCmd);
   if (!DyldChainedFixupsOrErr)
if (Str >= SymbolsEnd)
                             Twine(NameOffset) + " extends past end " +
                             Twine(DyldChainedFixups.datasize));
     Targets.emplace_back(LibOrdinal, NameOffset, Str, Addend, WeakImport);
   }
 
   return std::move(Targets);
 }
 
-ArrayRef<uint8_t> MachOObjectFile::getDyldInfoExportsTrie() const {
-  if (!DyldInfoLoadCmd)
+ArrayRef<uint8_t> MachOObjectFile::getDyldExportsTrie() const {
+  if (!DyldExportsTrieLoadCmd)
     return None;
 
-  auto DyldInfoOrErr =
-    getStructOrErr<MachO::dyld_info_command>(*this, DyldInfoLoadCmd);
-  if (!DyldInfoOrErr)
+  auto DyldExportsTrieOrError = getStructOrErr<MachO::linkedit_data_command>(
+      *this, DyldExportsTrieLoadCmd);
+  if (!DyldExportsTrieOrError)
     return None;
-  MachO::dyld_info_command DyldInfo = DyldInfoOrErr.get();
+  MachO::linkedit_data_command DyldExportsTrie = DyldExportsTrieOrError.get();
   const uint8_t *Ptr =
-      reinterpret_cast<const uint8_t *>(getPtr(*this, DyldInfo.export_off));
-  return makeArrayRef(Ptr, DyldInfo.export_size);
+      reinterpret_cast<const uint8_t *>(getPtr(*this, DyldExportsTrie.dataoff));
+  return makeArrayRef(Ptr, DyldExportsTrie.datasize);
 }
 
 SmallVector<uint64_t> MachOObjectFile::getFunctionStarts() const {
   if (!FuncStartsLoadCmd)
     return {};
 
   auto InfoOrErr =
       getStructOrErr<MachO::linkedit_data_command>(*this, FuncStartsLoadCmd);

llvm/lib/ObjectYAML/MachOEmitter.cpp

private:
   void writeWeakBindOpcodes(raw_ostream &OS);
   void writeLazyBindOpcodes(raw_ostream &OS);
   void writeNameList(raw_ostream &OS);
   void writeStringTable(raw_ostream &OS);
   void writeExportTrie(raw_ostream &OS);
   void writeDynamicSymbolTable(raw_ostream &OS);
   void writeFunctionStarts(raw_ostream &OS);
   void writeChainedFixups(raw_ostream &OS);
+  void writeDyldExportsTrie(raw_ostream &OS);
   void writeDataInCode(raw_ostream &OS);
 
   void dumpExportEntry(raw_ostream &OS, MachOYAML::ExportEntry &Entry);
   void ZeroToOffset(raw_ostream &OS, size_t offset);
 
   MachOYAML::Object &Obj;
   bool is64Bit;
   uint64_t fileStart;
void MachOWriter::writeLinkEditData(raw_ostream &OS) {
   typedef std::pair<uint64_t, writeHandler> writeOperation;
   std::vector<writeOperation> WriteQueue;
 
   MachO::dyld_info_command *DyldInfoOnlyCmd = nullptr;
   MachO::symtab_command *SymtabCmd = nullptr;
   MachO::dysymtab_command *DSymtabCmd = nullptr;
   MachO::linkedit_data_command *FunctionStartsCmd = nullptr;
   MachO::linkedit_data_command *ChainedFixupsCmd = nullptr;
+  MachO::linkedit_data_command *DyldExportsTrieCmd = nullptr;
   MachO::linkedit_data_command *DataInCodeCmd = nullptr;
   for (auto &LC : Obj.LoadCommands) {
     switch (LC.Data.load_command_data.cmd) {
     case MachO::LC_SYMTAB:
       SymtabCmd = &LC.Data.symtab_command_data;
       WriteQueue.push_back(
           std::make_pair(SymtabCmd->symoff, &MachOWriter::writeNameList));
       WriteQueue.push_back(
case MachO::LC_FUNCTION_STARTS:
       WriteQueue.push_back(std::make_pair(FunctionStartsCmd->dataoff,
                                           &MachOWriter::writeFunctionStarts));
       break;
     case MachO::LC_DYLD_CHAINED_FIXUPS:
       ChainedFixupsCmd = &LC.Data.linkedit_data_command_data;
       WriteQueue.push_back(std::make_pair(ChainedFixupsCmd->dataoff,
                                           &MachOWriter::writeChainedFixups));
       break;
+    case MachO::LC_DYLD_EXPORTS_TRIE:
+      DyldExportsTrieCmd = &LC.Data.linkedit_data_command_data;
+      WriteQueue.push_back(std::make_pair(DyldExportsTrieCmd->dataoff,
+                                          &MachOWriter::writeDyldExportsTrie));
+      break;
     case MachO::LC_DATA_IN_CODE:
       DataInCodeCmd = &LC.Data.linkedit_data_command_data;
       WriteQueue.push_back(std::make_pair(DataInCodeCmd->dataoff,
                                           &MachOWriter::writeDataInCode));
       break;
     }
   }
 

 }
 
 void MachOWriter::writeChainedFixups(raw_ostream &OS) {
   if (Obj.LinkEdit.ChainedFixups.size() > 0)
     OS.write(reinterpret_cast<const char *>(Obj.LinkEdit.ChainedFixups.data()),
              Obj.LinkEdit.ChainedFixups.size());
 }
 
+void MachOWriter::writeDyldExportsTrie(raw_ostream &OS) {
+  dumpExportEntry(OS, Obj.LinkEdit.ExportTrie);
+}
+
 class UniversalWriter {
 public:
   UniversalWriter(yaml::YamlObjectFile &ObjectFile)
       : ObjectFile(ObjectFile), fileStart(0) {}
 
   Error writeMachO(raw_ostream &OS);
 
 private:

llvm/test/ObjectYAML/MachO/export_trie_lc_dyld_exports_trie.yaml

 # RUN: yaml2obj %s -o=%t
 # RUN: obj2yaml %t | FileCheck %s
 # RUN: llvm-objdump --macho --exports-trie %t | FileCheck %s --check-prefix=OBJDUMP-VERIFY
 
 --- !mach-o
-FileHeader:      
+FileHeader:
   magic:           0xFEEDFACF
   cputype:         0x01000007
   cpusubtype:      0x80000003
   filetype:        0x00000002
   ncmds:           16
   sizeofcmds:      1408
   flags:           0x00218085
   reserved:        0x00000000
-LoadCommands:    
+LoadCommands:
   - cmd:             LC_SEGMENT_64
     cmdsize:         72
     segname:         __PAGEZERO
     vmaddr:          0
     vmsize:          4294967296
     fileoff:         0
     filesize:        0
     maxprot:         0
- cmd:             LC_SEGMENT_64
     nsects:          3
     flags:           0
   - cmd:             LC_SEGMENT_64
     cmdsize:         72
     segname:         __LINKEDIT
     vmaddr:          4294979584
     vmsize:          4096
     fileoff:         12288
-    filesize:        2508
+    filesize:        1884
     maxprot:         7
     initprot:        1
     nsects:          0
     flags:           0
-  - cmd:             LC_DYLD_INFO_ONLY
-    cmdsize:         48
-    rebase_off:      12288
-    rebase_size:     8
-    bind_off:        12296
-    bind_size:       96
-    weak_bind_off:   0
-    weak_bind_size:  0
-    lazy_bind_off:   12392
-    lazy_bind_size:  624
-    export_off:      13016
-    export_size:     48
+  - cmd:             LC_DYLD_CHAINED_FIXUPS
+    cmdsize:         16
+    dataoff:         12288
+    datasize:        104
+  - cmd:             LC_DYLD_EXPORTS_TRIE
+    cmdsize:         16
+    dataoff:         12392
+    datasize:        48
   - cmd:             LC_SYMTAB
     cmdsize:         24
-    symoff:          13080
+    symoff:          12456
     nsyms:           30
-    stroff:          13700
+    stroff:          13076
     strsize:         1096
   - cmd:             LC_DYSYMTAB
     cmdsize:         80
     ilocalsym:       0
     nlocalsym:       9
     iextdefsym:      9
     nextdefsym:      2
     iundefsym:       11
     nundefsym:       19
     tocoff:          0
     ntoc:            0
     modtaboff:       0
     nmodtab:         0
     extrefsymoff:    0
     nextrefsyms:     0
-    indirectsymoff:  13560
+    indirectsymoff:  12936
     nindirectsyms:   35
     extreloff:       0
     nextrel:         0
     locreloff:       0
     nlocrel:         0
   - cmd:             LC_LOAD_DYLINKER
     cmdsize:         32
     name:            12
- cmd:             LC_SOURCE_VERSION
     cmdsize:         16
     version:         0
   - cmd:             LC_MAIN
     cmdsize:         24
     entryoff:        4448
     stacksize:       0
   - cmd:             LC_LOAD_DYLIB
     cmdsize:         48
-    dylib:           
+    dylib:
       name:            24
       timestamp:       2
       current_version: 7864576
       compatibility_version: 65536
     Content:         '/usr/lib/libc++.1.dylib'
     ZeroPadBytes:    1
   - cmd:             LC_LOAD_DYLIB
     cmdsize:         56
-    dylib:           
+    dylib:
       name:            24
       timestamp:       2
       current_version: 80349697
       compatibility_version: 65536
     Content:         /usr/lib/libSystem.B.dylib
     ZeroPadBytes:    6
   - cmd:             LC_FUNCTION_STARTS
     cmdsize:         16
-    dataoff:         13064
+    dataoff:         12440
     datasize:        16
   - cmd:             LC_DATA_IN_CODE
     cmdsize:         16
-    dataoff:         13080
+    dataoff:         12456
     datasize:        0
-LinkEditData:    
-  ExportTrie:      
+LinkEditData:
+  ExportTrie:
     TerminalSize:    0
     NodeOffset:      0
     Name:            ''
     Flags:           0x0000000000000000
     Address:         0x0000000000000000
     Other:           0x0000000000000000
     ImportName:      ''
-    Children:        
+    Children:
       - TerminalSize:    0
         NodeOffset:      5
         Name:            _
         Flags:           0x0000000000000000
         Address:         0x0000000000000000
         Other:           0x0000000000000000
         ImportName:      ''
-        Children:        
+        Children:
           - TerminalSize:    2
             NodeOffset:      33
             Name:            _mh_execute_header
             Flags:           0x0000000000000000
             Address:         0x0000000000000000
             Other:           0x0000000000000000
             ImportName:      ''
           - TerminalSize:    3
             NodeOffset:      37
             Name:            main
             Flags:           0x0000000000000000
             Address:         0x0000000000003FA0
             Other:           0x0000000000000000
             ImportName:      ''
+  ChainedFixups:   [ 0x0, 0x0, 0x0, 0x0, 0x20, 0x0, 0x0, 0x0, 0x50, 0x0,
+                     0x0, 0x0, 0x58, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0,
+                     0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+                     0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+                     0x0, 0x0, 0x0, 0x0, 0x18, 0x0, 0x0, 0x0, 0x0, 0x0,
+                     0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x18, 0x0, 0x0, 0x0,
+                     0x0, 0x10, 0x6, 0x0, 0x0, 0x40, 0x0, 0x0, 0x0, 0x0,
+                     0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0,
+                     0x1, 0x2, 0x0, 0x0, 0xFD, 0xC, 0x0, 0x0, 0x0, 0x5F,
+                     0x66, 0x6F, 0x6F, 0x0, 0x5F, 0x77, 0x65, 0x61, 0x6B,
+                     0x5F, 0x66, 0x6F, 0x6F, 0x0 ]
 ...
 
-#CHECK: ExportTrie:      
+#CHECK: ExportTrie:
 #CHECK:     TerminalSize:    0
 #CHECK:     NodeOffset:      0
 #CHECK:     Name:            ''
-#CHECK:     Children:        
+#CHECK:     Children:
 #CHECK:       - TerminalSize:    0
 #CHECK:         NodeOffset:      5
 #CHECK:         Name:            _
-#CHECK:         Children:        
+#CHECK:         Children:
 #CHECK:           - TerminalSize:    2
 #CHECK:             NodeOffset:      33
 #CHECK:             Name:            _mh_execute_header
 #CHECK:             Address:         0x0
 #CHECK:           - TerminalSize:    3
 #CHECK:             NodeOffset:      37
 #CHECK:             Name:            main
 #CHECK:             Address:         0x3FA0
 
 # OBJDUMP-VERIFY: Exports trie:
 # OBJDUMP-VERIFY: 0x100000000  __mh_execute_header
 # OBJDUMP-VERIFY: 0x100003FA0  _main

llvm/tools/obj2yaml/macho2yaml.cpp

const uint8_t *processExportNode(const uint8_t *CurrPtr,
   for (auto &Child : Entry.Children) {
     CurrPtr = processExportNode(CurrPtr, End, Child);
   }
   return CurrPtr;
 }
 
 void MachODumper::dumpExportTrie(std::unique_ptr<MachOYAML::Object> &Y) {
   MachOYAML::LinkEditData &LEData = Y->LinkEdit;
+  // The exports trie can be in LC_DYLD_INFO or LC_DYLD_EXPORTS_TRIE
   auto ExportsTrie = Obj.getDyldInfoExportsTrie();
+  if (ExportsTrie.empty())
+    ExportsTrie = Obj.getDyldExportsTrie();
   processExportNode(ExportsTrie.begin(), ExportsTrie.end(), LEData.ExportTrie);
 }
 
 template <typename nlist_t>
 MachOYAML::NListEntry constructNameList(const nlist_t &nlist) {
   MachOYAML::NListEntry NL;
   NL.n_strx = nlist.n_strx;
   NL.n_type = nlist.n_type;