[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

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.

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 () -> ()

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;

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;
   }
 }
 

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">;

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;
     }
   }
 }

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;