[lld-macho] implement options -(un)exported_symbol(s_list)

[gkm]

Summary

Implement command-line options to alter a dylib's exported-symbols list:

• -exported_symbol* options override the default export list. The export list is compiled according to the command-line option(s) only.
• -unexported_symbol* options hide otherwise public symbols.
• -*exported_symbol PATTERN options specify a single literal or glob pattern.
• -*exported_symbols_list FILE options specify a file containing a series of lines containing symbol literals or glob patterns. Whitespace and #-prefix comments are stripped.

Note: This is a simple implementation of the primary use case. ld64 has much more complexity surrounding interactions with other options, many of which are obscure and undocumented. We will start simple and complexity as necessary.

[gkm]

I need to check vs. ld64 behavior before this is ready to review.

[gkm]

• Make the -export_symbol* options properly override the default public/private status of all symbols.

[oontvoo]

lld/MachO/Driver.cpp

728

nit: for completeness, consider adding ] too?
(if symbolName had ] without other stuff, it's likely malformed and it would be good to error out)

[gkm]

• revise according to review feedback

[int3]

lld/MachO/Config.h

54–55

Hmm, GlobPattern.cpp has a special case for patterns without wildcards, but I guess having a separate literals set is still faster with the O(1) lookup

Since we're optimizing for speed here, I would use CachedHashStringRef for the literals set

lld/MachO/Driver.cpp

741

avoid auto

lld/test/MachO/export-options.s

12–13

the lack of comment character works because split-file means it isn't passed into llvm-mc, but we should still make it a comment for uniformity

17–21

I'm not sure I see much value in these macros... you're not really saving that many characters in exchange for an additional level of indirection

Without macros, you could save on the retqs by doing

_keep_globl:
_hide_globl:
_keep_private:
_show_private:
  retq

just my 2c, keep the macros if you like them

[gkm]

lld/MachO/Config.h

54–55

Yes, the separate literals set is for performance. I will comment it as such.

lld/MachO/Driver.cpp

741

11 of 37 range-for loops in lld/MachO/Driver.cpp use auto. I will cleanup with a separate [NFC] diff.

lld/test/MachO/export-options.s

17–21

My primary goals were to (a) minimize vertical space, and (b) eliminate the need to type the symbol name twice--once for the scope directive, and again for the label. Placing all of the labels at the same address doesn't help me with the latter.

[gkm]

• revise according to review feedback

[gkm]

• use CachedHashStringRef for SymbolPatterns::literals

[thakis]

lld/MachO/SyntheticSections.cpp

611

This looks very branch and nested. How about:

static bool shouldExportSymbol(const Defined& defined) {
  if (!config->exportedSymbols.empty())
    return  config->exportedSymbols.match(defined->getName());

  return defined->privateExtern ||
            config->unexportedSymbols.match(defined->getName());
}

...

     if (!shouldExportSymbol(*defined))
       continue;
...

? This would also help with the issue below:

611

*very branchY and nested

833

You have to make a similar change here like you made in finalizeContents(), else the symbol table (what nm -m) won't match the export trie.

In general, tests that test --exports-trie output (or --bind output) should always also test nm -m output to make sure they're in sync.

[gkm]

@thakis, see D98381 [lld-macho] Handle error cases properly for -exported_symbol(s_list)

lld/MachO/Config.h

 //===- Config.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_MACHO_CONFIG_H
 #define LLD_MACHO_CONFIG_H
 
+#include "llvm/ADT/CachedHashString.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/Support/GlobPattern.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/TextAPI/MachO/Architecture.h"
 #include "llvm/TextAPI/MachO/Platform.h"
 #include "llvm/TextAPI/MachO/Target.h"
 
 #include <vector>
 
 namespace lld {

 enum class UndefinedSymbolTreatment {
   unknown,
   error,
   warning,
   suppress,
   dynamic_lookup,
 };
 
+class SymbolPatterns {
+  // GlobPattern can also match literals,
+  // but we prefer the O(1) lookup of DenseSet.
+  llvm::DenseSet<llvm::CachedHashStringRef> literals;
+  std::vector<llvm::GlobPattern> globs;
+
+public:
+  bool empty() const { return literals.empty() && globs.empty(); }
+  void clear();
+  void insert(llvm::StringRef symbolName);
+  bool matchLiteral(llvm::StringRef symbolName) const;
+  bool matchGlob(llvm::StringRef symbolName) const;
+  bool match(llvm::StringRef symbolName) const;
+};
+
 struct Configuration {
   Symbol *entry;
   bool hasReexports = false;
   bool allLoad = false;
   bool forceLoadObjC = false;
   bool staticLink = false;
   bool implicitDylibs = false;
   bool isPic = false;
struct Configuration {
   UndefinedSymbolTreatment undefinedSymbolTreatment =
       UndefinedSymbolTreatment::error;
   llvm::MachO::HeaderFileType outputType;
   std::vector<llvm::StringRef> systemLibraryRoots;
   std::vector<llvm::StringRef> librarySearchPaths;
   std::vector<llvm::StringRef> frameworkSearchPaths;
   std::vector<llvm::StringRef> runtimePaths;
   std::vector<Symbol *> explicitUndefineds;
+
   llvm::DenseMap<llvm::StringRef, SymbolPriorityEntry> priorities;
   SectionRenameMap sectionRenameMap;
   SegmentRenameMap segmentRenameMap;
+
+  SymbolPatterns exportedSymbols;
+  SymbolPatterns unexportedSymbols;
 };
 
 // The symbol with the highest priority should be ordered first in the output
 // section (modulo input section contiguity constraints). Using priority
 // (highest first) instead of order (lowest first) has the convenient property
 // that the default-constructed zero priority -- for symbols/sections without a
 // user-defined order -- naturally ends up putting them at the end of the
 // output.

lld/MachO/Driver.cpp

static uint32_t parseDylibVersion(const opt::ArgList& args, unsigned id) {
   if (!version.parse32(arg->getValue())) {
     error(arg->getAsString(args) + ": malformed version");
     return 0;
   }
 
   return version.rawValue();
 }
 
+void SymbolPatterns::clear() {
+  literals.clear();
+  globs.clear();
+}
+
+void SymbolPatterns::insert(StringRef symbolName) {
+  if (symbolName.find_first_of("*?[]") == StringRef::npos)
+    literals.insert(CachedHashStringRef(symbolName));
+  else if (Expected<GlobPattern> pattern = GlobPattern::create(symbolName))
+    globs.emplace_back(*pattern);
+  else
+    error("invalid symbol-name pattern: " + symbolName);
+}
+
+bool SymbolPatterns::matchLiteral(StringRef symbolName) const {
+  return literals.contains(CachedHashStringRef(symbolName));
+}
+
+bool SymbolPatterns::matchGlob(StringRef symbolName) const {
+  for (const llvm::GlobPattern &glob : globs)
+    if (glob.match(symbolName))
+      return true;
+  return false;
+}
+
+bool SymbolPatterns::match(StringRef symbolName) const {
+  return matchLiteral(symbolName) || matchGlob(symbolName);
+}
+
+static void handleSymbolPatterns(opt::InputArgList &args,
+                                 SymbolPatterns &symbolPatterns,
+                                 unsigned singleOptionCode,
+                                 unsigned listFileOptionCode) {
+  for (opt::Arg *arg : args.filtered(singleOptionCode))
+    symbolPatterns.insert(arg->getValue());
+  for (opt::Arg *arg : args.filtered(listFileOptionCode)) {
+    StringRef path = arg->getValue();
+    Optional<MemoryBufferRef> buffer = readFile(path);
+    if (!buffer) {
+      error("Could not read symbol file: " + path);
+      continue;
+    }
+    MemoryBufferRef mbref = *buffer;
+    for (StringRef line : args::getLines(mbref)) {
+      line = line.take_until([](char c) { return c == '#'; }).trim();
+      if (!line.empty())
+        symbolPatterns.insert(line);
+    }
+  }
+}
+
 bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
                  raw_ostream &stdoutOS, raw_ostream &stderrOS) {
   lld::stdoutOS = &stdoutOS;
   lld::stderrOS = &stderrOS;
 
   errorHandler().cleanupCallback = []() { freeArena(); };
 
   errorHandler().logName = args::getFilenameWithoutExe(argsArr[0]);
config->sectionRenameMap[{validName(arg->getValue(0)),
                               validName(arg->getValue(1))}] = {
         validName(arg->getValue(2)), validName(arg->getValue(3))};
   }
   for (opt::Arg *arg : args.filtered(OPT_rename_segment)) {
     config->segmentRenameMap[validName(arg->getValue(0))] =
         validName(arg->getValue(1));
   }
 
+  handleSymbolPatterns(args, config->exportedSymbols, OPT_exported_symbol,
+                       OPT_exported_symbols_list);
+  handleSymbolPatterns(args, config->unexportedSymbols, OPT_unexported_symbol,
+                       OPT_unexported_symbols_list);
+  if (!config->exportedSymbols.empty() && !config->unexportedSymbols.empty()) {
+    error("cannot use both -exported_symbol* and -unexported_symbol* options\n"
+          ">>> ignoring unexports");
+    config->unexportedSymbols.clear();
+  }
+
   config->saveTemps = args.hasArg(OPT_save_temps);
 
   config->adhocCodesign = args.hasFlag(
       OPT_adhoc_codesign, OPT_no_adhoc_codesign,
       config->target.Arch == AK_arm64 || config->target.Arch == AK_arm64e);
 
   if (args.hasArg(OPT_v)) {
     message(getLLDVersion());

lld/MachO/Options.td

def keep_private_externs : Flag<["-"], "keep_private_externs">,
      Group<grp_object>;
 def d : Flag<["-"], "d">,
      HelpText<"Force tentative into real definitions for common symbols">,
      Flags<[HelpHidden]>,
      Group<grp_object>;
 
 def grp_resolve : OptionGroup<"resolve">, HelpText<"SYMBOL RESOLUTION">;
 
-def exported_symbols_list : Separate<["-"], "exported_symbols_list">,
-     MetaVarName<"<file>">,
-     HelpText<"Symbols specified in <file> remain global, while others become private externs">,
-     Flags<[HelpHidden]>,
-     Group<grp_resolve>;
 def exported_symbol : Separate<["-"], "exported_symbol">,
      MetaVarName<"<symbol>">,
      HelpText<"<symbol> remains global, while others become private externs">,
-     Flags<[HelpHidden]>,
      Group<grp_resolve>;
-def unexported_symbols_list : Separate<["-"], "unexported_symbols_list">,
+def exported_symbols_list : Separate<["-"], "exported_symbols_list">,
      MetaVarName<"<file>">,
-     HelpText<"Global symbols specified in <file> become private externs">,
-     Flags<[HelpHidden]>,
+     HelpText<"Symbols specified in <file> remain global, while others become private externs">,
      Group<grp_resolve>;
 def unexported_symbol : Separate<["-"], "unexported_symbol">,
      MetaVarName<"<symbol>">,
      HelpText<"Global <symbol> becomes private extern">,
-     Flags<[HelpHidden]>,
+     Group<grp_resolve>;
+def unexported_symbols_list : Separate<["-"], "unexported_symbols_list">,
+     MetaVarName<"<file>">,
+     HelpText<"Global symbols specified in <file> become private externs">,
      Group<grp_resolve>;
 def reexported_symbols_list : Separate<["-"], "reexported_symbols_list">,
      MetaVarName<"<file>">,
      HelpText<"Symbols from dependent dylibs specified in <file> are reexported by this dylib">,
      Flags<[HelpHidden]>,
      Group<grp_resolve>;
 def alias : MultiArg<["-"], "alias", 2>,
      MetaVarName<"<symbol_name> <alternate_name>">,

lld/MachO/SyntheticSections.cpp

 
 ExportSection::ExportSection()
     : LinkEditSection(segment_names::linkEdit, section_names::export_) {}
 
 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)
+      if (config->exportedSymbols.empty()) {
+        if (defined->privateExtern ||
+            config->unexportedSymbols.match(defined->getName()))
           continue;
+      } else {
+        if (!config->exportedSymbols.match(defined->getName()))
+          continue;
+      }
       trieBuilder.addSymbol(*defined);
       hasWeakSymbol = hasWeakSymbol || sym->isWeakDef();
     }
   }
   size = trieBuilder.build();
 }
 
 void ExportSection::writeTo(uint8_t *buf) const { trieBuilder.writeTo(buf); }
void SymtabSection::writeTo(uint8_t *buf) const {
   }
 
   for (const SymtabEntry &entry : concat<const SymtabEntry>(
            localSymbols, externalSymbols, undefinedSymbols)) {
     nList->n_strx = entry.strx;
     // TODO populate n_desc with more flags
     if (auto *defined = dyn_cast<Defined>(entry.sym)) {
       uint8_t scope = 0;
       if (defined->privateExtern) {
         // Private external -- dylib scoped symbol.
         // Promote to non-external at link time.
         assert(defined->isExternal() && "invalid input file");
         scope = MachO::N_PEXT;
       } else if (defined->isExternal()) {
         // Normal global symbol.
         scope = MachO::N_EXT;
       } else {

lld/test/MachO/export-options.s

+# REQUIRES: x86
+
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %t/default.s -o %t/default.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
+
+# CONFLICT: error: cannot use both -exported_symbol* and -unexported_symbol* options
+# CONFLICT-NEXT: >>> ignoring unexports
+
+#--- default.s
+
+.macro DEFSYM, type, sym
+\type \sym
+\sym:
+  retq
+.endm
+
+DEFSYM .globl, _keep_globl
+DEFSYM .globl, _hide_globl
+DEFSYM .private_extern, _keep_private
+DEFSYM .private_extern, _show_private
+
+## Check that the export trie is unaltered
+# RUN: %lld -dylib %t/default.o -o %t/default
+# RUN: llvm-objdump --macho --exports-trie %t/default | \
+# RUN:     FileCheck --check-prefix=DEFAULT %s
+
+# DEFAULT-LABEL: Exports trie:
+# DEFAULT-DAG:   _hide_globl
+# DEFAULT-DAG:   _keep_globl
+# DEFAULT-NOT:   _hide_private
+# DEFAULT-NOT:   _show_private
+
+## Check that the export trie is properly augmented
+## Check that non-matching literal pattern has no effect
+# RUN: %lld -dylib %t/default.o -o %t/export \
+# RUN:     -exported_symbol _show_private \
+# RUN:     -exported_symbol _extra_cruft -exported_symbol '*xtra_cr?ft'
+# RUN: llvm-objdump --macho --exports-trie %t/export | \
+# RUN:     FileCheck --check-prefix=EXPORTED %s
+
+# EXPORTED-LABEL: Exports trie:
+# EXPORTED-DAG:   _show_private
+# EXPORTED-NOT:   _hide_globl
+# EXPORTED-NOT:   _keep_globl
+# EXPORTED-NOT:   _hide_private
+# EXPORTED-NOT:   {{.*}}xtra_cr{{.}}ft
+
+## Check that the export trie is properly diminished
+## Check that non-matching glob pattern has no effect
+# RUN: %lld -dylib %t/default.o -o %t/unexport \
+# RUN:     -unexported_symbol _hide_global
+# RUN: llvm-objdump --macho --exports-trie %t/unexport | \
+# RUN:     FileCheck --check-prefix=UNEXPORTED %s
+
+# UNEXPORTED-LABEL: Exports trie:
+# UNEXPORTED-DAG:   _keep_globl
+# UNEXPORTED-NOT:   _hide_globl
+# UNEXPORTED-NOT:   _show_private
+# UNEXPORTED-NOT:   _hide_private
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/symdefs.s -o %t/symdefs.o
+
+#--- symdefs.s
+
+.macro DEFSYM, sym
+.private_extern \sym
+\sym:
+  retq
+.endm
+
+DEFSYM literal_only
+DEFSYM literal_also
+DEFSYM globby_only
+DEFSYM globby_also
+
+#--- literals
+
+  literal_only # comment
+  literal_also
+
+# globby_only
+  globby_also
+
+## Check that only string-literal patterns match
+## Check that comments and blank lines are stripped from symbol list
+# RUN: %lld -dylib %t/symdefs.o -o %t/literal \
+# RUN:     -exported_symbols_list %t/literals
+# RUN: llvm-objdump --macho --exports-trie %t/literal | \
+# RUN:     FileCheck --check-prefix=LITERAL %s
+
+# LITERAL-DAG: literal_only
+# LITERAL-DAG: literal_also
+# LITERAL-DAG: globby_also
+# LITERAL-NOT: globby_only
+
+#--- globbys
+
+# literal_only
+  l?ter[aeiou]l_*[^y] # comment
+
+  *gl?bby_*
+
+## Check that only glob patterns match
+## Check that comments and blank lines are stripped from symbol list
+# RUN: %lld -dylib %t/symdefs.o -o %t/globby \
+# RUN:     -exported_symbols_list %t/globbys
+# RUN: llvm-objdump --macho --exports-trie %t/globby | \
+# RUN:     FileCheck --check-prefix=GLOBBY %s
+
+# GLOBBY-DAG: literal_also
+# GLOBBY-DAG: globby_only
+# GLOBBY-DAG: globby_also
+# GLOBBY-NOT: literal_only