Bug 1620197 - Enable multiple results from WebAssembly functions r=lth

This patch enables multi-value calls and returns, adding some tests, and conditionally disabling a couple expect-fail reftests that now pass. It also changes to make calling multi-result WebAssembly functions from JavaScript raise a run-time error, and likewise for calling out to multi-result JS function from wasm. Previously these would abort at stub generation time. Differential Revision: https://phabricator.services.mozilla.com/D65502
Brli · Mar 9, 2020 · c293160 · c293160
1 parent 8af4298
commit c293160
Show file tree

Hide file tree

Showing 17 changed files with 317 additions and 58 deletions.
diff --git a/js/src/jit-test/etc/wasm-spec-tests.patch b/js/src/jit-test/etc/wasm-spec-tests.patch
@@ -17,3 +17,25 @@ index 15518b4d7f60..9184b46010a4 100644
  // type.wast:52
  assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x86\x80\x80\x80\x00\x01\x60\x00\x02\x7f\x7f");
 
+diff --git a/js/src/jit-test/tests/wasm/spec/func.wast.js b/js/src/jit-test/tests/wasm/spec/func.wast.js
+index 9e3c1f09429f..dc86e5aceb3c 100644
+--- a/js/src/jit-test/tests/wasm/spec/func.wast.js
++++ b/js/src/jit-test/tests/wasm/spec/func.wast.js
+@@ -278,11 +278,17 @@ assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x85\x80\x80\x80\x00\x01\x60
+ // func.wast:484
+ assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x86\x80\x80\x80\x00\x01\x60\x02\x7c\x7e\x00\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x8b\x80\x80\x80\x00\x01\x85\x80\x80\x80\x00\x00\x20\x01\x9a\x0b");
+
++// These two tests check that function types returning more than 2
++// results are invalid, but with multi-value of course that's no longer
++// the case.  Until the feature fully lands and we can import from
++// upstream, skip these tests.
++if (!wasmMultiValueEnabled()) {
+ // func.wast:492
+ assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x86\x80\x80\x80\x00\x01\x60\x00\x02\x7f\x7f\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x89\x80\x80\x80\x00\x01\x83\x80\x80\x80\x00\x00\x00\x0b");
+
+ // func.wast:496
+ assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x86\x80\x80\x80\x00\x01\x60\x00\x02\x7f\x7f\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x89\x80\x80\x80\x00\x01\x83\x80\x80\x80\x00\x00\x00\x0b");
++}
+
+ // func.wast:505
+ assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x85\x80\x80\x80\x00\x01\x60\x00\x01\x7f\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x88\x80\x80\x80\x00\x01\x82\x80\x80\x80\x00\x00\x0b");
diff --git a/js/src/jit-test/tests/wasm/multi-value/call-js.js b/js/src/jit-test/tests/wasm/multi-value/call-js.js
@@ -0,0 +1,23 @@
+
+function expectRunFailure(text, pattern, imports) {
+    let instance = wasmEvalText(text, imports);
+    assertErrorMessage(() => instance.exports.run(),
+		       TypeError,
+                       pattern);
+}
+
+expectRunFailure(`
+  (module
+    (import "env" "f" (func $f (result i32 i32)))
+    (func (export "run") (result i32)
+      (call $f)
+      i32.sub))`,
+                 /calling JavaScript functions with multiple results from WebAssembly not yet implemented/,
+                 { env: { f: () => [52, 10] } });
+
+expectRunFailure(`
+  (module
+    (func (export "run") (result i32 i32)
+      (i32.const 52)
+      (i32.const 10)))`,
+                 /calling WebAssembly functions with multiple results from JavaScript not yet implemented/);
diff --git a/js/src/jit-test/tests/wasm/multi-value/call-ref.js b/js/src/jit-test/tests/wasm/multi-value/call-ref.js
@@ -0,0 +1,31 @@
+// |jit-test| skip-if: !wasmReftypesEnabled()
+
+let counter = 0;
+let i = new WebAssembly.Instance(
+    new WebAssembly.Module(wasmTextToBinary(`
+      (module
+        (func $boxNextInt (import "imports" "boxNextInt")
+          (result anyref))
+        (func $unboxInt (import "imports" "unboxInt")
+          (param anyref) (result i32))
+
+        (func $boxNextTwoInts (result anyref anyref)
+          call $boxNextInt
+          call $boxNextInt)
+
+        (func $unboxTwoInts (param anyref anyref) (result i32 i32)
+          local.get 0
+          call $unboxInt
+          local.get 1
+          call $unboxInt)
+
+        (func $addNextTwoInts (export "addNextTwoInts") (result i32)
+          call $boxNextTwoInts
+          call $unboxTwoInts
+          i32.add))`)),
+    {imports: {boxNextInt: () => ({val: counter++}),
+               unboxInt: box => box.val}});
+
+for (let n = 0; n < 200000; n += 2) {
+    assertEq(i.exports.addNextTwoInts(), n * 2 + 1);
+}
diff --git a/js/src/jit-test/tests/wasm/multi-value/call-run.js b/js/src/jit-test/tests/wasm/multi-value/call-run.js
@@ -0,0 +1,120 @@
+wasmFullPass(`
+  (module
+    (func (result i32 i32)
+      (i32.const 52)
+      (i32.const 10))
+    (func (export "run") (result i32)
+      (call 0)
+      i32.sub))`,
+            42);
+
+wasmFullPass(`
+  (module
+    (func (param i32 i32) (result i32 i32)
+      (local.get 0)
+      (local.get 1))
+    (func (export "run") (result i32)
+      (i32.const 52)
+      (i32.const 10)
+      (call 0)
+      i32.sub))`,
+            42);
+
+wasmFullPass(`
+  (module
+    (func (param i32 i32 i32 i32 i32
+                 i32 i32 i32 i32 i32)
+          (result i32 i32)
+      (local.get 8)
+      (local.get 9))
+    (func (export "run") (result i32)
+      (i32.const 0)
+      (i32.const 1)
+      (i32.const 2)
+      (i32.const 3)
+      (i32.const 4)
+      (i32.const 5)
+      (i32.const 6)
+      (i32.const 7)
+      (i32.const 52)
+      (i32.const 10)
+      (call 0)
+      i32.sub))`,
+            42);
+
+wasmFullPass(`
+  (module
+    (func (param i32 i32 i32 i32 i32
+                 i32 i32 i32 i32 i32)
+          (result i32 i32)
+      (local i32 i32 i32 i32)
+      (local.get 8)
+      (local.get 9))
+    (func (export "run") (result i32)
+      (i32.const 0)
+      (i32.const 1)
+      (i32.const 2)
+      (i32.const 3)
+      (i32.const 4)
+      (i32.const 5)
+      (i32.const 6)
+      (i32.const 7)
+      (i32.const 52)
+      (i32.const 10)
+      (call 0)
+      i32.sub))`,
+            42);
+
+wasmFullPass(`
+  (module
+    (func (param i32 i32 i32 i32 i32
+                 i32 i32 i32 i32 i32)
+          (result i32 i32 i32)
+      (local i32 i32 i32 i32)
+      (local.get 7)
+      (local.get 8)
+      (local.get 9))
+    (func (export "run") (result i32)
+      (i32.const 0)
+      (i32.const 1)
+      (i32.const 2)
+      (i32.const 3)
+      (i32.const 4)
+      (i32.const 5)
+      (i32.const 6)
+      (i32.const 7)
+      (i32.const 52)
+      (i32.const 10)
+      (call 0)
+      i32.sub
+      i32.sub))`,
+            -35);
+
+wasmFullPass(`
+  (module
+    (func (param i32 i64 i32 i64 i32
+                 i64 i32 i64 i32 i64)
+          (result i64 i32 i64)
+      (local i32 i64 i32 i64)
+      (local.get 7)
+      (local.get 8)
+      (local.get 9))
+    (func (export "run") (result i32)
+      (i32.const 0)
+      (i64.const 1)
+      (i32.const 2)
+      (i64.const 3)
+      (i32.const 4)
+      (i64.const 5)
+      (i32.const 6)
+      (i64.const 7)
+      (i32.const 52)
+      (i64.const 10)
+      (call 0)
+      i32.wrap_i64
+      i32.sub
+      i64.extend_i32_s
+      i64.sub
+      i32.wrap_i64))`,
+            -35);
+
diff --git a/js/src/jit-test/tests/wasm/multi-value/call-validate.js b/js/src/jit-test/tests/wasm/multi-value/call-validate.js
@@ -0,0 +1,47 @@
+wasmValidateText(`
+  (module
+    (func (result i32 i32)
+      (i32.const 32)
+      (i32.const 10)))`);
+
+wasmValidateText(`
+  (module
+    (type $t (func (result i32 i32)))
+    (func (type $t)
+      (i32.const 32)
+      (i32.const 10)))`);
+
+wasmValidateText(`
+  (module
+    (func (result i32 i32)
+      (block (result i32 i32)
+        (i32.const 32)
+        (i32.const 10))))`);
+
+wasmValidateText(`
+  (module
+    (func $return-2 (result i32 i32)
+      (i32.const 32)
+      (i32.const 10))
+    (func $tail-call (result i32 i32)
+      (call 0)))`);
+
+wasmValidateText(`
+  (module
+    (func $return-2 (result i32 i32)
+      (i32.const 32)
+      (i32.const 10))
+    (func $add (result i32)
+      (call 0)
+      i32.add))`);
+
+wasmValidateText(`
+  (module
+    (func $return-2 (param i32 i32) (result i32 i32)
+      (local.get 0)
+      (local.get 1))
+    (func (export "run") (result i32)
+      (i32.const 32)
+      (i32.const 10)
+      (call 0)
+      i32.add))`);
diff --git a/js/src/jit-test/tests/wasm/spec/func.wast.js b/js/src/jit-test/tests/wasm/spec/func.wast.js
@@ -278,11 +278,17 @@ assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x85\x80\x80\x80\x00\x01\x60
 // func.wast:484
 assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x86\x80\x80\x80\x00\x01\x60\x02\x7c\x7e\x00\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x8b\x80\x80\x80\x00\x01\x85\x80\x80\x80\x00\x00\x20\x01\x9a\x0b");
 
+// These two tests check that function types returning more than 2
+// results are invalid, but with multi-value of course that's no longer
+// the case.  Until the feature fully lands and we can import from
+// upstream, skip these tests.
+if (!wasmMultiValueEnabled()) {
 // func.wast:492
 assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x86\x80\x80\x80\x00\x01\x60\x00\x02\x7f\x7f\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x89\x80\x80\x80\x00\x01\x83\x80\x80\x80\x00\x00\x00\x0b");
 
 // func.wast:496
 assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x86\x80\x80\x80\x00\x01\x60\x00\x02\x7f\x7f\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x89\x80\x80\x80\x00\x01\x83\x80\x80\x80\x00\x00\x00\x0b");
+}
 
 // func.wast:505
 assert_invalid("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x85\x80\x80\x80\x00\x01\x60\x00\x01\x7f\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x88\x80\x80\x80\x00\x01\x82\x80\x80\x80\x00\x00\x0b");

diff --git a/js/src/js.msg b/js/src/js.msg
@@ -435,6 +435,8 @@ MSG_DEF(JSMSG_WASM_TEXT_FAIL,          1, JSEXN_SYNTAXERR,   "wasm text error: {
 MSG_DEF(JSMSG_WASM_MISSING_MAXIMUM,    0, JSEXN_TYPEERR,     "'shared' is true but maximum is not specified")
 MSG_DEF(JSMSG_WASM_GLOBAL_IMMUTABLE,   0, JSEXN_TYPEERR,     "can't set value of immutable global")
 MSG_DEF(JSMSG_WASM_NULL_REQUIRED,      0, JSEXN_TYPEERR,     "nullref requires a null value")
+MSG_DEF(JSMSG_WASM_MULTIPLE_RESULT_EXIT_UNIMPLEMENTED, 0, JSEXN_TYPEERR, "calling JavaScript functions with multiple results from WebAssembly not yet implemented")
+MSG_DEF(JSMSG_WASM_MULTIPLE_RESULT_ENTRY_UNIMPLEMENTED, 0, JSEXN_TYPEERR, "calling WebAssembly functions with multiple results from JavaScript not yet implemented")
 
 // Proxy
 MSG_DEF(JSMSG_BAD_TRAP_RETURN_VALUE,   2, JSEXN_TYPEERR,"trap {1} for {0} returned a primitive value")

diff --git a/js/src/wasm/WasmBaselineCompile.cpp b/js/src/wasm/WasmBaselineCompile.cpp
@@ -1738,7 +1738,7 @@ class BaseStackFrame final : public BaseStackFrameAllocator {
                                          RegPtr dest) {
     MOZ_ASSERT(results.height() <= masm.framePushed());
     uint32_t offsetFromSP = masm.framePushed() - results.height();
-    masm.movePtr(AsRegister(sp_), dest);
+    masm.moveStackPtrTo(dest);
     if (offsetFromSP) {
       masm.addPtr(Imm32(offsetFromSP), dest);
     }

diff --git a/js/src/wasm/WasmCode.h b/js/src/wasm/WasmCode.h
@@ -239,6 +239,7 @@ class FuncExport {
 
   bool canHaveJitEntry() const {
     return !funcType_.temporarilyUnsupportedReftypeForEntry() &&
+           !funcType_.temporarilyUnsupportedResultCountForEntry() &&
            JitOptions.enableWasmJitEntry;
   }
 

diff --git a/js/src/wasm/WasmConstants.h b/js/src/wasm/WasmConstants.h
@@ -603,11 +603,6 @@ static const unsigned MaxBrTableElems = 1000000;
 static const unsigned MaxMemoryInitialPages = 16384;
 static const unsigned MaxCodeSectionBytes = MaxModuleBytes;
 
-// FIXME: Temporary limit to function result counts.  Replace with MaxResults:
-// bug 1585909.
-
-static const unsigned MaxFuncResults = 1;
-
 // A magic value of the FramePointer to indicate after a return to the entry
 // stub that an exception has been caught and that we should throw.
 

diff --git a/js/src/wasm/WasmInstance.cpp b/js/src/wasm/WasmInstance.cpp
@@ -123,6 +123,12 @@ bool Instance::callImport(JSContext* cx, uint32_t funcImportIndex,
     return false;
   }
 
+  if (fi.funcType().temporarilyUnsupportedResultCountForExit()) {
+    JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
+                             JSMSG_WASM_MULTIPLE_RESULT_EXIT_UNIMPLEMENTED);
+    return false;
+  }
+
   MOZ_ASSERT(fi.funcType().args().length() == argc);
   for (size_t i = 0; i < argc; i++) {
     switch (fi.funcType().args()[i].kind()) {
@@ -1698,6 +1704,12 @@ bool Instance::callExport(JSContext* cx, uint32_t funcIndex, CallArgs args) {
     return false;
   }
 
+  if (funcType->temporarilyUnsupportedResultCountForEntry()) {
+    JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
+                             JSMSG_WASM_MULTIPLE_RESULT_ENTRY_UNIMPLEMENTED);
+    return false;
+  }
+
   // The calling convention for an external call into wasm is to pass an
   // array of 16-byte values where each value contains either a coerced int32
   // (in the low word), or a double value (in the low dword) value, with the

diff --git a/js/src/wasm/WasmOpIter.h b/js/src/wasm/WasmOpIter.h
@@ -2061,12 +2061,6 @@ inline bool OpIter<Policy>::readCallIndirect(uint32_t* funcTypeIndex,
 
   const FuncType& funcType = env_.types[*funcTypeIndex].funcType();
 
-  // FIXME: Remove this check when full multi-value function returns land.
-  // Bug 1585909.
-  if (funcType.results().length() > MaxFuncResults) {
-    return fail("too many returns in signature");
-  }
-
 #ifdef WASM_PRIVATE_REFTYPES
   if (env_.tables[*tableIndex].importedOrExported &&
       funcType.exposesTypeIndex()) {
@@ -2131,12 +2125,6 @@ inline bool OpIter<Policy>::readOldCallIndirect(uint32_t* funcTypeIndex,
 
   const FuncType& funcType = env_.types[*funcTypeIndex].funcType();
 
-  // FIXME: Remove this check when full multi-value function returns land.
-  // Bug 1585909.
-  if (funcType.results().length() > MaxFuncResults) {
-    return fail("too many returns in signature");
-  }
-
   if (!popCallArgs(funcType.args(), argValues)) {
     return false;
   }