[lld-macho] Accept dylibs with LC_DYLD_EXPORTS_TRIE

[BertalanD]

Summary

This load command specifies the offset and size of the exports trie.
This information used to be a field in LC_DYLD_INFO, but in newer
libraries, it has a dedicated load command: LC_DYLD_EXPORTS_TRIE.

The format of the trie is the same for both load commands, so the code
for parsing it can be shared.

LLD does not generate this yet; it is mainly useful when chained fixups
are in use, as the other members of LC_DYLD_INFO are unused then, so the
smaller LC_DYLD_EXPORTS_TRIE can be output instead.

LLDB gained support for this in D107673.

Fixes #54550

[BertalanD]

Removed unused variable

[int3]

Nice! Could we have a test? We can use yaml2obj to generate the test input.

[BertalanD]

What should that test look like? Do I just check if lld returns 0, or does the output file need to be checked as well?

[BertalanD]

It looks like obj2yaml only dumps the exports trie if it sees LC_DYLD_INFO_ONLY. I'm going to submit a patch for that first then...

[int3]

Checking that LLD returns zero when looking up a symbol in a dylib using LC_DYLD_EXPORTS_TRIE should be good

[BertalanD]

Added a test case (generated using obj2yaml --raw-segment=linkedit as obj2yaml does not automatically dump the trie when it sees LC_DYLD_EXPORTS_TRIE)

[thakis]

Nice!

lld/MachO/InputFiles.cpp

1727–1736

Should we error out if both LC_DYLD_INFO_ONLY and LC_DYLD_EXPORTS_TRIE are present? (Does ld64?)

lld/test/MachO/lc-dyld-exports-trie.yaml

12 ↗

(On Diff #444241)

Can you add a comment that explains how you created this file?

[BertalanD]

lld/MachO/InputFiles.cpp

1727–1736

It's not clear to me. The LLDB diff I linked asserts (!) that only one of them can be present; ld64 checks for LC_DYLD_INFO_ONLY first, and only reads the new load command if the former is not specified, while dyld does it the other way around.

[thakis]

Let's add an error then and see if anyone complains.

[int3]

lgtm thanks!

lld/test/MachO/lc-dyld-exports-trie.yaml

3 ↗

(On Diff #444388)

[thakis]

Lgtm too.

lld/MachO/InputFiles.h

public:
   bool isBundleLoader;
 
 private:
   bool handleLDSymbol(StringRef originalName);
   void handleLDPreviousSymbol(StringRef name, StringRef originalName);
   void handleLDInstallNameSymbol(StringRef name, StringRef originalName);
   void handleLDHideSymbol(StringRef name, StringRef originalName);
   void checkAppExtensionSafety(bool dylibIsAppExtensionSafe) const;
+  void parseExportedSymbols(uint32_t offset, uint32_t size);
 
   llvm::DenseSet<llvm::CachedHashStringRef> hiddenSymbols;
 };
 
 // .a file
 class ArchiveFile final : public InputFile {
 public:
   explicit ArchiveFile(std::unique_ptr<llvm::object::Archive> &&file);

lld/MachO/InputFiles.cpp

for (auto *cmd : findCommands<rpath_command>(hdr, LC_RPATH)) {
     StringRef rpath{reinterpret_cast<const char *>(cmd) + cmd->path};
     rpaths.push_back(rpath);
   }
 
   // Initialize symbols.
   exportingFile = isImplicitlyLinked(installName) ? this : this->umbrella;
   if (const load_command *cmd = findCommand(hdr, LC_DYLD_INFO_ONLY)) {
     auto *c = reinterpret_cast<const dyld_info_command *>(cmd);
-    struct TrieEntry {
-      StringRef name;
-      uint64_t flags;
-    };
+    parseExportedSymbols(c->export_off, c->export_size);
+  } else if (const load_command *cmd = findCommand(hdr, LC_DYLD_EXPORTS_TRIE)) {
+    auto *c = reinterpret_cast<const linkedit_data_command *>(cmd);
+    parseExportedSymbols(c->dataoff, c->datasize);
+  } else {
+    error("No LC_DYLD_INFO_ONLY or LC_DYLD_EXPORTS_TRIE found in " +
+          toString(this));
+    return;
+  }
+}
+
+void DylibFile::parseExportedSymbols(uint32_t offset, uint32_t size) {
 
-    std::vector<TrieEntry> entries;
-    // Find all the $ld$* symbols to process first.
-    parseTrie(buf + c->export_off, c->export_size,
-              [&](const Twine &name, uint64_t flags) {
-                StringRef savedName = saver().save(name);
-                if (handleLDSymbol(savedName))
-                  return;
-                entries.push_back({savedName, flags});
-              });
+  auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
+  parseTrie(buf + offset, size, [&](const Twine &name, uint64_t flags) {
 
-    // Process the "normal" symbols.
-    for (TrieEntry &entry : entries) {
-      if (exportingFile->hiddenSymbols.contains(
-              CachedHashStringRef(entry.name)))
-        continue;
+    if (exportingFile->hiddenSymbols.contains(CachedHashStringRef(entry.name)))
 
-      bool isWeakDef = entry.flags & EXPORT_SYMBOL_FLAGS_WEAK_DEFINITION;
-      bool isTlv = entry.flags & EXPORT_SYMBOL_FLAGS_KIND_THREAD_LOCAL;
 
-      symbols.push_back(
-          symtab->addDylib(entry.name, exportingFile, isWeakDef, isTlv));
-    }
-
-  } else {
-    error("LC_DYLD_INFO_ONLY not found in " + toString(this));
-    return;
-  }
 }
 
 void DylibFile::parseLoadCommands(MemoryBufferRef mb) {
   auto *hdr = reinterpret_cast<const mach_header *>(mb.getBufferStart());
   const uint8_t *p = reinterpret_cast<const uint8_t *>(mb.getBufferStart()) +
                      target->headerSize;
   for (uint32_t i = 0, n = hdr->ncmds; i < n; ++i) {
     auto *cmd = reinterpret_cast<const load_command *>(p);