[WASM-References] Remove subtyping rule for externref and funcref

https://bugs.webkit.org/show_bug.cgi?id=218885 Patch by Dmitry Bezhetskov <dbezhetskov@igalia.com> on 2020-11-18 Reviewed by Yusuke Suzuki. Make funcref is not a subtype of externref. The spec: https://webassembly.github.io/reference-types/core/ The PR for removing subtype from the spec: WebAssembly/reference-types#87. JSTests: * wasm/references/func_ref.js: (assert.eq.instance.exports.fix.fun): (assert.eq.instance.exports.fix): * wasm/references/validation.js: Source/JavaScriptCore: * wasm/WasmFormat.h: (JSC::Wasm::isSubtype): * wasm/WasmFunctionParser.h: (JSC::Wasm::FunctionParser<Context>::parseExpression): * wasm/js/WebAssemblyModuleRecord.cpp: (JSC::WebAssemblyModuleRecord::link): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@270015 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Igalia · Nov 19, 2020 · 81172f8 · 81172f8
1 parent bf69c83
commit 81172f8
Show file tree

Hide file tree

Showing 15 changed files with 104 additions and 64 deletions.
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
@@ -1,3 +1,20 @@
+2020-11-18  Dmitry Bezhetskov  <dbezhetskov@igalia.com>
+
+        [WASM-References] Remove subtyping rule for externref and funcref
+        https://bugs.webkit.org/show_bug.cgi?id=218885
+
+        Reviewed by Yusuke Suzuki.
+
+        Make funcref is not a subtype of externref.
+        The spec: https://webassembly.github.io/reference-types/core/
+        The PR for removing subtype from the spec:
+        https://github.com/WebAssembly/reference-types/pull/87.
+
+        * wasm/references/func_ref.js:
+        (assert.eq.instance.exports.fix.fun):
+        (assert.eq.instance.exports.fix):
+        * wasm/references/validation.js:
+
 2020-11-18  Ross Kirsling  <ross.kirsling@sony.com>
 
         [JSC] Reinstate String#at

diff --git a/JSTests/wasm/js-api/web-assembly-instantiate.js b/JSTests/wasm/js-api/web-assembly-instantiate.js
@@ -76,7 +76,7 @@ assert.eq(WebAssembly.instantiate.length, 1);
             let {module, instance} = await WebAssembly.instantiate(bin);
         } catch(e) {
             assert.truthy(e instanceof WebAssembly.CompileError);
-            assert.eq(e.message, "WebAssembly.Module doesn't validate: control flow returns with unexpected type. F32 is not a subtype of I32, in function at index 0");
+            assert.eq(e.message, "WebAssembly.Module doesn't validate: control flow returns with unexpected type. F32 is not a I32, in function at index 0");
         }
     }
 

diff --git a/JSTests/wasm/references/func_ref.js b/JSTests/wasm/references/func_ref.js
@@ -75,27 +75,20 @@ function makeFuncrefIdent() {
           .Type().End()
           .Function().End()
           .Export()
-              .Function("h")
               .Function("i")
-              .Function("get_h")
+              .Function("get_i")
               .Function("fix")
               .Function("get_not_exported")
               .Function("local_read")
           .End()
           .Code()
-            .Function("h", { params: ["funcref"], ret: "externref" }, ["externref"])
-              .GetLocal(0)
-              .SetLocal(1)
-              .GetLocal(1)
-            .End()
-
             .Function("i", { params: ["funcref"], ret: "funcref" }, ["funcref"])
               .GetLocal(0)
               .SetLocal(1)
               .GetLocal(1)
             .End()
 
-            .Function("get_h", { params: [], ret: "funcref" }, ["funcref"])
+            .Function("get_i", { params: [], ret: "funcref" }, ["funcref"])
               .I32Const(0)
               .RefFunc(0)
               .SetLocal(0)
@@ -111,11 +104,11 @@ function makeFuncrefIdent() {
             .End()
 
             .Function("fix", { params: [], ret: "funcref" }, [])
-              .RefFunc(3)
+              .RefFunc(2)
             .End()
 
             .Function("get_not_exported", { params: [], ret: "funcref" }, [])
-              .RefFunc(5)
+              .RefFunc(4)
             .End()
 
             .Function("ret_42", { params: [], ret: "i32" }, [])
@@ -134,29 +127,46 @@ function makeFuncrefIdent() {
     fullGC();
 
     assert.eq(instance.exports.local_read(), 1)
-    assert.eq(instance.exports.h(null), null)
-
-    assert.throws(() => instance.exports.h(fun), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
-    assert.eq(instance.exports.h(myfun), myfun)
-    assert.throws(() => instance.exports.h(5), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
-    assert.throws(() => instance.exports.h(undefined), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
 
     assert.eq(instance.exports.i(null), null)
     assert.eq(instance.exports.i(myfun), myfun)
     assert.throws(() => instance.exports.i(fun), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
     assert.throws(() => instance.exports.i(5), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
 
-    assert.throws(() => instance.exports.get_h()(fun), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
-    assert.eq(instance.exports.get_h()(null), null)
-    assert.eq(instance.exports.get_h()(myfun), myfun)
-    assert.throws(() => instance.exports.get_h()(5), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
+    assert.throws(() => instance.exports.get_i()(fun), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
+    assert.eq(instance.exports.get_i()(null), null)
+    assert.eq(instance.exports.get_i()(myfun), myfun)
+    assert.throws(() => instance.exports.get_i()(5), Error, "Funcref must be an exported wasm function (evaluating 'func(...args)')")
 
     assert.eq(instance.exports.get_not_exported()(), 42)
 
     assert.eq(instance.exports.fix()(), instance.exports.fix());
     assert.eq(instance.exports.fix(), instance.exports.fix);
 }
 
+// Casting the Funcref to Externref is forbidden.
+
+{
+    function fun() { return 41; }
+
+    const builder = (new Builder())
+          .Type().End()
+          .Function().End()
+          .Export()
+              .Function("h")
+          .End()
+          .Code()
+            .Function("h", { params: ["funcref"], ret: "externref" }, ["externref"])
+              .GetLocal(0)
+              .SetLocal(1)
+              .GetLocal(1)
+            .End()
+          .End();
+
+    const bin = builder.WebAssembly().get();
+    assert.throws(() => new WebAssembly.Module(bin), WebAssembly.CompileError, "WebAssembly.Module doesn't validate: set_local to type Funcref expected Externref, in function at index 0 (evaluating 'new WebAssembly.Module(bin)')");
+}
+
 // Globals
 
 {
@@ -246,7 +256,7 @@ assert.throws(() => new WebAssembly.Module((new Builder())
     .Function("h", { params: ["externref"], ret: "funcref" })
       .GetLocal(0)
     .End()
-  .End().WebAssembly().get()), Error, "WebAssembly.Module doesn't validate: control flow returns with unexpected type. Externref is not a subtype of Funcref, in function at index 0 (evaluating 'new WebAssembly.Module')");
+  .End().WebAssembly().get()), Error, "WebAssembly.Module doesn't validate: control flow returns with unexpected type. Externref is not a Funcref, in function at index 0 (evaluating 'new WebAssembly.Module')");
 
 assert.throws(() => new WebAssembly.Module((new Builder())
   .Type().End()

diff --git a/JSTests/wasm/references/validation.js b/JSTests/wasm/references/validation.js
@@ -19,7 +19,7 @@ import Builder from '../Builder.js';
     const bin = builder.WebAssembly();
     bin.trim();
 
-    assert.throws(() => new WebAssembly.Module(bin.get()), WebAssembly.CompileError, "WebAssembly.Module doesn't validate: ref.is_null to type I32 expected Externref, in function at index 0 (evaluating 'new WebAssembly.Module(bin.get())')");
+    assert.throws(() => new WebAssembly.Module(bin.get()), WebAssembly.CompileError, "WebAssembly.Module doesn't validate: ref.is_null to type I32 expected a reference type, in function at index 0 (evaluating 'new WebAssembly.Module(bin.get())')");
 }
 
 {
@@ -90,7 +90,7 @@ import Builder from '../Builder.js';
     const bin = builder.WebAssembly();
     bin.trim();
 
-    assert.throws(() => new WebAssembly.Module(bin.get()), WebAssembly.CompileError, "WebAssembly.Module doesn't validate: control flow returns with unexpected type. Externref is not a subtype of Funcref, in function at index 0 (evaluating 'new WebAssembly.Module(bin.get())')");
+    assert.throws(() => new WebAssembly.Module(bin.get()), WebAssembly.CompileError, "WebAssembly.Module doesn't validate: control flow returns with unexpected type. Externref is not a Funcref, in function at index 0 (evaluating 'new WebAssembly.Module(bin.get())')");
 }
 
 {

diff --git a/LayoutTests/workers/wasm-references/test.js b/LayoutTests/workers/wasm-references/test.js
@@ -1860,9 +1860,9 @@ function runTest() {
             .Function("tbl_size", { params: [], ret: "i32" })
               .TableSize(1)
             .End()
-            .Function("tbl_grow", { params: ["i32"], ret: "i32" })
-                .RefFunc(0)
+            .Function("tbl_grow", { params: ["externref", "i32"], ret: "i32" })
                 .GetLocal(0)
+                .GetLocal(1)
                 .TableGrow(1)
             .End()
             .Function("tbl_fill", { params: ["i32", "externref", "i32"], ret: "void" })

diff --git a/LayoutTests/workers/wasm-references/worker.js b/LayoutTests/workers/wasm-references/worker.js
@@ -173,10 +173,10 @@ onmessage = function({ data: module }) {
     WebAssembly.instantiate(module).then(($1) => {
         try {
             assert.eq($1.exports.tbl_size(), 20)
-            assert.eq($1.exports.tbl_grow(5), 20)
+            assert.eq($1.exports.tbl_grow("hi", 5), 20)
             assert.eq($1.exports.tbl_size(), 25)
             assert.eq($1.exports.tbl.get(0), null)
-            assert.eq($1.exports.tbl.get(24), $1.exports.tbl_size)
+            assert.eq($1.exports.tbl.get(24), "hi")
 
             for (let i=0; i<1000; ++i) {
                 $1.exports.tbl_fill(1,"hi",3)

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,22 @@
+2020-11-18  Dmitry Bezhetskov  <dbezhetskov@igalia.com>
+
+        [WASM-References] Remove subtyping rule for externref and funcref
+        https://bugs.webkit.org/show_bug.cgi?id=218885
+
+        Reviewed by Yusuke Suzuki.
+
+        Make funcref is not a subtype of externref.
+        The spec: https://webassembly.github.io/reference-types/core/
+        The PR for removing subtype from the spec:
+        https://github.com/WebAssembly/reference-types/pull/87.
+
+        * wasm/WasmFormat.h:
+        (JSC::Wasm::isSubtype):
+        * wasm/WasmFunctionParser.h:
+        (JSC::Wasm::FunctionParser<Context>::parseExpression):
+        * wasm/js/WebAssemblyModuleRecord.cpp:
+        (JSC::WebAssemblyModuleRecord::link):
+
 2020-11-18  Ross Kirsling  <ross.kirsling@sony.com>
 
         [JSC] Reinstate String#at

diff --git a/Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp b/Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp
@@ -1102,7 +1102,7 @@ auto AirIRGenerator::addTableSize(unsigned tableIndex, ExpressionType& result) -
 auto AirIRGenerator::addTableGrow(unsigned tableIndex, ExpressionType fill, ExpressionType delta, ExpressionType& result) -> PartialResult
 {
     ASSERT(fill.tmp());
-    ASSERT(isSubtype(fill.type(), m_info.tables[tableIndex].wasmType()));
+    ASSERT(fill.type() == m_info.tables[tableIndex].wasmType());
     ASSERT(delta.tmp());
     ASSERT(delta.type() == Type::I32);
     result = tmpForType(Type::I32);
@@ -1115,7 +1115,7 @@ auto AirIRGenerator::addTableGrow(unsigned tableIndex, ExpressionType fill, Expr
 auto AirIRGenerator::addTableFill(unsigned tableIndex, ExpressionType offset, ExpressionType fill, ExpressionType count) -> PartialResult
 {
     ASSERT(fill.tmp());
-    ASSERT(isSubtype(fill.type(), m_info.tables[tableIndex].wasmType()));
+    ASSERT(fill.type() == m_info.tables[tableIndex].wasmType());
     ASSERT(offset.tmp());
     ASSERT(offset.type() == Type::I32);
     ASSERT(count.tmp());
@@ -1251,7 +1251,7 @@ auto AirIRGenerator::setGlobal(uint32_t index, ExpressionType value) -> PartialR
             append(Add64, temp2, temp, temp);
             append(moveOpForValueType(type), value, Arg::addr(temp));
         }
-        if (isSubtype(type, Externref))
+        if (isRefType(type))
             emitWriteBarrierForJSWrapper();
         break;
     case Wasm::GlobalInformation::BindingMode::Portable:
@@ -1266,7 +1266,7 @@ auto AirIRGenerator::setGlobal(uint32_t index, ExpressionType value) -> PartialR
         }
         append(moveOpForValueType(type), value, Arg::addr(temp));
         // We emit a write-barrier onto JSWebAssemblyGlobal, not JSWebAssemblyInstance.
-        if (isSubtype(type, Externref)) {
+        if (isRefType(type)) {
             auto cell = g64();
             auto vm = g64();
             auto cellState = g32();
@@ -2330,7 +2330,7 @@ auto AirIRGenerator::addCallIndirect(unsigned tableIndex, const Signature& signa
 
 void AirIRGenerator::unify(const ExpressionType dst, const ExpressionType source)
 {
-    ASSERT(isSubtype(source.type(), dst.type()));
+    ASSERT(source.type() == dst.type());
     append(moveOpForValueType(dst.type()), source, dst);
 }
 

diff --git a/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp b/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
@@ -633,7 +633,7 @@ auto B3IRGenerator::addLocal(Type type, uint32_t count) -> PartialResult
     for (uint32_t i = 0; i < count; ++i) {
         Variable* local = m_proc.addVariable(toB3Type(type));
         m_locals.uncheckedAppend(local);
-        auto val = isSubtype(type, Externref) ? JSValue::encode(jsNull()) : 0;
+        auto val = isRefType(type) ? JSValue::encode(jsNull()) : 0;
         m_currentBlock->appendNew<VariableValue>(m_proc, Set, Origin(), local, constant(toB3Type(type), val, Origin()));
     }
     return { };
@@ -840,15 +840,15 @@ auto B3IRGenerator::setGlobal(uint32_t index, ExpressionType value) -> PartialRe
     switch (global.bindingMode) {
     case Wasm::GlobalInformation::BindingMode::EmbeddedInInstance:
         m_currentBlock->appendNew<MemoryValue>(m_proc, Store, origin(), value, globalsArray, safeCast<int32_t>(index * sizeof(Register)));
-        if (isSubtype(global.type, Externref))
+        if (isRefType(global.type))
             emitWriteBarrierForJSWrapper();
         break;
     case Wasm::GlobalInformation::BindingMode::Portable: {
         ASSERT(global.mutability == Wasm::GlobalInformation::Mutability::Mutable);
         Value* pointer = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, B3::Int64, origin(), globalsArray, safeCast<int32_t>(index * sizeof(Register)));
         m_currentBlock->appendNew<MemoryValue>(m_proc, Store, origin(), value, pointer);
         // We emit a write-barrier onto JSWebAssemblyGlobal, not JSWebAssemblyInstance.
-        if (isSubtype(global.type, Externref)) {
+        if (isRefType(global.type)) {
             Value* instance = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, pointerType(), origin(), instanceValue(), safeCast<int32_t>(Instance::offsetOfOwner()));
             Value* cell = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, pointerType(), origin(), pointer, Wasm::Global::offsetOfOwner() - Wasm::Global::offsetOfValue());
             Value* cellState = m_currentBlock->appendNew<MemoryValue>(m_proc, Load8Z, Int32, origin(), cell, safeCast<int32_t>(JSCell::cellStateOffset()));

diff --git a/Source/JavaScriptCore/wasm/WasmFormat.h b/Source/JavaScriptCore/wasm/WasmFormat.h
@@ -78,13 +78,6 @@ inline bool isValueType(Type type)
     return false;
 }
 
-inline bool isSubtype(Type sub, Type parent)
-{
-    if (sub == parent)
-        return true;
-    return sub == Funcref && parent == Externref;
-}
-
 inline bool isRefType(Type type)
 {
     return type == Externref || type == Funcref;