[lld-macho] Support re-exports of individual symbols

[int3]

Summary

Specifically, we support this:

ld64.lld -dylib foo.o libbar.dylib -exported_symbol _bar -o libfoo.dylib

Where _bar is defined in libbar.dylib.

[smeenai]

It's interesting that ld64 overloads -exported_symbol for this purpose instead of having an explicit -reexported_symbol analog to -reexported_symbols_list. We should follow suit though, of course.

I'll leave the comment about handleExplicitExports to your judgment. LGTM otherwise.

lld/MachO/SyntheticSections.cpp

982–983

What do you think of moving this logic to handleExplicitExports in Driver.cpp and setting a flag on the DylibSymbol that's checked here instead? I ask because that one is parallelized, which presumably means that provided a measurable speedup in some builds, plus it seems conceptually nicer to handle the checking in one place.

[smeenai]

Looks like ld64 actually has the same behavior for -exported_symbols_list as well (so they're consistent on that front). I believe we'll do the same already, but it's probably worth adding a test to be explicit.

[int3]

I don't really think a test for that is super necessary, the -exported_symbols_list is simply command-line sugar over -exported_symbol, and I don't think every variation of how -exported_symbol gets used needs to have a corresponding _list test...

lld/MachO/SyntheticSections.cpp

982–983

ooh, that's definitely cleaner. Thanks!

[smeenai]

In D144153#4225351, @int3 wrote:

I don't really think a test for that is super necessary, the -exported_symbols_list is simply command-line sugar over -exported_symbol, and I don't think every variation of how -exported_symbol gets used needs to have a corresponding _list test...

Fair enough. My logic was that I personally found it surprising that -exported_symbols_list performs re-exporting even though there's also -reexported_symbols_list and thought it might be useful to be explicit about that in a test. On the other hand, like you said, since -exported_symbols_list is equivalent to -exported_symbol, it does make sense that they'd behave identically in this aspect too (and would be really weird if it didn't). Not an issue either way.

[int3]

move logic to handleExplicitExports; also add test to show -unexported_symbol has no effect on dylibs

lld/MachO/Driver.cpp

for (const InputFile *file : inputFiles) {
     }
   }
 }
 
 static void handleExplicitExports() {
   if (config->hasExplicitExports) {
     parallelForEach(symtab->getSymbols(), [](Symbol *sym) {
       if (auto *defined = dyn_cast<Defined>(sym)) {
-        StringRef symbolName = defined->getName();
-        if (config->exportedSymbols.match(symbolName)) {
+        if (config->exportedSymbols.match(sym->getName())) {
           if (defined->privateExtern) {
             if (defined->weakDefCanBeHidden) {
               // weak_def_can_be_hidden symbols behave similarly to
               // private_extern symbols in most cases, except for when
               // it is explicitly exported.
               // The former can be exported but the latter cannot.
               defined->privateExtern = false;
             } else {
               warn("cannot export hidden symbol " + toString(*defined) +
                    "\n>>> defined in " + toString(defined->getFile()));
             }
           }
         } else {
           defined->privateExtern = true;
         }
+      } else if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
+        dysym->shouldReexport = config->exportedSymbols.match(sym->getName());
       }
     });
   } else if (!config->unexportedSymbols.empty()) {
     parallelForEach(symtab->getSymbols(), [](Symbol *sym) {
       if (auto *defined = dyn_cast<Defined>(sym))
         if (config->unexportedSymbols.match(defined->getName()))
           defined->privateExtern = true;
     });

lld/MachO/ExportTrie.cpp

struct Edge {
   Edge(StringRef s, TrieNode *node) : substring(s), child(node) {}
 
   StringRef substring;
   struct TrieNode *child;
 };
 
 struct ExportInfo {
   uint64_t address;
+  uint64_t ordinal = 0;
   uint8_t flags = 0;
   ExportInfo(const Symbol &sym, uint64_t imageBase)
       : address(sym.getVA() - imageBase) {
     using namespace llvm::MachO;
-    // Set the symbol type.
     if (sym.isWeakDef())
       flags |= EXPORT_SYMBOL_FLAGS_WEAK_DEFINITION;
-    // TODO: Add proper support for re-exports & stub-and-resolver flags.
-
-    // Set the symbol kind.
-    if (sym.isTlv()) {
+    if (sym.isTlv())
       flags |= EXPORT_SYMBOL_FLAGS_KIND_THREAD_LOCAL;
-    } else if (auto *defined = dyn_cast<Defined>(&sym)) {
+    // TODO: Add proper support for stub-and-resolver flags.
+
+    if (auto *defined = dyn_cast<Defined>(&sym)) {
       if (defined->isAbsolute())
         flags |= EXPORT_SYMBOL_FLAGS_KIND_ABSOLUTE;
+    } else if (auto *dysym = dyn_cast<DylibSymbol>(&sym)) {
+      flags |= EXPORT_SYMBOL_FLAGS_REEXPORT;
+      if (!dysym->isDynamicLookup())
+        ordinal = dysym->getFile()->ordinal;
     }
   }
 };
 
 } // namespace
 
 struct macho::TrieNode {
   std::vector<Edge> edges;
   std::optional<ExportInfo> info;
   // Estimated offset from the start of the serialized trie to the current node.
   // This will converge to the true offset when updateOffset() is run to a
   // fixpoint.
   size_t offset = 0;
 
+  uint32_t getTerminalSize() const;
   // Returns whether the new estimated offset differs from the old one.
   bool updateOffset(size_t &nextOffset);
   void writeTo(uint8_t *buf) const;
 };
 
+// For regular symbols, the node layout (excluding the children) is
+//
+//   uleb128 terminalSize;
+//   uleb128 flags;
+//   uleb128 address;
+//
+// For re-exported symbols, the layout is
+//
+//   uleb128 terminalSize;
+//   uleb128 flags;
+//   uleb128 ordinal;
+//   char[] originalName;
+//
+// If libfoo.dylib is linked against libbar.dylib, and libfoo exports an alias
+// _foo to a symbol _bar in libbar, then originalName will be "_bar". If libfoo
+// re-exports _bar directly (i.e. not via an alias), then originalName will be
+// the empty string.
+//
+// TODO: Support aliased re-exports. (Since we don't yet support these,
+// originalName will always be the empty string.)
+//
+// For stub-and-resolver nodes, the layout is
+//
+//   uleb128 terminalSize;
+//   uleb128 flags;
+//   uleb128 stubAddress;
+//   uleb128 resolverAddress;
+//
+// TODO: Support stub-and-resolver nodes.
+uint32_t TrieNode::getTerminalSize() const {
+  uint32_t size = getULEB128Size(info->flags);
+  if (info->flags & MachO::EXPORT_SYMBOL_FLAGS_REEXPORT)
+    size += getULEB128Size(info->ordinal) + 1; // + 1 for the null-terminator
+  else
+    size += getULEB128Size(info->address);
+  return size;
+}
+
 bool TrieNode::updateOffset(size_t &nextOffset) {
   // Size of the whole node (including the terminalSize and the outgoing edges.)
   // In contrast, terminalSize only records the size of the other data in the
   // node.
   size_t nodeSize;
   if (info) {
-    uint32_t terminalSize =
-        getULEB128Size(info->flags) + getULEB128Size(info->address);
+    uint32_t terminalSize = getTerminalSize();
     // Overall node size so far is the uleb128 size of the length of the symbol
     // info + the symbol info itself.
     nodeSize = terminalSize + getULEB128Size(terminalSize);
   } else {
     nodeSize = 1; // Size of terminalSize (which has a value of 0)
   }
   // Compute size of all child edges.
   ++nodeSize; // Byte for number of children.
   for (const Edge &edge : edges) {
     nodeSize += edge.substring.size() + 1             // String length.
                 + getULEB128Size(edge.child->offset); // Offset len.
   }
   // On input, 'nextOffset' is the new preferred location for this node.
   bool result = (offset != nextOffset);
   // Store new location in node object for use by parents.
   offset = nextOffset;
   nextOffset += nodeSize;
   return result;
 }
 
 void TrieNode::writeTo(uint8_t *buf) const {
   buf += offset;
   if (info) {
-    // TrieNodes with Symbol info: size, flags address
-    uint32_t terminalSize =
-        getULEB128Size(info->flags) + getULEB128Size(info->address);
+    uint32_t terminalSize = getTerminalSize();
     buf += encodeULEB128(terminalSize, buf);
     buf += encodeULEB128(info->flags, buf);
+    if (info->flags & MachO::EXPORT_SYMBOL_FLAGS_REEXPORT) {
+      buf += encodeULEB128(info->ordinal, buf);
+      *buf++ = 0; // empty originalName string
+    } else {
       buf += encodeULEB128(info->address, buf);
+    }
   } else {
     // TrieNode with no Symbol info.
     *buf++ = 0; // terminalSize
   }
   // Add number of children. TODO: Handle case where we have more than 256.
   assert(edges.size() < 256);
   *buf++ = edges.size();
   // Append each child edge substring and node offset.

lld/MachO/Symbols.h

public:
   const uint32_t align;
   const bool privateExtern;
 };
 
 class DylibSymbol : public Symbol {
 public:
   DylibSymbol(DylibFile *file, StringRefZ name, bool isWeakDef,
               RefState refState, bool isTlv)
-      : Symbol(DylibKind, name, file), refState(refState), weakDef(isWeakDef),
-        tlv(isTlv) {
+      : Symbol(DylibKind, name, file), shouldReexport(false),
+        refState(refState), weakDef(isWeakDef), tlv(isTlv) {
     if (file && refState > RefState::Unreferenced)
       file->numReferencedSymbols++;
   }
 
   uint64_t getVA() const override;
   bool isWeakDef() const override { return weakDef; }
 
   // Symbols from weak libraries/frameworks are also weakly-referenced.
public:
   void unreference() {
     // dynamic_lookup symbols have no file.
     if (refState > RefState::Unreferenced && file) {
       assert(getFile()->numReferencedSymbols > 0);
       getFile()->numReferencedSymbols--;
     }
   }
 
+  bool shouldReexport : 1;
 private:
   RefState refState : 2;
   const bool weakDef : 1;
   const bool tlv : 1;
 };
 
 class LazyArchive : public Symbol {
 public:

lld/MachO/SyntheticSections.cpp

 void ExportSection::finalizeContents() {
   trieBuilder.setImageBase(in.header->addr);
   for (const Symbol *sym : symtab->getSymbols()) {
     if (const auto *defined = dyn_cast<Defined>(sym)) {
       if (defined->privateExtern || !defined->isLive())
         continue;
       trieBuilder.addSymbol(*defined);
       hasWeakSymbol = hasWeakSymbol || sym->isWeakDef();
+    } else if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
+      if (dysym->shouldReexport)
+        trieBuilder.addSymbol(*dysym);
     }
   }
   size = trieBuilder.build();
 }
 
 void ExportSection::writeTo(uint8_t *buf) const { trieBuilder.writeTo(buf); }
 
 DataInCodeSection::DataInCodeSection()

lld/test/MachO/export-options.s

 # REQUIRES: x86
 
 # RUN: rm -rf %t; split-file %s %t
+# RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/empty.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %t/default.s -o %t/default.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %t/lazydef.s -o %t/lazydef.o
 # RUN: llvm-ar --format=darwin rcs %t/lazydef.a %t/lazydef.o
 
 ## Check that mixing exported and unexported symbol options yields an error
 # RUN: not %lld -dylib %t/default.o -o /dev/null \
 # RUN:         -exported_symbol a -unexported_symbol b 2>&1 | \
 # RUN:     FileCheck --check-prefix=CONFLICT %s

 # RUN: %lld -dylib %t/symdefs.o -o %t/noexports -no_exported_symbols
 # RUN: llvm-objdump --macho --exports-trie %t/noexports | FileCheck --check-prefix=NOEXPORTS %s
 
 # NOEXPORTS-NOT: globby_also
 # NOEXPORTS-NOT: globby_only
 # NOEXPORTS-NOT: literal_also
 # NOEXPORTS-NOT: literal_only
 
+# RUN: %lld -dylib %t/default.o -o %t/libdefault.dylib
+# RUN: %lld -dylib %t/empty.o %t/libdefault.dylib -exported_symbol _keep_globl \
+# RUN:   -exported_symbol _undef -exported_symbol _tlv \
+# RUN:   -undefined dynamic_lookup -o %t/reexport-dylib
+# RUN: llvm-objdump --macho --exports-trie %t/reexport-dylib
+
+# REEXPORT:      Exports trie:
+# REEXPORT-NEXT: [re-export] _tlv [per-thread] (from libdefault)
+# REEXPORT-NEXT: [re-export] _keep_globl (from libdefault)
+# REEXPORT-NEXT: [re-export] _undef (from unknown)
+
+## -unexported_symbol will not make us re-export symbols in dylibs.
+# RUN: %lld -dylib %t/default.o -o %t/libdefault.dylib
+# RUN: %lld -dylib %t/empty.o %t/libdefault.dylib -unexported_symbol _tlv \
+# RUN:   -o %t/unexport-dylib
+# RUN: llvm-objdump --macho --exports-trie %t/unexport-dylib | FileCheck %s \
+# RUN:   --check-prefix=EMPTY-TRIE
+
 #--- default.s
 
-.globl _keep_globl, _hide_globl
+.globl _keep_globl, _hide_globl, _tlv
 _keep_globl:
   retq
 _hide_globl:
   retq
 .private_extern _private_extern
 _private_extern:
   retq
 _private:
   retq
 
+.section __DATA,__thread_vars,thread_local_variables
+_tlv:
+
 #--- lazydef.s
 
 .globl _keep_lazy
 _keep_lazy:
   retq
 
 #--- symdefs.s