[lld][WebAssembly] Introduce Ctx::arg

[MaskRay]

and forward it to LinkerDriver's ctor so that some uses of the global
config can be dropped. This is similar to how the ELF port
migrates away from the global config.

[llvmbot]

@llvm/pr-subscribers-lld-wasm

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

and forward it to LinkerDriver's ctor so that some uses of the global
config can be dropped. This is similar to how the ELF port
migrates away from the global config.

---

Full diff: https://github.com/llvm/llvm-project/pull/119829.diff

2 Files Affected:

• (modified) lld/wasm/Config.h (+10-2)
• (modified) lld/wasm/Driver.cpp (+35-26)

diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index eb32ce80f4a3d9..0c2ba3eebffc4f 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -43,7 +43,7 @@ enum class BuildIdKind { None, Fast, Sha1, Hexstring, Uuid };
 // Most fields are direct mapping from the command line options
 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.
-struct Configuration {
+struct Config {
   bool allowMultipleDefinition;
   bool bsymbolic;
   bool checkFeatures;
@@ -126,11 +126,18 @@ struct Configuration {
   llvm::SmallVector<uint8_t, 0> buildIdVector;
 };
 
+struct ConfigWrapper {
+  Config c;
+  Config *operator->() { return &c; }
+};
+
 // The only instance of Configuration struct.
-extern Configuration *config;
+extern ConfigWrapper config;
 
 // The Ctx object hold all other (non-configuration) global state.
 struct Ctx {
+  Config &arg;
+
   llvm::SmallVector<ObjFile *, 0> objectFiles;
   llvm::SmallVector<StubFile *, 0> stubFiles;
   llvm::SmallVector<SharedFile *, 0> sharedFiles;
@@ -156,6 +163,7 @@ struct Ctx {
                     0>
       whyExtractRecords;
 
+  Ctx();
   void reset();
 };
 
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 00b5c82d9c7777..02471950fb5196 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -44,7 +44,7 @@ using namespace llvm::sys;
 using namespace llvm::wasm;
 
 namespace lld::wasm {
-Configuration *config;
+ConfigWrapper config;
 Ctx ctx;
 
 void errorOrWarn(const llvm::Twine &msg) {
@@ -54,7 +54,11 @@ void errorOrWarn(const llvm::Twine &msg) {
     error(msg);
 }
 
+Ctx::Ctx() : arg(config.c) {}
+
 void Ctx::reset() {
+  arg.~Config();
+  new (&arg) Config();
   objectFiles.clear();
   stubFiles.clear();
   sharedFiles.clear();
@@ -92,6 +96,7 @@ static void initLLVM() {
 
 class LinkerDriver {
 public:
+  LinkerDriver(Ctx &);
   void linkerMain(ArrayRef<const char *> argsArr);
 
 private:
@@ -99,6 +104,8 @@ class LinkerDriver {
   void addFile(StringRef path);
   void addLibrary(StringRef name);
 
+  Ctx &ctx;
+
   // True if we are in --whole-archive and --no-whole-archive.
   bool inWholeArchive = false;
 
@@ -122,19 +129,19 @@ static bool hasZOption(opt::InputArgList &args, StringRef key) {
 bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
           llvm::raw_ostream &stderrOS, bool exitEarly, bool disableOutput) {
   // This driver-specific context will be freed later by unsafeLldMain().
-  auto *ctx = new CommonLinkerContext;
+  auto *context = new CommonLinkerContext;
 
-  ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput);
-  ctx->e.cleanupCallback = []() { wasm::ctx.reset(); };
-  ctx->e.logName = args::getFilenameWithoutExe(args[0]);
-  ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use "
-                                 "-error-limit=0 to see all errors)";
+  context->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput);
+  context->e.cleanupCallback = []() { ctx.reset(); };
+  context->e.logName = args::getFilenameWithoutExe(args[0]);
+  context->e.errorLimitExceededMsg =
+      "too many errors emitted, stopping now (use "
+      "-error-limit=0 to see all errors)";
 
-  config = make<Configuration>();
   symtab = make<SymbolTable>();
 
   initLLVM();
-  LinkerDriver().linkerMain(args);
+  LinkerDriver(ctx).linkerMain(args);
 
   return errorCount() == 0;
 }
@@ -1256,6 +1263,8 @@ static void checkZOptions(opt::InputArgList &args) {
       warn("unknown -z value: " + StringRef(arg->getValue()));
 }
 
+LinkerDriver::LinkerDriver(Ctx &ctx) : ctx(ctx) {}
+
 void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   WasmOptTable parser;
   opt::InputArgList args = parser.parse(argsArr.slice(1));
@@ -1324,10 +1333,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   // Fail early if the output file or map file is not writable. If a user has a
   // long link, e.g. due to a large LTO link, they do not wish to run it and
   // find that it failed because there was a mistake in their command-line.
-  if (auto e = tryCreateFile(config->outputFile))
-    error("cannot open output file " + config->outputFile + ": " + e.message());
-  if (auto e = tryCreateFile(config->mapFile))
-    error("cannot open map file " + config->mapFile + ": " + e.message());
+  if (auto e = tryCreateFile(ctx.arg.outputFile))
+    error("cannot open output file " + ctx.arg.outputFile + ": " + e.message());
+  if (auto e = tryCreateFile(ctx.arg.mapFile))
+    error("cannot open map file " + ctx.arg.mapFile + ": " + e.message());
   if (errorCount())
     return;
 
@@ -1336,11 +1345,11 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     symtab->trace(arg->getValue());
 
   for (auto *arg : args.filtered(OPT_export_if_defined))
-    config->exportedSymbols.insert(arg->getValue());
+    ctx.arg.exportedSymbols.insert(arg->getValue());
 
   for (auto *arg : args.filtered(OPT_export)) {
-    config->exportedSymbols.insert(arg->getValue());
-    config->requiredExports.push_back(arg->getValue());
+    ctx.arg.exportedSymbols.insert(arg->getValue());
+    ctx.arg.requiredExports.push_back(arg->getValue());
   }
 
   createSyntheticSymbols();
@@ -1358,17 +1367,17 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
 
   // Handle the `--export <sym>` options
   // This works like --undefined but also exports the symbol if its found
-  for (auto &iter : config->exportedSymbols)
+  for (auto &iter : ctx.arg.exportedSymbols)
     handleUndefined(iter.first(), "--export");
 
   Symbol *entrySym = nullptr;
-  if (!config->relocatable && !config->entry.empty()) {
-    entrySym = handleUndefined(config->entry, "--entry");
+  if (!ctx.arg.relocatable && !ctx.arg.entry.empty()) {
+    entrySym = handleUndefined(ctx.arg.entry, "--entry");
     if (entrySym && entrySym->isDefined())
       entrySym->forceExport = true;
     else
       error("entry symbol not defined (pass --no-entry to suppress): " +
-            config->entry);
+            ctx.arg.entry);
   }
 
   // If the user code defines a `__wasm_call_dtors` function, remember it so
@@ -1376,10 +1385,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   // `__wasm_call_ctors` which we synthesize, `__wasm_call_dtors` is defined
   // by libc/etc., because destructors are registered dynamically with
   // `__cxa_atexit` and friends.
-  if (!config->relocatable && !config->shared &&
+  if (!ctx.arg.relocatable && !ctx.arg.shared &&
       !WasmSym::callCtors->isUsedInRegularObj &&
-      WasmSym::callCtors->getName() != config->entry &&
-      !config->exportedSymbols.count(WasmSym::callCtors->getName())) {
+      WasmSym::callCtors->getName() != ctx.arg.entry &&
+      !ctx.arg.exportedSymbols.count(WasmSym::callCtors->getName())) {
     if (Symbol *callDtors =
             handleUndefined("__wasm_call_dtors", "<internal>")) {
       if (auto *callDtorsFunc = dyn_cast<DefinedFunction>(callDtors)) {
@@ -1437,7 +1446,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   writeWhyExtract();
 
   // Bail out if normal linked output is skipped due to LTO.
-  if (config->thinLTOIndexOnly)
+  if (ctx.arg.thinLTOIndexOnly)
     return;
 
   createOptionalSymbols();
@@ -1452,13 +1461,13 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   if (!wrapped.empty())
     wrapSymbols(wrapped);
 
-  for (auto &iter : config->exportedSymbols) {
+  for (auto &iter : ctx.arg.exportedSymbols) {
     Symbol *sym = symtab->find(iter.first());
     if (sym && sym->isDefined())
       sym->forceExport = true;
   }
 
-  if (!config->relocatable && !ctx.isPic) {
+  if (!ctx.arg.relocatable && !ctx.isPic) {
     // Add synthetic dummies for weak undefined functions.  Must happen
     // after LTO otherwise functions may not yet have signatures.
     symtab->handleWeakUndefines();

[sbc100]

I generally support changes that bring us closer to the way the ELF linker works, so this change LGTM.

As a said in the other PR I'm a little sad that the extra indirection and noise, but I guess its worth it for the library use case.

[sbc100]

The convention we have been using for PR titles is [lld][WebAssembly] and we use [WebAssembly] when its not part of the linker. I know the whole wasm vs Wasm vs WebAssembly thing is inconsistent in llvm but we are fairly consistent with the PR titles.