[2/3] [LLD] [COFF] Add support for a new, mingw specific embedded directive -exclude-symbols:

[mstorsjo]

Summary

This is an entirely new embedded directive - extending the GNU ld
command line option --exclude-symbols to be usable in embedded
directives too.

(GNU ld.bfd doesn't support this option as an embedded directive yet,
but when given an object file with such a directive, GNU ld doesn't
error out - contrary to lld - but only warns about it. I'll send a
patch to binutils to add support for the same option there too; this
patch pretty much needs to be coordinated with whatever review feedback
it gets there too.)

This works as an inverse to the regular embedded dllexport directives,
for cases when autoexport of all eligible symbols is performed.

Naming wise, the plural form "exclude-symbols" feels a bit odd here
as it would probably only ever be used with one symbol at a time (I'm
only adding testcases for one symbol per option here), but I'd rather
have it named the same as the command line option, rather than vaguely
different.

[mstorsjo]

The corresponding binutils patch is up for review at https://sourceware.org/pipermail/binutils/2022-July/121902.html.

[mati865]

Even though the changes are small I not sure if approval from an outsider (me) should suffice here.

lld/test/COFF/exclude-symbols-embedded.s

25–26

Maybe one of these lines should list multiple symbols to cover that case?

[mstorsjo]

lld/test/COFF/exclude-symbols-embedded.s

25–26

Sure, I can extend the test to cover that too. I was a little on the fence regarding whether we should allow multiple symbols in the object files (there’s probably little reason to ever use it), but the binutils review seems to point towards keeping that aspect too, so let’s formalize it in the test.

[mstorsjo]

Test multiple symbols with one directive

[mstorsjo]

In D130120#3665910, @mati865 wrote:

Even though the changes are small I not sure if approval from an outsider (me) should suffice here.

Ping @rnk - what do you think of this feature? (I’ll try to sync this patch with the same feature for binutils, but I want some more eyes on the concept itself.)

[mstorsjo]

FWIW this feature has now been OK'd and applied on the binutils side - which should make this patch a bit simpler to reason about.

[rnk]

I think it's a reasonable feature.

lld/COFF/Driver.h

54–57

It's not great that "excludes" is literally the opposite of "includes" when it is meant to be the opposite of exports, but of course we should use the name that ld.bfd went with.

[mstorsjo]

lld/COFF/Driver.h

54–57

Yeah - also the directive name exclude-symbols feels a bit odd to have as plural, but I preferred to keep it matching the the existing command line option (existing in ld.bfd).

[MaskRay]

lld/COFF/Driver.h

163

Prefer llvm::DenseSet<StringRef>.

StringSet<> stores strings and is therefore more expensive. The values are backed by saver().

lld/test/COFF/exclude-symbols-embedded.s

6

See my comment in another patch that grep Name: can be removed.

[mstorsjo]

Simplify the test with --implicit-check-not=Name:, use DenseSet<StringRef> as suggested.

lld/COFF/Driver.h

 extern COFFOptTable optTable;
 
 // The result of parsing the .drective section. The /export: and /include:
 // options are handled separately because they reference symbols, and the number
 // of symbols can be quite large. The LLVM Option library will perform at least
 // one memory allocation per argument, and that is prohibitively slow for
 // parsing directives.
 struct ParsedDirectives {
   std::vector<StringRef> exports;
   std::vector<StringRef> includes;
+  std::vector<StringRef> excludes;
   llvm::opt::InputArgList args;
 };
 
 class ArgParser {
 public:
   // Parses command line options.
   llvm::opt::InputArgList parse(llvm::ArrayRef<const char *> args);
 
   // Tokenizes a given string and then parses as command line options.
private:
   void enqueueTask(std::function<void()> task);
   bool run();
 
   std::list<std::function<void()>> taskQueue;
   std::vector<StringRef> filePaths;
   std::vector<MemoryBufferRef> resources;
 
   llvm::DenseSet<StringRef> directivesExports;
+  llvm::DenseSet<StringRef> excludedSymbols;
 
   COFFLinkerContext &ctx;
 
   llvm::ToolsetLayout vsLayout = llvm::ToolsetLayout::OlderVS;
   std::string vcToolChainPath;
   llvm::SmallString<128> diaPath;
   bool useWinSysRootLibPath = false;
   llvm::SmallString<128> universalCRTLibPath;

lld/COFF/Driver.cpp

for (StringRef e : directives.exports) {
     exp.directives = true;
     config->exports.push_back(exp);
   }
 
   // Handle /include: in bulk.
   for (StringRef inc : directives.includes)
     addUndefined(inc);
 
+  // Handle /exclude-symbols: in bulk.
+  for (StringRef e : directives.excludes) {
+    SmallVector<StringRef, 2> vec;
+    e.split(vec, ',');
+    for (StringRef sym : vec)
+      excludedSymbols.insert(mangle(sym));
+  }
+
   // https://docs.microsoft.com/en-us/cpp/preprocessor/comment-c-cpp?view=msvc-160
   for (auto *arg : directives.args) {
     switch (arg->getOption().getID()) {
     case OPT_aligncomm:
       parseAligncomm(arg->getValue());
       break;
     case OPT_alternatename:
       parseAlternateName(arg->getValue());
if (!config->dll)
       return;
 
     if (!config->exports.empty())
       return;
     if (args.hasArg(OPT_exclude_all_symbols))
       return;
   }
 
-  AutoExporter exporter;
+  AutoExporter exporter(excludedSymbols);
 
   for (auto *arg : args.filtered(OPT_wholearchive_file))
     if (Optional<StringRef> path = doFindFile(arg->getValue()))
       exporter.addWholeArchive(*path);
 
   for (auto *arg : args.filtered(OPT_exclude_symbols)) {
     SmallVector<StringRef, 2> vec;
     StringRef(arg->getValue()).split(vec, ',');

lld/COFF/DriverUtils.cpp

ParsedDirectives ArgParser::parseDirectives(StringRef s) {
   cl::TokenizeWindowsCommandLineNoCopy(s, saver(), tokens);
   for (StringRef tok : tokens) {
     if (tok.startswith_insensitive("/export:") ||
         tok.startswith_insensitive("-export:"))
       result.exports.push_back(tok.substr(strlen("/export:")));
     else if (tok.startswith_insensitive("/include:") ||
              tok.startswith_insensitive("-include:"))
       result.includes.push_back(tok.substr(strlen("/include:")));
+    else if (tok.startswith_insensitive("/exclude-symbols:") ||
+             tok.startswith_insensitive("-exclude-symbols:"))
+      result.excludes.push_back(tok.substr(strlen("/exclude-symbols:")));
     else {
       // Copy substrings that are not valid C strings. The tokenizer may have
       // already copied quoted arguments for us, so those do not need to be
       // copied again.
       bool HasNul = tok.end() != s.end() && tok.data()[tok.size()] == '\0';
       rest.push_back(HasNul ? tok.data() : saver().save(tok).data());
     }
   }

lld/COFF/MinGW.h

 //===- MinGW.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_COFF_MINGW_H
 #define LLD_COFF_MINGW_H
 
 #include "Config.h"
 #include "Symbols.h"
 #include "lld/Common/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Option/ArgList.h"
 #include <vector>
 
 namespace lld::coff {
 class COFFLinkerContext;
 
 // Logic for deciding what symbols to export, when exporting all
 // symbols for MinGW.
 class AutoExporter {
 public:
-  AutoExporter();
+  AutoExporter(const llvm::DenseSet<StringRef> &manualExcludeSymbols);
 
   void addWholeArchive(StringRef path);
   void addExcludedSymbol(StringRef symbol);
 
   llvm::StringSet<> excludeSymbols;
   llvm::StringSet<> excludeSymbolPrefixes;
   llvm::StringSet<> excludeSymbolSuffixes;
   llvm::StringSet<> excludeLibs;
   llvm::StringSet<> excludeObjects;
 
+  const llvm::DenseSet<StringRef> &manualExcludeSymbols;
+
   bool shouldExport(const COFFLinkerContext &ctx, Defined *sym) const;
 };
 
 void writeDefFile(StringRef name);
 
 // The -wrap option is a feature to rename symbols so that you can write
 // wrappers for existing functions. If you pass `-wrap:foo`, all
 // occurrences of symbol `foo` are resolved to `__wrap_foo` (so, you are

lld/COFF/MinGW.cpp

 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::COFF;
 using namespace lld;
 using namespace lld::coff;
 
-AutoExporter::AutoExporter() {
+AutoExporter::AutoExporter(
+    const llvm::DenseSet<StringRef> &manualExcludeSymbols)
+    : manualExcludeSymbols(manualExcludeSymbols) {
   excludeLibs = {
       "libgcc",
       "libgcc_s",
       "libstdc++",
       "libmingw32",
       "libmingwex",
       "libg2c",
       "libsupc++",
bool AutoExporter::shouldExport(const COFFLinkerContext &ctx,
                                 Defined *sym) const {
   if (!sym || !sym->getChunk())
     return false;
 
   // Only allow the symbol kinds that make sense to export; in particular,
   // disallow import symbols.
   if (!isa<DefinedRegular>(sym) && !isa<DefinedCommon>(sym))
     return false;
-  if (excludeSymbols.count(sym->getName()))
+  if (excludeSymbols.count(sym->getName()) || manualExcludeSymbols.count(sym->getName()))
     return false;
 
   for (StringRef prefix : excludeSymbolPrefixes.keys())
     if (sym->getName().startswith(prefix))
       return false;
   for (StringRef suffix : excludeSymbolSuffixes.keys())
     if (sym->getName().endswith(suffix))
       return false;

lld/docs/ReleaseNotes.rst

 * ...
 
 MinGW Improvements
 ------------------
 
 * The ``--exclude-symbols`` option is now supported.
   (`D130118 <https://reviews.llvm.org/D130118>`_)
 
+* Support for an entirely new object file directive, ``-exclude-symbols:``,
+  has been implemented. (`D130120 <https://reviews.llvm.org/D130120>`_)
+
 MachO Improvements
 ------------------
 
 WebAssembly Improvements
 ------------------------
 

lld/test/COFF/exclude-symbols-embedded.s

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=i686-win32-gnu %s -o %t.o
+
+// RUN: lld-link -lldmingw -dll -out:%t.dll %t.o -noentry
+// RUN: llvm-readobj --coff-exports %t.dll | FileCheck --implicit-check-not=Name: %s
+
+// CHECK: Name:
+// CHECK: Name: sym1
+
+.global _sym1
+_sym1:
+  ret
+
+.global _sym2
+_sym2:
+  ret
+
+.global _sym3
+_sym3:
+  ret
+
+.section .drectve,"yn"
+.ascii " -exclude-symbols:sym2,unknownsym"
+.ascii " -exclude-symbols:unkonwnsym,sym3"