Changeset 271113 in webkit

Timestamp:

Dec 30, 2020 6:49:48 PM (19 months ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Wasm multivalue should iterate iterable result from JS function first before converting values
​https://bugs.webkit.org/show_bug.cgi?id=220206

Reviewed by Alexey Shvayka.

LayoutTests/imported/w3c:

• web-platform-tests/wasm/jsapi/constructor/multi-value.any-expected.txt:
• web-platform-tests/wasm/jsapi/constructor/multi-value.any.worker-expected.txt:

Source/JavaScriptCore:

When converting JS results to Wasm multivalue (result from JS when executing Wasm->JS calls), we should first iterate all results from iterable.
And then, we should convert each element into Wasm value. Currently, we are converting while iterating, this is not aligned to the spec.

• wasm/WasmOperations.cpp:

(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/constructor/multi-value.any-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/constructor/multi-value.any.worker-expected.txt (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmOperations.cpp (modified) (2 diffs)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2020-12-30  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Wasm multivalue should iterate iterable result from JS function first before converting values
+        https://bugs.webkit.org/show_bug.cgi?id=220206
+
+        Reviewed by Alexey Shvayka.
+
+        * web-platform-tests/wasm/jsapi/constructor/multi-value.any-expected.txt:
+        * web-platform-tests/wasm/jsapi/constructor/multi-value.any.worker-expected.txt:
+
 2020-12-29  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/constructor/multi-value.any-expected.txt

@@ -2 +2 @@
 PASS multiple return values from wasm to js
 PASS multiple return values inside wasm
-FAIL multiple return values from js to wasm assert_array_equals: expected property 6 to be "next call 2" but got "valueOf get 1" (expected array ["@@iterator getter", "@@iterator call", "next getter", "next call 1", "done call 1", "value call 1", "next call 2", "done call 2", "value call 2", "next call 3", "done call 3", "valueOf get 1", "valueOf call 1", "valueOf get 2", "valueOf call 2"] got ["@@iterator getter", "@@iterator call", "next getter", "next call 1", "done call 1", "value call 1", "valueOf get 1", "valueOf call 1", "next call 2", "done call 2", "value call 2", "valueOf get 2", "valueOf call 2", "next call 3", "done call 3"])
+PASS multiple return values from js to wasm
 

trunk/LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/constructor/multi-value.any.worker-expected.txt

@@ -2 +2 @@
 PASS multiple return values from wasm to js
 PASS multiple return values inside wasm
-FAIL multiple return values from js to wasm assert_array_equals: expected property 6 to be "next call 2" but got "valueOf get 1" (expected array ["@@iterator getter", "@@iterator call", "next getter", "next call 1", "done call 1", "value call 1", "next call 2", "done call 2", "value call 2", "next call 3", "done call 3", "valueOf get 1", "valueOf call 1", "valueOf get 2", "valueOf call 2"] got ["@@iterator getter", "@@iterator call", "next getter", "next call 1", "done call 1", "value call 1", "valueOf get 1", "valueOf call 1", "next call 2", "done call 2", "value call 2", "valueOf get 2", "valueOf call 2", "next call 3", "done call 3"])
+PASS multiple return values from js to wasm
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-12-30  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Wasm multivalue should iterate iterable result from JS function first before converting values
+        https://bugs.webkit.org/show_bug.cgi?id=220206
+
+        Reviewed by Alexey Shvayka.
+
+        When converting JS results to Wasm multivalue (result from JS when executing Wasm->JS calls), we should first iterate all results from iterable.
+        And then, we should convert each element into Wasm value. Currently, we are converting while iterating, this is not aligned to the spec.
+
+        * wasm/WasmOperations.cpp:
+        (JSC::Wasm::JSC_DEFINE_JIT_OPERATION):
+
 2020-12-29  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/wasm/WasmOperations.cpp

@@ -509 +509 @@
 }
 
+// https://webassembly.github.io/multi-value/js-api/index.html#run-a-host-function
 JSC_DEFINE_JIT_OPERATION(operationIterateResults, void, (CallFrame* callFrame, Instance* instance, const Signature* signature, JSValue result, uint64_t* registerResults, uint64_t* calleeFramePointer))
 {
@@ -522 +523 @@
     RegisterAtOffsetList registerResultOffsets = wasmCallInfo.computeResultsOffsetList();
 
-    unsigned itemsInserted = 0;
-    forEachInIterable(globalObject, result, [&] (VM& vm, JSGlobalObject* globalObject, JSValue value) -> void {
-        auto scope = DECLARE_THROW_SCOPE(vm);
-        if (itemsInserted < signature->returnCount()) {
-            uint64_t unboxedValue;
-            switch (signature->returnType(itemsInserted)) {
-            case I32:
-                unboxedValue = value.toInt32(globalObject);
-                break;
-            case F32:
-                unboxedValue = bitwise_cast<uint32_t>(value.toFloat(globalObject));
-                break;
-            case F64:
-                unboxedValue = bitwise_cast<uint64_t>(value.toNumber(globalObject));
-                break;
-            case Funcref:
-                if (!value.isCallable(vm)) {
-                    throwTypeError(globalObject, scope, "Funcref value is not a function"_s);
-                    return;
-                }
-                FALLTHROUGH;
-            case Externref:
-                unboxedValue = bitwise_cast<uint64_t>(value);
-                RELEASE_ASSERT(Options::useWebAssemblyReferences());
-                break;
-            default:
-                RELEASE_ASSERT_NOT_REACHED();
-            }
-
-            RETURN_IF_EXCEPTION(scope, void());
-            auto rep = wasmCallInfo.results[itemsInserted];
-            if (rep.isReg())
-                registerResults[registerResultOffsets.find(rep.reg())->offset() / sizeof(uint64_t)] = unboxedValue;
-            else
-                calleeFramePointer[rep.offsetFromFP() / sizeof(uint64_t)] = unboxedValue;
-        }
-        itemsInserted++;
+    MarkedArgumentBuffer buffer;
+    forEachInIterable(globalObject, result, [&] (VM&, JSGlobalObject*, JSValue value) -> void {
+        if (buffer.size() < signature->returnCount())
+            buffer.append(value);
     });
     RETURN_IF_EXCEPTION(scope, void());
-    if (itemsInserted != signature->returnCount())
+
+    if (buffer.hasOverflowed()) {
+        throwOutOfMemoryError(globalObject, scope, "JS results to Wasm are too large");
+        return;
+    }
+
+    if (buffer.size() != signature->returnCount()) {
         throwVMTypeError(globalObject, scope, "Incorrect number of values returned to Wasm from JS");
+        return;
+    }
+
+    for (unsigned index = 0; index < buffer.size(); ++index) {
+        JSValue value = buffer.at(index);
+
+        uint64_t unboxedValue = 0;
+        switch (signature->returnType(index)) {
+        case I32:
+            unboxedValue = value.toInt32(globalObject);
+            break;
+        case F32:
+            unboxedValue = bitwise_cast<uint32_t>(value.toFloat(globalObject));
+            break;
+        case F64:
+            unboxedValue = bitwise_cast<uint64_t>(value.toNumber(globalObject));
+            break;
+        case Funcref:
+            if (!value.isCallable(vm)) {
+                throwTypeError(globalObject, scope, "Funcref value is not a function"_s);
+                return;
+            }
+            FALLTHROUGH;
+        case Externref:
+            unboxedValue = bitwise_cast<uint64_t>(value);
+            RELEASE_ASSERT(Options::useWebAssemblyReferences());
+            break;
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+        }
+        RETURN_IF_EXCEPTION(scope, void());
+
+        auto rep = wasmCallInfo.results[index];
+        if (rep.isReg())
+            registerResults[registerResultOffsets.find(rep.reg())->offset() / sizeof(uint64_t)] = unboxedValue;
+        else
+            calleeFramePointer[rep.offsetFromFP() / sizeof(uint64_t)] = unboxedValue;
+    }
 }