[lld] [lld][WebAssembly] Handle stub symbol dependencies when an explicit import name is used (PR #80169)
Sam Clegg via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 19 16:45:12 PDT 2024
https://github.com/sbc100 updated https://github.com/llvm/llvm-project/pull/80169
>From 5cbb61e59f87058377eec3252abae5232c2d7035 Mon Sep 17 00:00:00 2001
From: Sam Clegg <sbc at chromium.org>
Date: Wed, 19 Jun 2024 15:34:55 -0700
Subject: [PATCH 1/2] [lld][WebAssembly] Allow `--trace-symbol` to work with
symbols with custom import names
---
lld/test/wasm/trace-symbol.s | 14 +++++++++++++-
lld/wasm/SymbolTable.cpp | 37 ++++++++++++++++++++++++++++++------
lld/wasm/SymbolTable.h | 4 ++++
3 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/lld/test/wasm/trace-symbol.s b/lld/test/wasm/trace-symbol.s
index 88e5c6f5829e3..3d79fa96f0942 100644
--- a/lld/test/wasm/trace-symbol.s
+++ b/lld/test/wasm/trace-symbol.s
@@ -1,19 +1,28 @@
+# Test -y symbol and -trace-symbol=symbol
+
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/ret32.s -o %t.ret32.o
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.start.o %s
# RUN: wasm-ld -o %t.wasm %t.start.o %t.ret32.o -y ret32 -y _start | FileCheck %s -check-prefix=BOTH
# RUN: wasm-ld -o %t.wasm %t.ret32.o %t.start.o -y ret32 -y _start | FileCheck %s -check-prefix=REVERSED
-# check alias
+# check long argument form
# RUN: wasm-ld -o %t.wasm %t.start.o %t.ret32.o -trace-symbol=_start | FileCheck %s -check-prefixes=JUST-START
+# check import names can be traced
+# RUN: wasm-ld -o %t.wasm %t.start.o %t.ret32.o -trace-symbol=foo_external | FileCheck %s -check-prefixes=IMPORT-NAME
+
.functype ret32 (f32) -> (i32)
+.functype foo_import () -> ()
+.import_name foo_import, foo_external
+
.globl _start
_start:
.functype _start () -> ()
f32.const 0.0
call ret32
drop
+ call foo_import
end_function
# BOTH: start.o: definition of _start
@@ -26,3 +35,6 @@ _start:
# JUST-START: start.o: definition of _start
# JUST-START-NOT: ret32
+
+# IMPORT-NAME: start.o: reference to foo_external
+# IMPORT-NAME-NOT: ret32
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index 00c347ea3ef24..65f394f14bb30 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -507,6 +507,15 @@ static void setImportAttributes(T *existing,
}
}
+static void traceImport(std::optional<StringRef> importName, InputFile *file) {
+ if (importName.has_value()) {
+ auto name = importName.value();
+ if (symtab->isTraced(name)) {
+ printTraceSymbolUndefined(name, file);
+ }
+ }
+}
+
Symbol *SymbolTable::addUndefinedFunction(StringRef name,
std::optional<StringRef> importName,
std::optional<StringRef> importModule,
@@ -526,6 +535,7 @@ Symbol *SymbolTable::addUndefinedFunction(StringRef name,
printTraceSymbolUndefined(name, file);
auto replaceSym = [&]() {
+ traceImport(importName, file);
replaceSymbol<UndefinedFunction>(s, name, importName, importModule, flags,
file, sig, isCalledDirectly);
};
@@ -560,6 +570,7 @@ Symbol *SymbolTable::addUndefinedFunction(StringRef name,
replaceSym();
}
if (existingUndefined) {
+ traceImport(importName, file);
setImportAttributes(existingUndefined, importName, importModule, flags,
file);
if (isCalledDirectly)
@@ -612,10 +623,11 @@ Symbol *SymbolTable::addUndefinedGlobal(StringRef name,
if (s->traced)
printTraceSymbolUndefined(name, file);
- if (wasInserted)
+ if (wasInserted) {
+ traceImport(importName, file);
replaceSymbol<UndefinedGlobal>(s, name, importName, importModule, flags,
file, type);
- else if (auto *lazy = dyn_cast<LazySymbol>(s))
+ } else if (auto *lazy = dyn_cast<LazySymbol>(s))
lazy->extract();
else if (s->isDefined())
checkGlobalType(s, file, type);
@@ -638,10 +650,11 @@ Symbol *SymbolTable::addUndefinedTable(StringRef name,
if (s->traced)
printTraceSymbolUndefined(name, file);
- if (wasInserted)
+ if (wasInserted) {
+ traceImport(importName, file);
replaceSymbol<UndefinedTable>(s, name, importName, importModule, flags,
file, type);
- else if (auto *lazy = dyn_cast<LazySymbol>(s))
+ } else if (auto *lazy = dyn_cast<LazySymbol>(s))
lazy->extract();
else if (s->isDefined())
checkTableType(s, file, type);
@@ -664,10 +677,11 @@ Symbol *SymbolTable::addUndefinedTag(StringRef name,
if (s->traced)
printTraceSymbolUndefined(name, file);
- if (wasInserted)
+ if (wasInserted) {
+ traceImport(importName, file);
replaceSymbol<UndefinedTag>(s, name, importName, importModule, flags, file,
sig);
- else if (auto *lazy = dyn_cast<LazySymbol>(s))
+ } else if (auto *lazy = dyn_cast<LazySymbol>(s))
lazy->extract();
else if (s->isDefined())
checkTagType(s, file, sig);
@@ -828,9 +842,20 @@ bool SymbolTable::getFunctionVariant(Symbol* sym, const WasmSignature *sig,
// Set a flag for --trace-symbol so that we can print out a log message
// if a new symbol with the same name is inserted into the symbol table.
void SymbolTable::trace(StringRef name) {
+ tracingEnabled = true;
symMap.insert({CachedHashStringRef(name), -1});
}
+bool SymbolTable::isTraced(StringRef name) {
+ // Early exit in the default case to avoid symbol hashmap lookup.
+ if (!tracingEnabled)
+ return false;
+ auto it = symMap.find(CachedHashStringRef(name));
+ if (it == symMap.end())
+ return false;
+ return it->second == -1 || symVector[it->second]->traced;
+}
+
void SymbolTable::wrap(Symbol *sym, Symbol *real, Symbol *wrap) {
// Swap symbols as instructed by -wrap.
int &origIdx = symMap[CachedHashStringRef(sym->getName())];
diff --git a/lld/wasm/SymbolTable.h b/lld/wasm/SymbolTable.h
index 42ebb8be8eb3f..f631b10961206 100644
--- a/lld/wasm/SymbolTable.h
+++ b/lld/wasm/SymbolTable.h
@@ -50,6 +50,8 @@ class SymbolTable {
void trace(StringRef name);
+ bool isTraced(StringRef name);
+
Symbol *addDefinedFunction(StringRef name, uint32_t flags, InputFile *file,
InputFunction *function);
Symbol *addDefinedData(StringRef name, uint32_t flags, InputFile *file,
@@ -132,6 +134,8 @@ class SymbolTable {
// For LTO.
std::unique_ptr<BitcodeCompiler> lto;
+
+ bool tracingEnabled = false;
};
extern SymbolTable *symtab;
>From c5373a2fd6874e19cfc72b058eea25a44b6cfa2d Mon Sep 17 00:00:00 2001
From: Sam Clegg <sbc at chromium.org>
Date: Sun, 28 Jan 2024 10:19:49 -0800
Subject: [PATCH 2/2] [lld][WebAssembly] Handle stub symbol dependencies when
an exlicit import name is used
---
lld/test/wasm/Inputs/libstub.so | 2 +-
lld/test/wasm/stub-library-archive.s | 1 +
lld/test/wasm/stub-library.s | 5 +-
lld/wasm/Driver.cpp | 94 +++++++++++++++++-----------
4 files changed, 60 insertions(+), 42 deletions(-)
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);
}
}
}
More information about the llvm-commits
mailing list