Skip to content

[lld][WebAssembly] Handle stub symbol dependencies when an explicit import name is used #80169

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
Jun 20, 2024

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jan 31, 2024

No description provided.

Copy link

github-actions bot commented Jan 31, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@sbc100 sbc100 force-pushed the fix_stub_import_names branch 2 times, most recently from 00987e9 to cd13377 Compare June 19, 2024 18:39
@sbc100 sbc100 requested a review from dschuff June 19, 2024 18:39
@sbc100 sbc100 force-pushed the fix_stub_import_names branch from cd13377 to c5373a2 Compare June 19, 2024 23:45
@sbc100 sbc100 force-pushed the fix_stub_import_names branch from c5373a2 to 2a8c4bb Compare June 19, 2024 23:45
@sbc100 sbc100 marked this pull request as ready for review June 19, 2024 23:45
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2024

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-wasm

Author: Sam Clegg (sbc100)

Changes

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

4 Files Affected:

  • (modified) lld/test/wasm/Inputs/libstub.so (+1-1)
  • (modified) lld/test/wasm/stub-library-archive.s (+1)
  • (modified) lld/test/wasm/stub-library.s (+1-4)
  • (modified) lld/wasm/Driver.cpp (+57-37)
diff --git a/lld/test/wasm/Inputs/libstub.so b/lld/test/wasm/Inputs/libstub.so
index f7e38b4272f24..70f6648c60eee 100644
--- a/lld/test/wasm/Inputs/libstub.so
+++ b/lld/test/wasm/Inputs/libstub.so
@@ -1,6 +1,6 @@
 #STUB
 # This is a comment
-foo: foodep1,foodep2
+foo_import: foodep1,foodep2
 # This symbols as no dependencies
 bar
 baz: bazdep
diff --git a/lld/test/wasm/stub-library-archive.s b/lld/test/wasm/stub-library-archive.s
index 7aacf676b3436..76483d1463d64 100644
--- a/lld/test/wasm/stub-library-archive.s
+++ b/lld/test/wasm/stub-library-archive.s
@@ -14,6 +14,7 @@
 # depeds on baz which is also defined in libstub.so.
 
 .functype foo () -> ()
+.import_name foo, foo_import
 
 .globl _start
 _start:
diff --git a/lld/test/wasm/stub-library.s b/lld/test/wasm/stub-library.s
index 9cbf2505ea9e7..b972003c5b05d 100644
--- a/lld/test/wasm/stub-library.s
+++ b/lld/test/wasm/stub-library.s
@@ -6,15 +6,12 @@
 # RUN: not wasm-ld %t.o %p/Inputs/libstub-missing-dep.so -o %t.wasm 2>&1 | FileCheck --check-prefix=MISSING-DEP %s
 
 # When the dependencies are missing the link fails
-# RUN: not wasm-ld %t.o %p/Inputs/libstub-missing-sym.so -o %t.wasm 2>&1 | FileCheck --check-prefix=MISSING-SYM %s
-
 # MISSING-DEP: libstub-missing-dep.so: undefined symbol: missing_dep. Required by foo
 # MISSING-DEP: libstub-missing-dep.so: undefined symbol: missing_dep2. Required by foo
 
-# MISSING-SYM: undefined symbol: foo
-
 # The function foo is defined in libstub.so but depend on foodep1 and foodep2
 .functype foo () -> ()
+.import_name foo, foo_import
 
 .globl foodep1
 foodep1:
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index b06c1e9b0be1b..d099689911fc6 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -955,6 +955,47 @@ static void processStubLibrariesPreLTO() {
   }
 }
 
+static bool addStubSymbolDeps(const StubFile *stub_file, Symbol *sym,
+                              ArrayRef<StringRef> deps) {
+  // The first stub library to define a given symbol sets this and
+  // definitions in later stub libraries are ignored.
+  if (sym->forceImport)
+    return false; // Already handled
+  sym->forceImport = true;
+  if (sym->traced)
+    message(toString(stub_file) + ": importing " + sym->getName());
+  else
+    LLVM_DEBUG(llvm::dbgs() << toString(stub_file) << ": importing "
+                            << sym->getName() << "\n");
+  bool depsAdded = false;
+  for (const auto dep : deps) {
+    auto *needed = symtab->find(dep);
+    if (!needed) {
+      error(toString(stub_file) + ": undefined symbol: " + dep +
+            ". Required by " + toString(*sym));
+    } else if (needed->isUndefined()) {
+      error(toString(stub_file) + ": undefined symbol: " + toString(*needed) +
+            ". Required by " + toString(*sym));
+    } else {
+      if (needed->traced)
+        message(toString(stub_file) + ": exported " + toString(*needed) +
+                " due to import of " + sym->getName());
+      else
+        LLVM_DEBUG(llvm::dbgs()
+                   << "force export: " << toString(*needed) << "\n");
+      needed->forceExport = true;
+      if (auto *lazy = dyn_cast<LazySymbol>(needed)) {
+        depsAdded = true;
+        lazy->extract();
+        if (!config->whyExtract.empty())
+          ctx.whyExtractRecords.emplace_back(toString(stub_file),
+                                             sym->getFile(), *sym);
+      }
+    }
+  }
+  return depsAdded;
+}
+
 static void processStubLibraries() {
   log("-- processStubLibraries");
   bool depsAdded = false;
@@ -963,49 +1004,28 @@ static void processStubLibraries() {
     for (auto &stub_file : ctx.stubFiles) {
       LLVM_DEBUG(llvm::dbgs()
                  << "processing stub file: " << stub_file->getName() << "\n");
+
+      // First look for any imported symbols that directly match
+      // the names of the stub imports
       for (auto [name, deps]: stub_file->symbolDependencies) {
         auto* sym = symtab->find(name);
-        if (!sym || !sym->isUndefined()) {
+        if (sym && sym->isUndefined()) {
+          depsAdded |= addStubSymbolDeps(stub_file, sym, deps);
+        } else {
           if (sym && sym->traced)
             message(toString(stub_file) + ": stub symbol not needed: " + name);
           else
-            LLVM_DEBUG(llvm::dbgs() << "stub symbol not needed: `" << name << "`\n");
-          continue;
+            LLVM_DEBUG(llvm::dbgs()
+                       << "stub symbol not needed: `" << name << "`\n");
         }
-        // The first stub library to define a given symbol sets this and
-        // definitions in later stub libraries are ignored.
-        if (sym->forceImport)
-          continue;  // Already handled
-        sym->forceImport = true;
-        if (sym->traced)
-          message(toString(stub_file) + ": importing " + name);
-        else
-          LLVM_DEBUG(llvm::dbgs()
-                     << toString(stub_file) << ": importing " << name << "\n");
-        for (const auto dep : deps) {
-          auto* needed = symtab->find(dep);
-          if (!needed) {
-            error(toString(stub_file) + ": undefined symbol: " + dep +
-                  ". Required by " + toString(*sym));
-          } else if (needed->isUndefined()) {
-            error(toString(stub_file) +
-                  ": undefined symbol: " + toString(*needed) +
-                  ". Required by " + toString(*sym));
-          } else {
-            if (needed->traced)
-              message(toString(stub_file) + ": exported " + toString(*needed) +
-                      " due to import of " + name);
-            else
-              LLVM_DEBUG(llvm::dbgs()
-                         << "force export: " << toString(*needed) << "\n");
-            needed->forceExport = true;
-            if (auto *lazy = dyn_cast<LazySymbol>(needed)) {
-              depsAdded = true;
-              lazy->extract();
-              if (!config->whyExtract.empty())
-                ctx.whyExtractRecords.emplace_back(stub_file->getName(),
-                                                   sym->getFile(), *sym);
-            }
+      }
+
+      // Secondly looks for any symbols with an `importName` that matches
+      for (Symbol *sym : symtab->symbols()) {
+        if (sym->isUndefined() && sym->importName.has_value()) {
+          auto it = stub_file->symbolDependencies.find(sym->importName.value());
+          if (it != stub_file->symbolDependencies.end()) {
+            depsAdded |= addStubSymbolDeps(stub_file, sym, it->second);
           }
         }
       }

@sbc100 sbc100 merged commit afb3f7e into llvm:main Jun 20, 2024
11 checks passed
@sbc100 sbc100 deleted the fix_stub_import_names branch June 20, 2024 16:48
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
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