[lld] 3c45a06 - [lld][WebAssembly] Allow exporting of mutable globals

Sam Clegg via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 30 17:54:15 PDT 2020


Author: Sam Clegg
Date: 2020-09-30T17:53:27-07:00
New Revision: 3c45a06f26edfb7e94003adf58cb8951ea9c2ce6

URL: https://github.com/llvm/llvm-project/commit/3c45a06f26edfb7e94003adf58cb8951ea9c2ce6
DIFF: https://github.com/llvm/llvm-project/commit/3c45a06f26edfb7e94003adf58cb8951ea9c2ce6.diff

LOG: [lld][WebAssembly] Allow exporting of mutable globals

In particular allow explict exporting of `__stack_pointer` but
exclud this from `--export-all` to avoid requiring the mutable
globals feature whenenve `--export-all` is used.

This uncovered a bug in populateTargetFeatures regarding checking
if the mutable-globals feature is allowed.

See: https://github.com/WebAssembly/binaryen/issues/2934

Differential Revision: https://reviews.llvm.org/D88506

Added: 
    lld/test/wasm/mutable-global-exports.s

Modified: 
    lld/docs/WebAssembly.rst
    lld/test/wasm/mutable-globals.s
    lld/wasm/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/docs/WebAssembly.rst b/lld/docs/WebAssembly.rst
index b23f2cd462b4..bf1f008e608e 100644
--- a/lld/docs/WebAssembly.rst
+++ b/lld/docs/WebAssembly.rst
@@ -39,6 +39,10 @@ WebAssembly-specific options:
 
   Export all symbols (normally combined with --no-gc-sections)
 
+  Note that this will not export linker-generated mutable globals unless
+  the resulting binaryen already includes the 'mutable-globals' features
+  since that would otherwise create and invalid binaryen.
+
 .. option:: --export-dynamic
 
   When building an executable, export any non-hidden symbols.  By default only

diff  --git a/lld/test/wasm/mutable-global-exports.s b/lld/test/wasm/mutable-global-exports.s
new file mode 100644
index 000000000000..e2e45ff93a4b
--- /dev/null
+++ b/lld/test/wasm/mutable-global-exports.s
@@ -0,0 +1,88 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+#
+# Should fail without mutable globals feature enabled.
+# RUN: not wasm-ld --export-all %t.o -o %t.wasm 2>&1 | FileCheck -check-prefix=CHECK-ERR %s
+# RUN: not wasm-ld --export=foo_global %t.o -o %t.wasm 2>&1 | FileCheck -check-prefix=CHECK-ERR %s
+#
+# RUN: wasm-ld --features=mutable-globals --export=foo_global %t.o -o %t.wasm
+# RUN: obj2yaml %t.wasm | FileCheck %s
+
+# Explcitly check that __stack_pointer can be exported
+# RUN: wasm-ld --features=mutable-globals --export=__stack_pointer %t.o -o %t.wasm
+# RUN: obj2yaml %t.wasm | FileCheck -check-prefix=CHECK-SP %s
+
+# RUN: wasm-ld --features=mutable-globals --export-all %t.o -o %t.wasm
+# RUN: obj2yaml %t.wasm | FileCheck -check-prefix=CHECK-ALL %s
+
+
+.globl _start
+.globl foo_global
+
+.globaltype foo_global, i32
+foo_global:
+
+_start:
+  .functype _start () -> ()
+  end_function
+
+# CHECK-ERR: mutable global exported but 'mutable-globals' feature not present in inputs: `foo_global`. Use --no-check-features to suppress
+
+#      CHECK:  - Type:            EXPORT
+# CHECK-NEXT:    Exports:
+# CHECK-NEXT:      - Name:            memory
+# CHECK-NEXT:        Kind:            MEMORY
+# CHECK-NEXT:        Index:           0
+# CHECK-NEXT:      - Name:            _start
+# CHECK-NEXT:        Kind:            FUNCTION
+# CHECK-NEXT:        Index:           0
+# CHECK-NEXT:      - Name:            foo_global
+# CHECK-NEXT:        Kind:            GLOBAL
+# CHECK-NEXT:        Index:           1
+# CHECK-NEXT:  - Type:            CODE
+
+#      CHECK-SP:  - Type:            EXPORT
+# CHECK-SP-NEXT:    Exports:
+# CHECK-SP-NEXT:      - Name:            memory
+# CHECK-SP-NEXT:        Kind:            MEMORY
+# CHECK-SP-NEXT:        Index:           0
+# CHECK-SP-NEXT:      - Name:            __stack_pointer
+# CHECK-SP-NEXT:        Kind:            GLOBAL
+# CHECK-SP-NEXT:        Index:           0
+# CHECK-SP-NEXT:      - Name:            _start
+# CHECK-SP-NEXT:        Kind:            FUNCTION
+# CHECK-SP-NEXT:        Index:           0
+# CHECK-SP-NEXT:  - Type:            CODE
+
+#      CHECK-ALL:  - Type:            EXPORT
+# CHECK-ALL-NEXT:    Exports:
+# CHECK-ALL-NEXT:      - Name:            memory
+# CHECK-ALL-NEXT:        Kind:            MEMORY
+# CHECK-ALL-NEXT:        Index:           0
+# CHECK-ALL-NEXT:      - Name:            __wasm_call_ctors
+# CHECK-ALL-NEXT:        Kind:            FUNCTION
+# CHECK-ALL-NEXT:        Index:           0
+# CHECK-ALL-NEXT:      - Name:            _start
+# CHECK-ALL-NEXT:        Kind:            FUNCTION
+# CHECK-ALL-NEXT:        Index:           1
+# CHECK-ALL-NEXT:      - Name:            foo_global
+# CHECK-ALL-NEXT:        Kind:            GLOBAL
+# CHECK-ALL-NEXT:        Index:           1
+# CHECK-ALL-NEXT:      - Name:            __dso_handle
+# CHECK-ALL-NEXT:        Kind:            GLOBAL
+# CHECK-ALL-NEXT:        Index:           2
+# CHECK-ALL-NEXT:      - Name:            __data_end
+# CHECK-ALL-NEXT:        Kind:            GLOBAL
+# CHECK-ALL-NEXT:        Index:           3
+# CHECK-ALL-NEXT:      - Name:            __global_base
+# CHECK-ALL-NEXT:        Kind:            GLOBAL
+# CHECK-ALL-NEXT:        Index:           4
+# CHECK-ALL-NEXT:      - Name:            __heap_base
+# CHECK-ALL-NEXT:        Kind:            GLOBAL
+# CHECK-ALL-NEXT:        Index:           5
+# CHECK-ALL-NEXT:      - Name:            __memory_base
+# CHECK-ALL-NEXT:        Kind:            GLOBAL
+# CHECK-ALL-NEXT:        Index:           6
+# CHECK-ALL-NEXT:      - Name:            __table_base
+# CHECK-ALL-NEXT:        Kind:            GLOBAL
+# CHECK-ALL-NEXT:        Index:           7
+# CHECK-ALL-NEXT:  - Type:            CODE

diff  --git a/lld/test/wasm/mutable-globals.s b/lld/test/wasm/mutable-globals.s
index ea856e511289..9e8911b02bf2 100644
--- a/lld/test/wasm/mutable-globals.s
+++ b/lld/test/wasm/mutable-globals.s
@@ -1,5 +1,6 @@
 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
 # RUN: not wasm-ld %t.o -o %t.wasm 2>&1 | FileCheck %s
+# RUN: wasm-ld --features=mutable-globals %t.o -o %t.wasm
 
 .globl _start
 _start:

diff  --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 90dd96bd1c89..1d669ca7a723 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -453,7 +453,7 @@ void Writer::populateTargetFeatures() {
   if (!config->checkFeatures)
     return;
 
-  if (!config->relocatable && used.count("mutable-globals") == 0) {
+  if (!config->relocatable && allowed.count("mutable-globals") == 0) {
     for (const Symbol *sym : out.importSec->importedSymbols) {
       if (auto *global = dyn_cast<GlobalSymbol>(sym)) {
         if (global->getGlobalType()->Mutable) {
@@ -571,12 +571,13 @@ void Writer::calculateExports() {
       }
       export_ = {name, WASM_EXTERNAL_FUNCTION, f->getFunctionIndex()};
     } else if (auto *g = dyn_cast<DefinedGlobal>(sym)) {
-      // TODO(sbc): Remove this check once to mutable global proposal is
-      // implement in all major browsers.
-      // See: https://github.com/WebAssembly/mutable-global
-      if (g->getGlobalType()->Mutable) {
-        // Only __stack_pointer and __tls_base should ever be create as mutable.
-        assert(g == WasmSym::stackPointer || g == WasmSym::tlsBase);
+      if (g->getGlobalType()->Mutable && !g->getFile() && !g->forceExport) {
+        // Avoid exporting mutable globals are linker synthesized (e.g.
+        // __stack_pointer or __tls_base) unless they are explicitly exported
+        // from the command line.
+        // Without this check `--export-all` would cause any program using the
+        // stack pointer to export a mutable global even if none of the input
+        // files were built with the `mutable-globals` feature.
         continue;
       }
       export_ = {name, WASM_EXTERNAL_GLOBAL, g->getGlobalIndex()};


        


More information about the llvm-commits mailing list