[lld] 758633f - [lld][WebAssembly] Add new `--import-undefined` option
Sam Clegg via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 17 11:51:08 PDT 2021
Author: Sam Clegg
Date: 2021-06-17T11:44:21-07:00
New Revision: 758633f92226ad25c9007a8a05f01cb848b97ab8
URL: https://github.com/llvm/llvm-project/commit/758633f92226ad25c9007a8a05f01cb848b97ab8
DIFF: https://github.com/llvm/llvm-project/commit/758633f92226ad25c9007a8a05f01cb848b97ab8.diff
LOG: [lld][WebAssembly] Add new `--import-undefined` option
This change revisits https://reviews.llvm.org/D79248 which originally
added support for the --unresolved-symbols flag.
At the time I thought it would make sense to add a third option to this
flag called `import-functions` but it turns out (as was suspects by on
the reviewers IIRC) that this option can be authoganal.
Instead I've added a new option called `--import-undefined` that only
operates on symbols that can be imported (for example, function symbols
can always be imported as opposed to data symbols we can only be
imported when compiling with PIC).
This option gives us the full expresivitiy that emscripten needs to be
able allow reporting of undefined data symbols as well as the option to
disable that.
This change does remove the `--unresolved-symbols=import-functions`
option, which is been in the codebase now for about a year but I would
be extremely surprised if anyone was using it.
Differential Revision: https://reviews.llvm.org/D103290
Added:
Modified:
lld/docs/WebAssembly.rst
lld/test/wasm/unresolved-symbols.s
lld/wasm/Config.h
lld/wasm/Driver.cpp
lld/wasm/Options.td
lld/wasm/Relocations.cpp
lld/wasm/Writer.cpp
Removed:
################################################################################
diff --git a/lld/docs/WebAssembly.rst b/lld/docs/WebAssembly.rst
index 749f580d365c1..c01df99cddb96 100644
--- a/lld/docs/WebAssembly.rst
+++ b/lld/docs/WebAssembly.rst
@@ -72,7 +72,8 @@ WebAssembly-specific options:
.. option:: --allow-undefined
Allow undefined symbols in linked binary. This is the legacy
- flag which corresponds to ``--unresolved-symbols=import-functions``.
+ flag which corresponds to ``--unresolve-symbols=ignore`` +
+ ``--import-undefined``.
.. option:: --unresolved-symbols=<method>
@@ -91,16 +92,17 @@ WebAssembly-specific options:
this is trivial. For direct function calls, the linker will generate a
trapping stub function in place of the undefined function.
- import-functions:
-
- Generate WebAssembly imports for any undefined functions. Undefined data
- symbols are resolved to zero as in ``ignore-all``. This corresponds to
- the legacy ``--allow-undefined`` flag.
-
.. option:: --import-memory
Import memory from the environment.
+.. option:: --import-undefined
+
+ Generate WebAssembly imports for undefined symbols, where possible. For
+ example, for function symbols this is always possible, but in general this
+ is not possible for undefined data symbols. Undefined data symbols will
+ still be reported as normal (in accordance with ``--unresolved-symbols``).
+
.. option:: --initial-memory=<value>
Initial size of the linear memory. Default: static data size.
diff --git a/lld/test/wasm/unresolved-symbols.s b/lld/test/wasm/unresolved-symbols.s
index 26079a9641024..56abd3c275fc6 100644
--- a/lld/test/wasm/unresolved-symbols.s
+++ b/lld/test/wasm/unresolved-symbols.s
@@ -1,9 +1,9 @@
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t1.o
-## Check that %t1.o contains undefined symbol undef.
+## Check that %t1.o contains undefined symbol undef_func.
# RUN: not wasm-ld %t1.o -o /dev/null 2>&1 | \
# RUN: FileCheck -check-prefix=ERRUND %s
-# ERRUND: error: {{.*}}1.o: undefined symbol: undef
+# ERRUND: error: {{.*}}1.o: undefined symbol: undef_func
## report-all is the default one. Check that we get the same error
# RUN: not wasm-ld %t1.o -o /dev/null --unresolved-symbols=report-all 2>&1 | \
@@ -52,19 +52,25 @@
# IGNORE-NEXT: - Index: 3
# IGNORE-NEXT: Name: get_func_addr
-## import-functions should not produce errors and should resolve in
-# imports for the missing functions but not the missing data symbols.
-# `--allow-undefined` should behave exactly the same.
-# RUN: wasm-ld %t1.o -o %t3.wasm --unresolved-symbols=import-functions
+## --import-undefined should handle unresolved funtions symbols
+# by importing them but still report errors/warning for missing data symbols.
+# `--allow-undefined` should behave like `--import-undefined` +
+# `--unresolve-symbols=ignore`
+# RUN: wasm-ld %t1.o -o %t3.wasm --import-undefined --unresolved-symbols=ignore-all
# RUN: obj2yaml %t3.wasm | FileCheck -check-prefix=IMPORT %s
# IMPORT: - Type: IMPORT
# IMPORT-NEXT: Imports:
# IMPORT-NEXT: - Module: env
-# IMPORT-NEXT: Field: undef
+# IMPORT-NEXT: Field: undef_func
# IMPORT-NEXT: Kind: FUNCTION
# IMPORT-NEXT: SigIndex: 0
# IMPORT-NEXT: - Type: FUNCTION
+## Check that --import-undefined reports unresolved data symbols.
+# RUN: not wasm-ld %t1.o -o %t3.wasm --import-undefined --unresolved-symbols=report-all 2>&1 | FileCheck -check-prefix=IMPORTUNDEFINED %s
+# IMPORTUNDEFINED-NOT: error: {{.*}}1.o: undefined symbol: undef_func
+# IMPORTUNDEFINED: error: {{.*}}1.o: undefined symbol: undef_data
+
## Do not report undefines if linking relocatable.
# RUN: wasm-ld -r %t1.o -o %t4.wasm --unresolved-symbols=report-all
# RUN: llvm-readobj %t4.wasm > /dev/null 2>&1
@@ -72,7 +78,7 @@
.globl _start
_start:
.functype _start () -> ()
- call undef
+ call undef_func
call get_data_addr
call get_func_addr
end_function
@@ -87,8 +93,8 @@ get_data_addr:
.globl get_func_addr
get_func_addr:
.functype get_func_addr () -> (i32)
- i32.const undef
+ i32.const undef_func
return
end_function
-.functype undef () -> ()
+.functype undef_func () -> ()
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index 796610e5e72b7..7b20a6b03854f 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -18,10 +18,7 @@ namespace lld {
namespace wasm {
// For --unresolved-symbols.
-// The `ImportFuncs` mode is an additional mode that corresponds to the
-// --allow-undefined flag which turns undefined functions in imports
-// as opposed ed to Ignore or Warn which turn them into unreachables.
-enum class UnresolvedPolicy { ReportError, Warn, Ignore, ImportFuncs };
+enum class UnresolvedPolicy { ReportError, Warn, Ignore };
// This struct contains the global configuration for the linker.
// Most fields are direct mapping from the command line options
@@ -43,6 +40,7 @@ struct Configuration {
bool importMemory;
bool sharedMemory;
bool importTable;
+ bool importUndefined;
llvm::Optional<bool> is64;
bool mergeDataSegments;
bool pie;
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 61eeb0bd25221..c5db63e3507ec 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -344,18 +344,11 @@ static UnresolvedPolicy getUnresolvedSymbolPolicy(opt::InputArgList &args) {
StringRef s = arg->getValue();
if (s == "ignore-all")
return UnresolvedPolicy::Ignore;
- if (s == "import-functions")
- return UnresolvedPolicy::ImportFuncs;
if (s == "report-all")
return errorOrWarn;
error("unknown --unresolved-symbols value: " + s);
}
- // Legacy --allow-undefined flag which is equivalent to
- // --unresolve-symbols=ignore-all
- if (args.hasArg(OPT_allow_undefined))
- return UnresolvedPolicy::ImportFuncs;
-
return errorOrWarn;
}
@@ -378,6 +371,7 @@ static void readConfigs(opt::InputArgList &args) {
config->importMemory = args.hasArg(OPT_import_memory);
config->sharedMemory = args.hasArg(OPT_shared_memory);
config->importTable = args.hasArg(OPT_import_table);
+ config->importUndefined = args.hasArg(OPT_import_undefined);
config->ltoo = args::getInteger(args, OPT_lto_O, 2);
config->ltoPartitions = args::getInteger(args, OPT_lto_partitions, 1);
config->ltoNewPassManager =
@@ -453,6 +447,13 @@ static void readConfigs(opt::InputArgList &args) {
config->features->push_back(std::string(s));
}
+ // Legacy --allow-undefined flag which is equivalent to
+ // --unresolve-symbols=ignore + --import-undefined
+ if (args.hasArg(OPT_allow_undefined)) {
+ config->importUndefined = true;
+ config->unresolvedSymbols = UnresolvedPolicy::Ignore;
+ }
+
if (args.hasArg(OPT_print_map))
config->mapFile = "-";
}
@@ -481,7 +482,8 @@ static void setConfigs() {
if (config->shared) {
config->importMemory = true;
- config->unresolvedSymbols = UnresolvedPolicy::ImportFuncs;
+ config->importUndefined = true;
+ config->unresolvedSymbols = UnresolvedPolicy::Ignore;
}
}
diff --git a/lld/wasm/Options.td b/lld/wasm/Options.td
index 6b4c2f4baf313..40cb314c4ab09 100644
--- a/lld/wasm/Options.td
+++ b/lld/wasm/Options.td
@@ -141,7 +141,11 @@ def z: JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,
// The follow flags are unique to wasm
def allow_undefined: F<"allow-undefined">,
- HelpText<"Allow undefined symbols in linked binary">;
+ HelpText<"Allow undefined symbols in linked binary. This options is equivelant "
+ "to --import-undefined and --unresolved-symbols=ignore-all">;
+
+def import_undefined: F<"import-undefined">,
+ HelpText<"Turn undefined symbols into imports where possible">;
def allow_undefined_file: J<"allow-undefined-file=">,
HelpText<"Allow symbols listed in <file> to be undefined in linked binary">;
diff --git a/lld/wasm/Relocations.cpp b/lld/wasm/Relocations.cpp
index eedd7d7e1be2c..bb2019a6cce58 100644
--- a/lld/wasm/Relocations.cpp
+++ b/lld/wasm/Relocations.cpp
@@ -35,7 +35,7 @@ static bool allowUndefined(const Symbol* sym) {
// Undefined functions and globals with explicit import name are allowed to be
// undefined at link time.
if (auto *f = dyn_cast<UndefinedFunction>(sym))
- if (f->importName)
+ if (f->importName || config->importUndefined)
return true;
if (auto *g = dyn_cast<UndefinedGlobal>(sym))
if (g->importName)
@@ -56,20 +56,20 @@ static void reportUndefined(Symbol *sym) {
warn(toString(sym->getFile()) + ": undefined symbol: " + toString(*sym));
break;
case UnresolvedPolicy::Ignore:
- if (auto *f = dyn_cast<UndefinedFunction>(sym)) {
- if (!f->stubFunction) {
- LLVM_DEBUG(dbgs()
- << "ignoring undefined symbol: " + toString(*sym) + "\n");
- f->stubFunction = symtab->createUndefinedStub(*f->getSignature());
- f->stubFunction->markLive();
- // Mark the function itself as a stub which prevents it from being
- // assigned a table entry.
- f->isStub = true;
+ LLVM_DEBUG(dbgs() << "ignoring undefined symbol: " + toString(*sym) +
+ "\n");
+ if (!config->importUndefined) {
+ if (auto *f = dyn_cast<UndefinedFunction>(sym)) {
+ if (!f->stubFunction) {
+ f->stubFunction = symtab->createUndefinedStub(*f->getSignature());
+ f->stubFunction->markLive();
+ // Mark the function itself as a stub which prevents it from being
+ // assigned a table entry.
+ f->isStub = true;
+ }
}
}
break;
- case UnresolvedPolicy::ImportFuncs:
- break;
}
}
}
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index ce6171519202e..5512e5dcb0892 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -558,8 +558,7 @@ static bool shouldImport(Symbol *sym) {
if (isa<DataSymbol>(sym))
return false;
- if ((config->isPic || config->relocatable) ||
- config->unresolvedSymbols == UnresolvedPolicy::ImportFuncs)
+ if (config->isPic || config->relocatable || config->importUndefined)
return true;
if (config->allowUndefinedSymbols.count(sym->getName()) != 0)
return true;
More information about the llvm-commits
mailing list