Skip to content

[LLD][COFF] Separate EC and native exports for ARM64X #123652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Jan 20, 2025

Store exports in SymbolTable instead of Configuration.

@cjacek
Copy link
Contributor Author

cjacek commented Jan 20, 2025

Depends on #123651.

This is the main refactoring for ARM64X export support. The observable effect in tests is that EC exports are no longer exported from the native view. For the EC view, a separate export table will be needed, with ARM64X relocations modifying it. I plan to submit that next.

@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2025

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld

Author: Jacek Caban (cjacek)

Changes

Store exports in SymbolTable instead of Configuration.


Patch is 31.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123652.diff

11 Files Affected:

  • (modified) lld/COFF/Config.h (-2)
  • (modified) lld/COFF/DLL.cpp (+18-16)
  • (modified) lld/COFF/DLL.h (+2-14)
  • (modified) lld/COFF/Driver.cpp (+32-27)
  • (modified) lld/COFF/Driver.h (-3)
  • (modified) lld/COFF/DriverUtils.cpp (-136)
  • (modified) lld/COFF/MapFile.cpp (+1-1)
  • (modified) lld/COFF/SymbolTable.cpp (+135)
  • (modified) lld/COFF/SymbolTable.h (+8)
  • (modified) lld/COFF/Writer.cpp (+11-10)
  • (added) lld/test/COFF/arm64x-export.test (+123)
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 924560fef0231d..1473705893ab01 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -161,8 +161,6 @@ struct Configuration {
   bool dll = false;
   StringRef implib;
   bool noimplib = false;
-  std::vector<Export> exports;
-  bool hadExplicitExports;
   std::set<std::string> delayLoads;
   std::map<std::string, int> dllOrder;
   Symbol *delayLoadHelper = nullptr;
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 875ada9d605394..6a3f8eb21e8475 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -639,22 +639,22 @@ class ExportDirectoryChunk : public NonSectionChunk {
 
 class AddressTableChunk : public NonSectionChunk {
 public:
-  explicit AddressTableChunk(COFFLinkerContext &ctx, size_t baseOrdinal,
+  explicit AddressTableChunk(SymbolTable &symtab, size_t baseOrdinal,
                              size_t maxOrdinal)
       : baseOrdinal(baseOrdinal), size((maxOrdinal - baseOrdinal) + 1),
-        ctx(ctx) {}
+        symtab(symtab) {}
   size_t getSize() const override { return size * 4; }
 
   void writeTo(uint8_t *buf) const override {
     memset(buf, 0, getSize());
 
-    for (const Export &e : ctx.config.exports) {
+    for (const Export &e : symtab.exports) {
       assert(e.ordinal >= baseOrdinal && "Export symbol has invalid ordinal");
       // Subtract the OrdinalBase to get the index.
       uint8_t *p = buf + (e.ordinal - baseOrdinal) * 4;
       uint32_t bit = 0;
       // Pointer to thumb code must have the LSB set, so adjust it.
-      if (ctx.config.machine == ARMNT && !e.data)
+      if (symtab.machine == ARMNT && !e.data)
         bit = 1;
       if (e.forwardChunk) {
         write32le(p, e.forwardChunk->getRVA() | bit);
@@ -669,7 +669,7 @@ class AddressTableChunk : public NonSectionChunk {
 private:
   size_t baseOrdinal;
   size_t size;
-  const COFFLinkerContext &ctx;
+  const SymbolTable &symtab;
 };
 
 class NamePointersChunk : public NonSectionChunk {
@@ -690,13 +690,13 @@ class NamePointersChunk : public NonSectionChunk {
 
 class ExportOrdinalChunk : public NonSectionChunk {
 public:
-  explicit ExportOrdinalChunk(const COFFLinkerContext &ctx, size_t baseOrdinal,
+  explicit ExportOrdinalChunk(const SymbolTable &symtab, size_t baseOrdinal,
                               size_t tableSize)
-      : baseOrdinal(baseOrdinal), size(tableSize), ctx(ctx) {}
+      : baseOrdinal(baseOrdinal), size(tableSize), symtab(symtab) {}
   size_t getSize() const override { return size * 2; }
 
   void writeTo(uint8_t *buf) const override {
-    for (const Export &e : ctx.config.exports) {
+    for (const Export &e : symtab.exports) {
       if (e.noname)
         continue;
       assert(e.ordinal >= baseOrdinal && "Export symbol has invalid ordinal");
@@ -709,7 +709,7 @@ class ExportOrdinalChunk : public NonSectionChunk {
 private:
   size_t baseOrdinal;
   size_t size;
-  const COFFLinkerContext &ctx;
+  const SymbolTable &symtab;
 };
 
 } // anonymous namespace
@@ -920,9 +920,9 @@ Chunk *DelayLoadContents::newThunkChunk(DefinedImportData *s,
   }
 }
 
-EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
+void createEdataChunks(SymbolTable &symtab, std::vector<Chunk *> &chunks) {
   unsigned baseOrdinal = 1 << 16, maxOrdinal = 0;
-  for (Export &e : ctx.config.exports) {
+  for (Export &e : symtab.exports) {
     baseOrdinal = std::min(baseOrdinal, (unsigned)e.ordinal);
     maxOrdinal = std::max(maxOrdinal, (unsigned)e.ordinal);
   }
@@ -930,15 +930,16 @@ EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
   // https://learn.microsoft.com/en-us/cpp/build/reference/export-exports-a-function?view=msvc-170
   assert(baseOrdinal >= 1);
 
-  auto *dllName = make<StringChunk>(sys::path::filename(ctx.config.outputFile));
-  auto *addressTab = make<AddressTableChunk>(ctx, baseOrdinal, maxOrdinal);
+  auto *dllName =
+      make<StringChunk>(sys::path::filename(symtab.ctx.config.outputFile));
+  auto *addressTab = make<AddressTableChunk>(symtab, baseOrdinal, maxOrdinal);
   std::vector<Chunk *> names;
-  for (Export &e : ctx.config.exports)
+  for (Export &e : symtab.exports)
     if (!e.noname)
       names.push_back(make<StringChunk>(e.exportName));
 
   std::vector<Chunk *> forwards;
-  for (Export &e : ctx.config.exports) {
+  for (Export &e : symtab.exports) {
     if (e.forwardTo.empty())
       continue;
     e.forwardChunk = make<StringChunk>(e.forwardTo);
@@ -946,7 +947,8 @@ EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
   }
 
   auto *nameTab = make<NamePointersChunk>(names);
-  auto *ordinalTab = make<ExportOrdinalChunk>(ctx, baseOrdinal, names.size());
+  auto *ordinalTab =
+      make<ExportOrdinalChunk>(symtab, baseOrdinal, names.size());
   auto *dir =
       make<ExportDirectoryChunk>(baseOrdinal, maxOrdinal, names.size(), dllName,
                                  addressTab, nameTab, ordinalTab);
diff --git a/lld/COFF/DLL.h b/lld/COFF/DLL.h
index f7d2b57a20a020..724a323d62d205 100644
--- a/lld/COFF/DLL.h
+++ b/lld/COFF/DLL.h
@@ -77,20 +77,8 @@ class DelayLoadContents {
   COFFLinkerContext &ctx;
 };
 
-// Windows-specific.
-// EdataContents creates all chunks for the DLL export table.
-class EdataContents {
-public:
-  EdataContents(COFFLinkerContext &ctx);
-  std::vector<Chunk *> chunks;
-
-  uint64_t getRVA() { return chunks[0]->getRVA(); }
-  uint64_t getSize() {
-    return chunks.back()->getRVA() + chunks.back()->getSize() - getRVA();
-  }
-
-  COFFLinkerContext &ctx;
-};
+// Create all chunks for the DLL export table.
+void createEdataChunks(SymbolTable &symtab, std::vector<Chunk *> &chunks);
 
 } // namespace lld::coff
 
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 898c6c17d2062a..eca0af7a3378f4 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -458,7 +458,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
     // declarations, many object files may end up with having the
     // same /EXPORT options. In order to save cost of parsing them,
     // we dedup them first.
-    if (!directivesExports.insert(e).second)
+    if (!file->symtab.directivesExports.insert(e).second)
       continue;
 
     Export exp = parseExport(e);
@@ -469,7 +469,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
         exp.extName = saver().save("_" + exp.extName);
     }
     exp.source = ExportSource::Directives;
-    ctx.config.exports.push_back(exp);
+    file->symtab.exports.push_back(exp);
   }
 
   // Handle /include: in bulk.
@@ -956,7 +956,7 @@ std::string LinkerDriver::getImportName(bool asLib) {
 void LinkerDriver::createImportLibrary(bool asLib) {
   llvm::TimeTraceScope timeScope("Create import library");
   std::vector<COFFShortExport> exports;
-  for (Export &e1 : ctx.config.exports) {
+  for (Export &e1 : ctx.symtab.exports) {
     COFFShortExport e2;
     e2.Name = std::string(e1.name);
     e2.SymbolName = std::string(e1.symbolName);
@@ -1069,7 +1069,7 @@ void LinkerDriver::parseModuleDefs(StringRef path) {
     e2.isPrivate = e1.Private;
     e2.constant = e1.Constant;
     e2.source = ExportSource::ModuleDefinition;
-    ctx.config.exports.push_back(e2);
+    ctx.symtab.exports.push_back(e2);
   }
 }
 
@@ -1222,8 +1222,10 @@ static void findKeepUniqueSections(COFFLinkerContext &ctx) {
 
   // Exported symbols could be address-significant in other executables or DSOs,
   // so we conservatively mark them as address-significant.
-  for (Export &r : ctx.config.exports)
-    markAddrsig(r.sym);
+  ctx.forEachSymtab([](SymbolTable &symtab) {
+    for (Export &r : symtab.exports)
+      markAddrsig(r.sym);
+  });
 
   // Visit the address-significance table in each object file and mark each
   // referenced symbol as address-significant.
@@ -1376,13 +1378,13 @@ void LinkerDriver::maybeCreateECExportThunk(StringRef name, Symbol *&sym) {
 void LinkerDriver::createECExportThunks() {
   // Check if EXP+ symbols have corresponding $hp_target symbols and use them
   // to create export thunks when available.
-  for (Symbol *s : ctx.symtab.expSymbols) {
+  for (Symbol *s : ctx.symtabEC->expSymbols) {
     if (!s->isUsedInRegularObj)
       continue;
     assert(s->getName().starts_with("EXP+"));
     std::string targetName =
         (s->getName().substr(strlen("EXP+")) + "$hp_target").str();
-    Symbol *sym = ctx.symtab.find(targetName);
+    Symbol *sym = ctx.symtabEC->find(targetName);
     if (!sym)
       continue;
     Defined *targetSym;
@@ -1407,7 +1409,7 @@ void LinkerDriver::createECExportThunks() {
   if (ctx.symtabEC->entry)
     maybeCreateECExportThunk(ctx.symtabEC->entry->getName(),
                              ctx.symtabEC->entry);
-  for (Export &e : ctx.config.exports) {
+  for (Export &e : ctx.symtabEC->exports) {
     if (!e.data)
       maybeCreateECExportThunk(e.extName.empty() ? e.name : e.extName, e.sym);
   }
@@ -1430,7 +1432,7 @@ void LinkerDriver::maybeExportMinGWSymbols(const opt::InputArgList &args) {
     if (!ctx.config.dll)
       return;
 
-    if (!ctx.config.exports.empty())
+    if (!ctx.symtab.exports.empty())
       return;
     if (args.hasArg(OPT_exclude_all_symbols))
       return;
@@ -1466,7 +1468,7 @@ void LinkerDriver::maybeExportMinGWSymbols(const opt::InputArgList &args) {
       if (!(c->getOutputCharacteristics() & IMAGE_SCN_MEM_EXECUTE))
         e.data = true;
     s->isUsedInRegularObj = true;
-    ctx.config.exports.push_back(e);
+    ctx.symtab.exports.push_back(e);
   });
 }
 
@@ -2339,7 +2341,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
         if (!e.extName.empty() && !isDecorated(e.extName))
           e.extName = saver().save("_" + e.extName);
       }
-      config->exports.push_back(e);
+      mainSymtab.exports.push_back(e);
     }
   }
 
@@ -2351,7 +2353,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
 
   // Handle generation of import library from a def file.
   if (!args.hasArg(OPT_INPUT, OPT_wholearchive_file)) {
-    fixupExports();
+    ctx.forEachSymtab([](SymbolTable &symtab) { symtab.fixupExports(); });
     if (!config->noimplib)
       createImportLibrary(/*asLib=*/true);
     return;
@@ -2537,16 +2539,16 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
         // search for its mangled names.
         if (symtab.entry)
           symtab.mangleMaybe(symtab.entry);
-      });
 
-      // Windows specific -- Make sure we resolve all dllexported symbols.
-      for (Export &e : config->exports) {
-        if (!e.forwardTo.empty())
-          continue;
-        e.sym = ctx.symtab.addGCRoot(e.name, !e.data);
-        if (e.source != ExportSource::Directives)
-          e.symbolName = ctx.symtab.mangleMaybe(e.sym);
-      }
+        // Windows specific -- Make sure we resolve all dllexported symbols.
+        for (Export &e : symtab.exports) {
+          if (!e.forwardTo.empty())
+            continue;
+          e.sym = symtab.addGCRoot(e.name, !e.data);
+          if (e.source != ExportSource::Directives)
+            e.symbolName = symtab.mangleMaybe(e.sym);
+        }
+      });
 
       // Add weak aliases. Weak aliases is a mechanism to give remaining
       // undefined symbols final chance to be resolved successfully.
@@ -2647,7 +2649,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   if (errorCount())
     return;
 
-  config->hadExplicitExports = !config->exports.empty();
+  ctx.forEachSymtab([](SymbolTable &symtab) {
+    symtab.hadExplicitExports = !symtab.exports.empty();
+  });
   if (config->mingw) {
     // In MinGW, all symbols are automatically exported if no symbols
     // are chosen to be exported.
@@ -2712,17 +2716,18 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   // Windows specific -- when we are creating a .dll file, we also
   // need to create a .lib file. In MinGW mode, we only do that when the
   // -implib option is given explicitly, for compatibility with GNU ld.
-  if (!config->exports.empty() || config->dll) {
+  if (!ctx.symtab.exports.empty() || config->dll) {
     llvm::TimeTraceScope timeScope("Create .lib exports");
-    fixupExports();
+    ctx.forEachSymtab([](SymbolTable &symtab) { symtab.fixupExports(); });
     if (!config->noimplib && (!config->mingw || !config->implib.empty()))
       createImportLibrary(/*asLib=*/false);
-    assignExportOrdinals();
+    ctx.forEachSymtab(
+        [](SymbolTable &symtab) { symtab.assignExportOrdinals(); });
   }
 
   // Handle /output-def (MinGW specific).
   if (auto *arg = args.getLastArg(OPT_output_def))
-    writeDefFile(ctx, arg->getValue(), config->exports);
+    writeDefFile(ctx, arg->getValue(), ctx.symtab.exports);
 
   // Set extra alignment for .comm symbols
   for (auto pair : config->alignComm) {
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 8ce2e13129ba67..f1ccb93124576f 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -182,7 +182,6 @@ class LinkerDriver {
   std::list<std::function<void()>> taskQueue;
   std::vector<MemoryBufferRef> resources;
 
-  llvm::DenseSet<StringRef> directivesExports;
   llvm::DenseSet<StringRef> excludedSymbols;
 
   COFFLinkerContext &ctx;
@@ -246,8 +245,6 @@ class LinkerDriver {
 
   // Used for dllexported symbols.
   Export parseExport(StringRef arg);
-  void fixupExports();
-  void assignExportOrdinals();
 
   // Parses a string in the form of "key=value" and check
   // if value matches previous values for the key.
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
index 1148be09fb10cc..2d17dc1bc7c441 100644
--- a/lld/COFF/DriverUtils.cpp
+++ b/lld/COFF/DriverUtils.cpp
@@ -640,142 +640,6 @@ Export LinkerDriver::parseExport(StringRef arg) {
   llvm_unreachable("");
 }
 
-// Convert stdcall/fastcall style symbols into unsuffixed symbols,
-// with or without a leading underscore. (MinGW specific.)
-static StringRef killAt(StringRef sym, bool prefix) {
-  if (sym.empty())
-    return sym;
-  // Strip any trailing stdcall suffix
-  sym = sym.substr(0, sym.find('@', 1));
-  if (!sym.starts_with("@")) {
-    if (prefix && !sym.starts_with("_"))
-      return saver().save("_" + sym);
-    return sym;
-  }
-  // For fastcall, remove the leading @ and replace it with an
-  // underscore, if prefixes are used.
-  sym = sym.substr(1);
-  if (prefix)
-    sym = saver().save("_" + sym);
-  return sym;
-}
-
-static StringRef exportSourceName(ExportSource s) {
-  switch (s) {
-  case ExportSource::Directives:
-    return "source file (directives)";
-  case ExportSource::Export:
-    return "/export";
-  case ExportSource::ModuleDefinition:
-    return "/def";
-  default:
-    llvm_unreachable("unknown ExportSource");
-  }
-}
-
-// Performs error checking on all /export arguments.
-// It also sets ordinals.
-void LinkerDriver::fixupExports() {
-  llvm::TimeTraceScope timeScope("Fixup exports");
-  // Symbol ordinals must be unique.
-  std::set<uint16_t> ords;
-  for (Export &e : ctx.config.exports) {
-    if (e.ordinal == 0)
-      continue;
-    if (!ords.insert(e.ordinal).second)
-      Fatal(ctx) << "duplicate export ordinal: " << e.name;
-  }
-
-  for (Export &e : ctx.config.exports) {
-    if (!e.exportAs.empty()) {
-      e.exportName = e.exportAs;
-      continue;
-    }
-
-    StringRef sym =
-        !e.forwardTo.empty() || e.extName.empty() ? e.name : e.extName;
-    if (ctx.config.machine == I386 && sym.starts_with("_")) {
-      // In MSVC mode, a fully decorated stdcall function is exported
-      // as-is with the leading underscore (with type IMPORT_NAME).
-      // In MinGW mode, a decorated stdcall function gets the underscore
-      // removed, just like normal cdecl functions.
-      if (ctx.config.mingw || !sym.contains('@')) {
-        e.exportName = sym.substr(1);
-        continue;
-      }
-    }
-    if (isArm64EC(ctx.config.machine) && !e.data && !e.constant) {
-      if (std::optional<std::string> demangledName =
-              getArm64ECDemangledFunctionName(sym)) {
-        e.exportName = saver().save(*demangledName);
-        continue;
-      }
-    }
-    e.exportName = sym;
-  }
-
-  if (ctx.config.killAt && ctx.config.machine == I386) {
-    for (Export &e : ctx.config.exports) {
-      e.name = killAt(e.name, true);
-      e.exportName = killAt(e.exportName, false);
-      e.extName = killAt(e.extName, true);
-      e.symbolName = killAt(e.symbolName, true);
-    }
-  }
-
-  // Uniquefy by name.
-  DenseMap<StringRef, std::pair<Export *, unsigned>> map(
-      ctx.config.exports.size());
-  std::vector<Export> v;
-  for (Export &e : ctx.config.exports) {
-    auto pair = map.insert(std::make_pair(e.exportName, std::make_pair(&e, 0)));
-    bool inserted = pair.second;
-    if (inserted) {
-      pair.first->second.second = v.size();
-      v.push_back(e);
-      continue;
-    }
-    Export *existing = pair.first->second.first;
-    if (e == *existing || e.name != existing->name)
-      continue;
-    // If the existing export comes from .OBJ directives, we are allowed to
-    // overwrite it with /DEF: or /EXPORT without any warning, as MSVC link.exe
-    // does.
-    if (existing->source == ExportSource::Directives) {
-      *existing = e;
-      v[pair.first->second.second] = e;
-      continue;
-    }
-    if (existing->source == e.source) {
-      Warn(ctx) << "duplicate " << exportSourceName(existing->source)
-                << " option: " << e.name;
-    } else {
-      Warn(ctx) << "duplicate export: " << e.name << " first seen in "
-                << exportSourceName(existing->source) << ", now in "
-                << exportSourceName(e.source);
-    }
-  }
-  ctx.config.exports = std::move(v);
-
-  // Sort by name.
-  llvm::sort(ctx.config.exports, [](const Export &a, const Export &b) {
-    return a.exportName < b.exportName;
-  });
-}
-
-void LinkerDriver::assignExportOrdinals() {
-  // Assign unique ordinals if default (= 0).
-  uint32_t max = 0;
-  for (Export &e : ctx.config.exports)
-    max = std::max(max, (uint32_t)e.ordinal);
-  for (Export &e : ctx.config.exports)
-    if (e.ordinal == 0)
-      e.ordinal = ++max;
-  if (max > std::numeric_limits<uint16_t>::max())
-    Fatal(ctx) << "too many exported symbols (got " << max << ", max "
-               << Twine(std::numeric_limits<uint16_t>::max()) << ")";
-}
-
 // Parses a string in the form of "key=value" and check
 // if value matches previous values for the same key.
 void LinkerDriver::checkFailIfMismatch(StringRef arg, InputFile *source) {
diff --git a/lld/COFF/MapFile.cpp b/lld/COFF/MapFile.cpp
index af87587d143d56..eb98bb484f9f44 100644
--- a/lld/COFF/MapFile.cpp
+++ b/lld/COFF/MapFile.cpp
@@ -326,7 +326,7 @@ void lld::coff::writeMapFile(COFFLinkerContext &ctx) {
     os << " Exports\n";
     os << "\n";
     os << "  ordinal    name\n\n";
-    for (Export &e : ctx.config.exports) {
+    for (Export &e : ctx.symtab.exports) {
       os << format("  %7d", e.ordinal) << "    " << e.name << "\n";
       if (!e.extName.empty() && e.extName != e.name)
         os << "               exported name: " << e.extName << "\n";
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index bf965e8a2332db..a4bd7dd21b89a5 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -1118,6 +1118,141 @@ void SymbolTable::addUndefinedGlob(StringRef arg) {
     addGCRoot(sym->getName());
 }
 
+// Convert stdcall/fastcall style symbols into unsuffixed symbols,
+// with or without a leading underscore. (MinGW specific.)
+static StringRef killAt(StringRef sym, bool prefix) {
+  if (sym.empty())
+    return sym;
+  // Strip any trailing stdcall suffix
+  sym = sym.substr(0, sym.find('@', 1));
+  if (!sym.starts_with("@")) {
+    if (prefix && !sym.starts_with("_"))
+      return saver().save("_" + sym);
+    return sym;
+  }
+  // For fastcall, remove the leading @ and replace it with an
+  // underscore, if prefixes are used.
+  sym = sym.substr(1);
+  if (prefix)
+    sym = saver().save("_" + sym);
+  return sym;
+}
+
+static StringRef exportSourceName(ExportSource s) {
+  switch (s) {
+  case ExportSource::Directives:
+    return "source file (directives)";
+  case ExportSource::Export:
+    return "/export";
+  case ExportSource::ModuleDefinition:
+    return "/def";
+  default:
+    llvm_unreachable("unknown ExportSource");
+  }
+}
+
+// Performs error checking on all /export arguments.
+// It also sets ordinals.
+void SymbolTable::fixupExports() {
+  llvm::TimeTraceScope timeS...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2025

@llvm/pr-subscribers-lld-coff

Author: Jacek Caban (cjacek)

Changes

Store exports in SymbolTable instead of Configuration.


Patch is 31.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123652.diff

11 Files Affected:

  • (modified) lld/COFF/Config.h (-2)
  • (modified) lld/COFF/DLL.cpp (+18-16)
  • (modified) lld/COFF/DLL.h (+2-14)
  • (modified) lld/COFF/Driver.cpp (+32-27)
  • (modified) lld/COFF/Driver.h (-3)
  • (modified) lld/COFF/DriverUtils.cpp (-136)
  • (modified) lld/COFF/MapFile.cpp (+1-1)
  • (modified) lld/COFF/SymbolTable.cpp (+135)
  • (modified) lld/COFF/SymbolTable.h (+8)
  • (modified) lld/COFF/Writer.cpp (+11-10)
  • (added) lld/test/COFF/arm64x-export.test (+123)
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 924560fef0231d..1473705893ab01 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -161,8 +161,6 @@ struct Configuration {
   bool dll = false;
   StringRef implib;
   bool noimplib = false;
-  std::vector<Export> exports;
-  bool hadExplicitExports;
   std::set<std::string> delayLoads;
   std::map<std::string, int> dllOrder;
   Symbol *delayLoadHelper = nullptr;
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 875ada9d605394..6a3f8eb21e8475 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -639,22 +639,22 @@ class ExportDirectoryChunk : public NonSectionChunk {
 
 class AddressTableChunk : public NonSectionChunk {
 public:
-  explicit AddressTableChunk(COFFLinkerContext &ctx, size_t baseOrdinal,
+  explicit AddressTableChunk(SymbolTable &symtab, size_t baseOrdinal,
                              size_t maxOrdinal)
       : baseOrdinal(baseOrdinal), size((maxOrdinal - baseOrdinal) + 1),
-        ctx(ctx) {}
+        symtab(symtab) {}
   size_t getSize() const override { return size * 4; }
 
   void writeTo(uint8_t *buf) const override {
     memset(buf, 0, getSize());
 
-    for (const Export &e : ctx.config.exports) {
+    for (const Export &e : symtab.exports) {
       assert(e.ordinal >= baseOrdinal && "Export symbol has invalid ordinal");
       // Subtract the OrdinalBase to get the index.
       uint8_t *p = buf + (e.ordinal - baseOrdinal) * 4;
       uint32_t bit = 0;
       // Pointer to thumb code must have the LSB set, so adjust it.
-      if (ctx.config.machine == ARMNT && !e.data)
+      if (symtab.machine == ARMNT && !e.data)
         bit = 1;
       if (e.forwardChunk) {
         write32le(p, e.forwardChunk->getRVA() | bit);
@@ -669,7 +669,7 @@ class AddressTableChunk : public NonSectionChunk {
 private:
   size_t baseOrdinal;
   size_t size;
-  const COFFLinkerContext &ctx;
+  const SymbolTable &symtab;
 };
 
 class NamePointersChunk : public NonSectionChunk {
@@ -690,13 +690,13 @@ class NamePointersChunk : public NonSectionChunk {
 
 class ExportOrdinalChunk : public NonSectionChunk {
 public:
-  explicit ExportOrdinalChunk(const COFFLinkerContext &ctx, size_t baseOrdinal,
+  explicit ExportOrdinalChunk(const SymbolTable &symtab, size_t baseOrdinal,
                               size_t tableSize)
-      : baseOrdinal(baseOrdinal), size(tableSize), ctx(ctx) {}
+      : baseOrdinal(baseOrdinal), size(tableSize), symtab(symtab) {}
   size_t getSize() const override { return size * 2; }
 
   void writeTo(uint8_t *buf) const override {
-    for (const Export &e : ctx.config.exports) {
+    for (const Export &e : symtab.exports) {
       if (e.noname)
         continue;
       assert(e.ordinal >= baseOrdinal && "Export symbol has invalid ordinal");
@@ -709,7 +709,7 @@ class ExportOrdinalChunk : public NonSectionChunk {
 private:
   size_t baseOrdinal;
   size_t size;
-  const COFFLinkerContext &ctx;
+  const SymbolTable &symtab;
 };
 
 } // anonymous namespace
@@ -920,9 +920,9 @@ Chunk *DelayLoadContents::newThunkChunk(DefinedImportData *s,
   }
 }
 
-EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
+void createEdataChunks(SymbolTable &symtab, std::vector<Chunk *> &chunks) {
   unsigned baseOrdinal = 1 << 16, maxOrdinal = 0;
-  for (Export &e : ctx.config.exports) {
+  for (Export &e : symtab.exports) {
     baseOrdinal = std::min(baseOrdinal, (unsigned)e.ordinal);
     maxOrdinal = std::max(maxOrdinal, (unsigned)e.ordinal);
   }
@@ -930,15 +930,16 @@ EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
   // https://learn.microsoft.com/en-us/cpp/build/reference/export-exports-a-function?view=msvc-170
   assert(baseOrdinal >= 1);
 
-  auto *dllName = make<StringChunk>(sys::path::filename(ctx.config.outputFile));
-  auto *addressTab = make<AddressTableChunk>(ctx, baseOrdinal, maxOrdinal);
+  auto *dllName =
+      make<StringChunk>(sys::path::filename(symtab.ctx.config.outputFile));
+  auto *addressTab = make<AddressTableChunk>(symtab, baseOrdinal, maxOrdinal);
   std::vector<Chunk *> names;
-  for (Export &e : ctx.config.exports)
+  for (Export &e : symtab.exports)
     if (!e.noname)
       names.push_back(make<StringChunk>(e.exportName));
 
   std::vector<Chunk *> forwards;
-  for (Export &e : ctx.config.exports) {
+  for (Export &e : symtab.exports) {
     if (e.forwardTo.empty())
       continue;
     e.forwardChunk = make<StringChunk>(e.forwardTo);
@@ -946,7 +947,8 @@ EdataContents::EdataContents(COFFLinkerContext &ctx) : ctx(ctx) {
   }
 
   auto *nameTab = make<NamePointersChunk>(names);
-  auto *ordinalTab = make<ExportOrdinalChunk>(ctx, baseOrdinal, names.size());
+  auto *ordinalTab =
+      make<ExportOrdinalChunk>(symtab, baseOrdinal, names.size());
   auto *dir =
       make<ExportDirectoryChunk>(baseOrdinal, maxOrdinal, names.size(), dllName,
                                  addressTab, nameTab, ordinalTab);
diff --git a/lld/COFF/DLL.h b/lld/COFF/DLL.h
index f7d2b57a20a020..724a323d62d205 100644
--- a/lld/COFF/DLL.h
+++ b/lld/COFF/DLL.h
@@ -77,20 +77,8 @@ class DelayLoadContents {
   COFFLinkerContext &ctx;
 };
 
-// Windows-specific.
-// EdataContents creates all chunks for the DLL export table.
-class EdataContents {
-public:
-  EdataContents(COFFLinkerContext &ctx);
-  std::vector<Chunk *> chunks;
-
-  uint64_t getRVA() { return chunks[0]->getRVA(); }
-  uint64_t getSize() {
-    return chunks.back()->getRVA() + chunks.back()->getSize() - getRVA();
-  }
-
-  COFFLinkerContext &ctx;
-};
+// Create all chunks for the DLL export table.
+void createEdataChunks(SymbolTable &symtab, std::vector<Chunk *> &chunks);
 
 } // namespace lld::coff
 
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 898c6c17d2062a..eca0af7a3378f4 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -458,7 +458,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
     // declarations, many object files may end up with having the
     // same /EXPORT options. In order to save cost of parsing them,
     // we dedup them first.
-    if (!directivesExports.insert(e).second)
+    if (!file->symtab.directivesExports.insert(e).second)
       continue;
 
     Export exp = parseExport(e);
@@ -469,7 +469,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
         exp.extName = saver().save("_" + exp.extName);
     }
     exp.source = ExportSource::Directives;
-    ctx.config.exports.push_back(exp);
+    file->symtab.exports.push_back(exp);
   }
 
   // Handle /include: in bulk.
@@ -956,7 +956,7 @@ std::string LinkerDriver::getImportName(bool asLib) {
 void LinkerDriver::createImportLibrary(bool asLib) {
   llvm::TimeTraceScope timeScope("Create import library");
   std::vector<COFFShortExport> exports;
-  for (Export &e1 : ctx.config.exports) {
+  for (Export &e1 : ctx.symtab.exports) {
     COFFShortExport e2;
     e2.Name = std::string(e1.name);
     e2.SymbolName = std::string(e1.symbolName);
@@ -1069,7 +1069,7 @@ void LinkerDriver::parseModuleDefs(StringRef path) {
     e2.isPrivate = e1.Private;
     e2.constant = e1.Constant;
     e2.source = ExportSource::ModuleDefinition;
-    ctx.config.exports.push_back(e2);
+    ctx.symtab.exports.push_back(e2);
   }
 }
 
@@ -1222,8 +1222,10 @@ static void findKeepUniqueSections(COFFLinkerContext &ctx) {
 
   // Exported symbols could be address-significant in other executables or DSOs,
   // so we conservatively mark them as address-significant.
-  for (Export &r : ctx.config.exports)
-    markAddrsig(r.sym);
+  ctx.forEachSymtab([](SymbolTable &symtab) {
+    for (Export &r : symtab.exports)
+      markAddrsig(r.sym);
+  });
 
   // Visit the address-significance table in each object file and mark each
   // referenced symbol as address-significant.
@@ -1376,13 +1378,13 @@ void LinkerDriver::maybeCreateECExportThunk(StringRef name, Symbol *&sym) {
 void LinkerDriver::createECExportThunks() {
   // Check if EXP+ symbols have corresponding $hp_target symbols and use them
   // to create export thunks when available.
-  for (Symbol *s : ctx.symtab.expSymbols) {
+  for (Symbol *s : ctx.symtabEC->expSymbols) {
     if (!s->isUsedInRegularObj)
       continue;
     assert(s->getName().starts_with("EXP+"));
     std::string targetName =
         (s->getName().substr(strlen("EXP+")) + "$hp_target").str();
-    Symbol *sym = ctx.symtab.find(targetName);
+    Symbol *sym = ctx.symtabEC->find(targetName);
     if (!sym)
       continue;
     Defined *targetSym;
@@ -1407,7 +1409,7 @@ void LinkerDriver::createECExportThunks() {
   if (ctx.symtabEC->entry)
     maybeCreateECExportThunk(ctx.symtabEC->entry->getName(),
                              ctx.symtabEC->entry);
-  for (Export &e : ctx.config.exports) {
+  for (Export &e : ctx.symtabEC->exports) {
     if (!e.data)
       maybeCreateECExportThunk(e.extName.empty() ? e.name : e.extName, e.sym);
   }
@@ -1430,7 +1432,7 @@ void LinkerDriver::maybeExportMinGWSymbols(const opt::InputArgList &args) {
     if (!ctx.config.dll)
       return;
 
-    if (!ctx.config.exports.empty())
+    if (!ctx.symtab.exports.empty())
       return;
     if (args.hasArg(OPT_exclude_all_symbols))
       return;
@@ -1466,7 +1468,7 @@ void LinkerDriver::maybeExportMinGWSymbols(const opt::InputArgList &args) {
       if (!(c->getOutputCharacteristics() & IMAGE_SCN_MEM_EXECUTE))
         e.data = true;
     s->isUsedInRegularObj = true;
-    ctx.config.exports.push_back(e);
+    ctx.symtab.exports.push_back(e);
   });
 }
 
@@ -2339,7 +2341,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
         if (!e.extName.empty() && !isDecorated(e.extName))
           e.extName = saver().save("_" + e.extName);
       }
-      config->exports.push_back(e);
+      mainSymtab.exports.push_back(e);
     }
   }
 
@@ -2351,7 +2353,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
 
   // Handle generation of import library from a def file.
   if (!args.hasArg(OPT_INPUT, OPT_wholearchive_file)) {
-    fixupExports();
+    ctx.forEachSymtab([](SymbolTable &symtab) { symtab.fixupExports(); });
     if (!config->noimplib)
       createImportLibrary(/*asLib=*/true);
     return;
@@ -2537,16 +2539,16 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
         // search for its mangled names.
         if (symtab.entry)
           symtab.mangleMaybe(symtab.entry);
-      });
 
-      // Windows specific -- Make sure we resolve all dllexported symbols.
-      for (Export &e : config->exports) {
-        if (!e.forwardTo.empty())
-          continue;
-        e.sym = ctx.symtab.addGCRoot(e.name, !e.data);
-        if (e.source != ExportSource::Directives)
-          e.symbolName = ctx.symtab.mangleMaybe(e.sym);
-      }
+        // Windows specific -- Make sure we resolve all dllexported symbols.
+        for (Export &e : symtab.exports) {
+          if (!e.forwardTo.empty())
+            continue;
+          e.sym = symtab.addGCRoot(e.name, !e.data);
+          if (e.source != ExportSource::Directives)
+            e.symbolName = symtab.mangleMaybe(e.sym);
+        }
+      });
 
       // Add weak aliases. Weak aliases is a mechanism to give remaining
       // undefined symbols final chance to be resolved successfully.
@@ -2647,7 +2649,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   if (errorCount())
     return;
 
-  config->hadExplicitExports = !config->exports.empty();
+  ctx.forEachSymtab([](SymbolTable &symtab) {
+    symtab.hadExplicitExports = !symtab.exports.empty();
+  });
   if (config->mingw) {
     // In MinGW, all symbols are automatically exported if no symbols
     // are chosen to be exported.
@@ -2712,17 +2716,18 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   // Windows specific -- when we are creating a .dll file, we also
   // need to create a .lib file. In MinGW mode, we only do that when the
   // -implib option is given explicitly, for compatibility with GNU ld.
-  if (!config->exports.empty() || config->dll) {
+  if (!ctx.symtab.exports.empty() || config->dll) {
     llvm::TimeTraceScope timeScope("Create .lib exports");
-    fixupExports();
+    ctx.forEachSymtab([](SymbolTable &symtab) { symtab.fixupExports(); });
     if (!config->noimplib && (!config->mingw || !config->implib.empty()))
       createImportLibrary(/*asLib=*/false);
-    assignExportOrdinals();
+    ctx.forEachSymtab(
+        [](SymbolTable &symtab) { symtab.assignExportOrdinals(); });
   }
 
   // Handle /output-def (MinGW specific).
   if (auto *arg = args.getLastArg(OPT_output_def))
-    writeDefFile(ctx, arg->getValue(), config->exports);
+    writeDefFile(ctx, arg->getValue(), ctx.symtab.exports);
 
   // Set extra alignment for .comm symbols
   for (auto pair : config->alignComm) {
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 8ce2e13129ba67..f1ccb93124576f 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -182,7 +182,6 @@ class LinkerDriver {
   std::list<std::function<void()>> taskQueue;
   std::vector<MemoryBufferRef> resources;
 
-  llvm::DenseSet<StringRef> directivesExports;
   llvm::DenseSet<StringRef> excludedSymbols;
 
   COFFLinkerContext &ctx;
@@ -246,8 +245,6 @@ class LinkerDriver {
 
   // Used for dllexported symbols.
   Export parseExport(StringRef arg);
-  void fixupExports();
-  void assignExportOrdinals();
 
   // Parses a string in the form of "key=value" and check
   // if value matches previous values for the key.
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
index 1148be09fb10cc..2d17dc1bc7c441 100644
--- a/lld/COFF/DriverUtils.cpp
+++ b/lld/COFF/DriverUtils.cpp
@@ -640,142 +640,6 @@ Export LinkerDriver::parseExport(StringRef arg) {
   llvm_unreachable("");
 }
 
-// Convert stdcall/fastcall style symbols into unsuffixed symbols,
-// with or without a leading underscore. (MinGW specific.)
-static StringRef killAt(StringRef sym, bool prefix) {
-  if (sym.empty())
-    return sym;
-  // Strip any trailing stdcall suffix
-  sym = sym.substr(0, sym.find('@', 1));
-  if (!sym.starts_with("@")) {
-    if (prefix && !sym.starts_with("_"))
-      return saver().save("_" + sym);
-    return sym;
-  }
-  // For fastcall, remove the leading @ and replace it with an
-  // underscore, if prefixes are used.
-  sym = sym.substr(1);
-  if (prefix)
-    sym = saver().save("_" + sym);
-  return sym;
-}
-
-static StringRef exportSourceName(ExportSource s) {
-  switch (s) {
-  case ExportSource::Directives:
-    return "source file (directives)";
-  case ExportSource::Export:
-    return "/export";
-  case ExportSource::ModuleDefinition:
-    return "/def";
-  default:
-    llvm_unreachable("unknown ExportSource");
-  }
-}
-
-// Performs error checking on all /export arguments.
-// It also sets ordinals.
-void LinkerDriver::fixupExports() {
-  llvm::TimeTraceScope timeScope("Fixup exports");
-  // Symbol ordinals must be unique.
-  std::set<uint16_t> ords;
-  for (Export &e : ctx.config.exports) {
-    if (e.ordinal == 0)
-      continue;
-    if (!ords.insert(e.ordinal).second)
-      Fatal(ctx) << "duplicate export ordinal: " << e.name;
-  }
-
-  for (Export &e : ctx.config.exports) {
-    if (!e.exportAs.empty()) {
-      e.exportName = e.exportAs;
-      continue;
-    }
-
-    StringRef sym =
-        !e.forwardTo.empty() || e.extName.empty() ? e.name : e.extName;
-    if (ctx.config.machine == I386 && sym.starts_with("_")) {
-      // In MSVC mode, a fully decorated stdcall function is exported
-      // as-is with the leading underscore (with type IMPORT_NAME).
-      // In MinGW mode, a decorated stdcall function gets the underscore
-      // removed, just like normal cdecl functions.
-      if (ctx.config.mingw || !sym.contains('@')) {
-        e.exportName = sym.substr(1);
-        continue;
-      }
-    }
-    if (isArm64EC(ctx.config.machine) && !e.data && !e.constant) {
-      if (std::optional<std::string> demangledName =
-              getArm64ECDemangledFunctionName(sym)) {
-        e.exportName = saver().save(*demangledName);
-        continue;
-      }
-    }
-    e.exportName = sym;
-  }
-
-  if (ctx.config.killAt && ctx.config.machine == I386) {
-    for (Export &e : ctx.config.exports) {
-      e.name = killAt(e.name, true);
-      e.exportName = killAt(e.exportName, false);
-      e.extName = killAt(e.extName, true);
-      e.symbolName = killAt(e.symbolName, true);
-    }
-  }
-
-  // Uniquefy by name.
-  DenseMap<StringRef, std::pair<Export *, unsigned>> map(
-      ctx.config.exports.size());
-  std::vector<Export> v;
-  for (Export &e : ctx.config.exports) {
-    auto pair = map.insert(std::make_pair(e.exportName, std::make_pair(&e, 0)));
-    bool inserted = pair.second;
-    if (inserted) {
-      pair.first->second.second = v.size();
-      v.push_back(e);
-      continue;
-    }
-    Export *existing = pair.first->second.first;
-    if (e == *existing || e.name != existing->name)
-      continue;
-    // If the existing export comes from .OBJ directives, we are allowed to
-    // overwrite it with /DEF: or /EXPORT without any warning, as MSVC link.exe
-    // does.
-    if (existing->source == ExportSource::Directives) {
-      *existing = e;
-      v[pair.first->second.second] = e;
-      continue;
-    }
-    if (existing->source == e.source) {
-      Warn(ctx) << "duplicate " << exportSourceName(existing->source)
-                << " option: " << e.name;
-    } else {
-      Warn(ctx) << "duplicate export: " << e.name << " first seen in "
-                << exportSourceName(existing->source) << ", now in "
-                << exportSourceName(e.source);
-    }
-  }
-  ctx.config.exports = std::move(v);
-
-  // Sort by name.
-  llvm::sort(ctx.config.exports, [](const Export &a, const Export &b) {
-    return a.exportName < b.exportName;
-  });
-}
-
-void LinkerDriver::assignExportOrdinals() {
-  // Assign unique ordinals if default (= 0).
-  uint32_t max = 0;
-  for (Export &e : ctx.config.exports)
-    max = std::max(max, (uint32_t)e.ordinal);
-  for (Export &e : ctx.config.exports)
-    if (e.ordinal == 0)
-      e.ordinal = ++max;
-  if (max > std::numeric_limits<uint16_t>::max())
-    Fatal(ctx) << "too many exported symbols (got " << max << ", max "
-               << Twine(std::numeric_limits<uint16_t>::max()) << ")";
-}
-
 // Parses a string in the form of "key=value" and check
 // if value matches previous values for the same key.
 void LinkerDriver::checkFailIfMismatch(StringRef arg, InputFile *source) {
diff --git a/lld/COFF/MapFile.cpp b/lld/COFF/MapFile.cpp
index af87587d143d56..eb98bb484f9f44 100644
--- a/lld/COFF/MapFile.cpp
+++ b/lld/COFF/MapFile.cpp
@@ -326,7 +326,7 @@ void lld::coff::writeMapFile(COFFLinkerContext &ctx) {
     os << " Exports\n";
     os << "\n";
     os << "  ordinal    name\n\n";
-    for (Export &e : ctx.config.exports) {
+    for (Export &e : ctx.symtab.exports) {
       os << format("  %7d", e.ordinal) << "    " << e.name << "\n";
       if (!e.extName.empty() && e.extName != e.name)
         os << "               exported name: " << e.extName << "\n";
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index bf965e8a2332db..a4bd7dd21b89a5 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -1118,6 +1118,141 @@ void SymbolTable::addUndefinedGlob(StringRef arg) {
     addGCRoot(sym->getName());
 }
 
+// Convert stdcall/fastcall style symbols into unsuffixed symbols,
+// with or without a leading underscore. (MinGW specific.)
+static StringRef killAt(StringRef sym, bool prefix) {
+  if (sym.empty())
+    return sym;
+  // Strip any trailing stdcall suffix
+  sym = sym.substr(0, sym.find('@', 1));
+  if (!sym.starts_with("@")) {
+    if (prefix && !sym.starts_with("_"))
+      return saver().save("_" + sym);
+    return sym;
+  }
+  // For fastcall, remove the leading @ and replace it with an
+  // underscore, if prefixes are used.
+  sym = sym.substr(1);
+  if (prefix)
+    sym = saver().save("_" + sym);
+  return sym;
+}
+
+static StringRef exportSourceName(ExportSource s) {
+  switch (s) {
+  case ExportSource::Directives:
+    return "source file (directives)";
+  case ExportSource::Export:
+    return "/export";
+  case ExportSource::ModuleDefinition:
+    return "/def";
+  default:
+    llvm_unreachable("unknown ExportSource");
+  }
+}
+
+// Performs error checking on all /export arguments.
+// It also sets ordinals.
+void SymbolTable::fixupExports() {
+  llvm::TimeTraceScope timeS...
[truncated]

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming the large amount of moved code is unmodified (except for references to the fields that are moved around).

It's somewhat sad to see that we need to move this amount of code from Driver* into SymbolTable, but I guess it follows from the fact that this needs to operate on both namespaces separately.

@cjacek
Copy link
Contributor Author

cjacek commented Jan 20, 2025

Thanks!

It's somewhat sad to see that we need to move this amount of code from Driver* into SymbolTable, but I guess it follows from the fact that this needs to operate on both namespaces separately.

Yes, the code needs to know which symbol table it operates on. FWIW, in an earlier version of these changes, I simply passed it as an argument to driver functions. While the diff was smaller, it didn't seem particularly well-structured.

Store exports in SymbolTable instead of Configuration.
@cjacek cjacek merged commit 455b3d6 into llvm:main Jan 21, 2025
5 of 6 checks passed
@cjacek cjacek deleted the symtab-exports branch January 21, 2025 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants