[wasm-gc] Disallow anyref in JS interop

Also fix eqref/i31ref fromJS() handling to accept unwrapped Smis.
This does not convert HeapNumbers to Smis if they fit.

Bug: v8:7748
Change-Id: Ida70a826f9541b7f3fbe9eecbb2b4fe362b5ef70
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3829477
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82558}

src/compiler/wasm-compiler.cc

@@ -6302,7 +6302,6 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
           case wasm::HeapType::kEq:
           case wasm::HeapType::kData:
           case wasm::HeapType::kArray:
-          case wasm::HeapType::kI31:
             // TODO(7748): Update this when JS interop is settled.
             if (type.kind() == wasm::kRefNull) {
               auto done =
@@ -6322,30 +6321,11 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
             return node;
           case wasm::HeapType::kExtern:
             return node;
-          case wasm::HeapType::kAny: {
-            if (!enabled_features_.has_gc()) return node;
-            // Wrap {node} in object wrapper if it is an array/struct.
-            // Extract external function if this is a WasmInternalFunction.
-            // Otherwise (i.e. null and external refs), return input.
-            // Treat i31 as externref because they are indistinguishable from
-            // Smis.
-            // TODO(7748): Update this when JS interop is settled.
-            auto wrap = gasm_->MakeLabel();
-            auto done = gasm_->MakeLabel(MachineRepresentation::kTaggedPointer);
-            gasm_->GotoIf(IsSmi(node), &done, node);
-            gasm_->GotoIf(gasm_->IsDataRefMap(gasm_->LoadMap(node)), &wrap);
-            // This includes the case where {node == null}.
-            gasm_->Goto(&done, node);
-
-            gasm_->Bind(&wrap);
-            gasm_->Goto(&done, BuildAllocateObjectWrapper(node, context));
-
-            gasm_->Bind(&done);
-            return done.PhiAt(0);
-          }
           case wasm::HeapType::kNone:
           case wasm::HeapType::kNoFunc:
           case wasm::HeapType::kNoExtern:
+          case wasm::HeapType::kI31:
+          case wasm::HeapType::kAny:
             UNREACHABLE();
           default:
             DCHECK(type.has_index());
@@ -6468,10 +6448,14 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
                                      target, input, context);
   }
 
+  enum class I31Check : bool { Invalid, Valid };
+
   void BuildCheckValidRefValue(Node* input, Node* js_context,
-                               wasm::ValueType type) {
+                               wasm::ValueType type, I31Check i31_check) {
     // Make sure ValueType fits in a Smi.
     static_assert(wasm::ValueType::kLastUsedBit + 1 <= kSmiValueSize);
+
+    auto done = gasm_->MakeLabel();
     // The instance node is always defined: if an instance is not available, it
     // is the undefined value.
     Node* inputs[] = {GetInstance(), input,
@@ -6481,16 +6465,15 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
     Node* check = gasm_->BuildChangeSmiToInt32(BuildCallToRuntimeWithContext(
         Runtime::kWasmIsValidRefValue, js_context, inputs, 3));
 
-    Diamond type_check(graph(), mcgraph()->common(), check, BranchHint::kTrue);
-    type_check.Chain(control());
-    SetControl(type_check.if_false);
-
-    Node* old_effect = effect();
+    gasm_->GotoIf(check, &done, BranchHint::kTrue);
+    if (i31_check == I31Check::Valid) {
+      Node* is_smi = IsSmi(input);
+      gasm_->GotoIf(is_smi, &done, BranchHint::kTrue);
+    }
     BuildCallToRuntimeWithContext(Runtime::kWasmThrowJSTypeError, js_context,
                                   nullptr, 0);
-
-    SetEffectControl(type_check.EffectPhi(old_effect, effect()),
-                     type_check.merge);
+    gasm_->Goto(&done);
+    gasm_->Bind(&done);
   }
 
   Node* BuildCheckString(Node* input, Node* js_context, wasm::ValueType type) {
@@ -6520,33 +6503,23 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
         switch (type.heap_representation()) {
           case wasm::HeapType::kExtern:
             return input;
-          case wasm::HeapType::kAny:
-            if (!enabled_features_.has_gc()) return input;
-            // If this is a wrapper for arrays/structs/i31s, unpack it.
-            // TODO(7748): Update this when JS interop has settled.
-            // We prefer not to unwrap functions here, because deciding whether
-            // that's valid for a given function is expensive (specifically,
-            // it's not valid for plain JS functions, because they don't have
-            // a WasmFunctionData/WasmInternalFunction inside). The consequence
-            // is that passing a funcref to JS and taking it back as {anyref}
-            // turns it into an opaque pointer that can't be cast back to
-            // {funcref}.
-            return BuildUnpackObjectWrapper(input, js_context,
-                                            kLeaveFunctionsAlone);
           case wasm::HeapType::kFunc:
-            BuildCheckValidRefValue(input, js_context, type);
+            BuildCheckValidRefValue(input, js_context, type, I31Check::Invalid);
             return BuildUnpackObjectWrapper(input, js_context,
                                             kUnwrapWasmExternalFunctions);
           case wasm::HeapType::kData:
           case wasm::HeapType::kArray:
+            // TODO(7748): Update this when JS interop has settled.
+            BuildCheckValidRefValue(input, js_context, type, I31Check::Invalid);
+            // This will just return {input} if the object is not wrapped, i.e.
+            // if it is null (given the check just above).
+            return BuildUnpackObjectWrapper(input, js_context,
+                                            kLeaveFunctionsAlone);
           case wasm::HeapType::kEq:
-          case wasm::HeapType::kI31:
             // TODO(7748): Update this when JS interop has settled.
-            BuildCheckValidRefValue(input, js_context, type);
+            BuildCheckValidRefValue(input, js_context, type, I31Check::Valid);
             // This will just return {input} if the object is not wrapped, i.e.
             // if it is null (given the check just above).
-            // Skip function unpacking here to save code size (they can't occur
-            // anyway).
             return BuildUnpackObjectWrapper(input, js_context,
                                             kLeaveFunctionsAlone);
           case wasm::HeapType::kString:
@@ -6554,10 +6527,13 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
           case wasm::HeapType::kNone:
           case wasm::HeapType::kNoFunc:
           case wasm::HeapType::kNoExtern:
+          case wasm::HeapType::kAny:
+          case wasm::HeapType::kI31:
             UNREACHABLE();
           default:
             if (module_->has_signature(type.ref_index())) {
-              BuildCheckValidRefValue(input, js_context, type);
+              BuildCheckValidRefValue(input, js_context, type,
+                                      I31Check::Invalid);
               return BuildUnpackObjectWrapper(input, js_context,
                                               kUnwrapWasmExternalFunctions);
             }

src/wasm/wasm-opcodes.cc

@@ -50,6 +50,8 @@ bool IsJSCompatibleSignature(const FunctionSig* sig, const WasmModule* module,
         case HeapType::kNone:
         case HeapType::kNoFunc:
         case HeapType::kNoExtern:
+        case HeapType::kAny:
+        case HeapType::kI31:
           return false;
         default:
           break;

test/mjsunit/regress/wasm/regress-12789.js

@@ -11,13 +11,13 @@ var instance = (function () {
 
   let struct_index = builder.addStruct([makeField(kWasmI32, true)]);
   let callback = builder.addImport(
-      'import', 'callback', {params: [kWasmAnyRef], results: []});
+      'import', 'callback', {params: [kWasmExternRef], results: []});
 
   builder.addFunction("object", { params: [], results: [kWasmEqRef] })
     .addBody([kGCPrefix, kExprStructNewDefault, struct_index]).exportFunc();
 
   builder.addFunction(
-      'roundtrip', {params: [kWasmEqRef, kWasmAnyRef], results: []})
+      'roundtrip', {params: [kWasmEqRef, kWasmExternRef], results: []})
     .addBody([
       kExprLocalGet, 1,
       kExprCallFunction, callback,

test/mjsunit/wasm/gc-experimental-string-conversions.js

@@ -10,7 +10,7 @@ var builder = new WasmModuleBuilder();
 
 let i16Array = builder.addArray(kWasmI16, true);
 
-builder.addFunction('getHelloArray', makeSig([], [kWasmAnyRef]))
+builder.addFunction('getHelloArray', makeSig([], [kWasmArrayRef]))
     .addBody([
       ...wasmI32Const(72), ...wasmI32Const(69), ...wasmI32Const(76),
       ...wasmI32Const(76), ...wasmI32Const(79),
@@ -18,7 +18,7 @@ builder.addFunction('getHelloArray', makeSig([], [kWasmAnyRef]))
     ])
     .exportFunc();
 
-builder.addFunction('getChar', makeSig([kWasmAnyRef, kWasmI32], [kWasmI32]))
+builder.addFunction('getChar', makeSig([kWasmArrayRef, kWasmI32], [kWasmI32]))
     .addBody([
       kExprLocalGet, 0, kGCPrefix, kExprRefAsData, kGCPrefix,
       kExprRefCastStatic, i16Array, kExprLocalGet, 1, kGCPrefix, kExprArrayGetS,

test/mjsunit/wasm/reference-globals.js

@@ -162,7 +162,7 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
       kGCPrefix, kExprStructGet, composite_struct_index, 0])
     .exportFunc();
 
-  builder.addFunction("field_2_default", makeSig([], [kWasmAnyRef]))
+  builder.addFunction("field_2_default", makeSig([], [kWasmDataRef]))
     .addBody([
       kExprGlobalGet, global_default.index,
       kGCPrefix, kExprStructGet, composite_struct_index, 1])
@@ -245,7 +245,7 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
       kGCPrefix, kExprStructGet, struct_index, 0])
     .exportFunc();
 
-  builder.addFunction("element1", makeSig([], [kWasmAnyRef]))
+  builder.addFunction("element1", makeSig([], [kWasmDataRef]))
     .addBody([
       kExprGlobalGet, global.index,
       kExprI32Const, 1,

test/mjsunit/wasm/wasm-gc-externalize-internalize.js

@@ -20,7 +20,7 @@ let instance = (() => {
    * type_internalize -> consume type by externref, internalize
    */
 
-  builder.addFunction('struct_producer', makeSig([kWasmI32], [kWasmAnyRef]))
+  builder.addFunction('struct_producer', makeSig([kWasmI32], [kWasmEqRef]))
     .addBody([
       kExprLocalGet, 0,
       kGCPrefix, kExprStructNew, struct])
@@ -36,7 +36,7 @@ let instance = (() => {
     .exportFunc();
 
   builder.addFunction('struct_consumer',
-                      makeSig([kWasmAnyRef], [kWasmI32, kWasmI32]))
+                      makeSig([kWasmEqRef], [kWasmI32, kWasmI32]))
     .addBody([
       kExprLocalGet, 0,
       kExprRefIsNull,
@@ -76,7 +76,7 @@ let instance = (() => {
     ])
     .exportFunc();
 
-    builder.addFunction('i31_producer', makeSig([kWasmI32], [kWasmAnyRef]))
+    builder.addFunction('i31_producer', makeSig([kWasmI32], [kWasmEqRef]))
     .addBody([
       kExprLocalGet, 0,
       kGCPrefix, kExprI31New])
@@ -92,7 +92,7 @@ let instance = (() => {
     .exportFunc();
 
     builder.addFunction('i31_consumer',
-                        makeSig([kWasmAnyRef], [kWasmI32, kWasmI32]))
+                        makeSig([kWasmEqRef], [kWasmI32, kWasmI32]))
     .addBody([
       kExprLocalGet, 0,
       kExprRefIsNull,
@@ -132,7 +132,7 @@ let instance = (() => {
 
     let array = builder.addArray(kWasmI32, true);
 
-    builder.addFunction('array_producer', makeSig([kWasmI32], [kWasmAnyRef]))
+    builder.addFunction('array_producer', makeSig([kWasmI32], [kWasmEqRef]))
     .addBody([
       kExprLocalGet, 0,
       kGCPrefix, kExprArrayNewFixedStatic, array, 1])
@@ -148,7 +148,7 @@ let instance = (() => {
     .exportFunc();
 
     builder.addFunction('array_consumer',
-                        makeSig([kWasmAnyRef], [kWasmI32, kWasmI32]))
+                        makeSig([kWasmEqRef], [kWasmI32, kWasmI32]))
     .addBody([
       kExprLocalGet, 0,
       kExprRefIsNull,
@@ -198,9 +198,17 @@ for (let type of ["struct", "i31", "array"]) {
     let fnConsume = instance.exports[`${type}_${consume}`];
     // A null is converted to (ref null none).
     assertEquals([0, 1], fnConsume(null));
-    // Passing a JavaScript object is allowed and can be internalized
-    // but will fail on casting it.
-    assertTraps(kTrapIllegalCast, () => fnConsume({}));
+    if (consume == "internalize") {
+      // Passing a JavaScript object is fine on internalize but fails on
+      // casting it to dataref/arrayref/i31ref.
+      var errorType = WebAssembly.RuntimeError;
+      var errorMsg = "illegal cast";
+    } else {
+      // Passing a JavaScript object fails as it is not convertible to eqref.
+      var errorType = TypeError;
+      var errorMsg = "type incompatibility when transforming from/to JS";
+    }
+    assertThrows(() => fnConsume({}), errorType, errorMsg);
 
     for (let produce of ["producer", "externalize"]) {
       let fnProduce = instance.exports[`${type}_${produce}`];

test/mjsunit/wasm/wasm-gc-js-anyref.js → test/mjsunit/wasm/wasm-gc-js-ref.js

@@ -10,7 +10,7 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
 let instance = (() => {
   let builder = new WasmModuleBuilder();
   let struct = builder.addStruct([makeField(kWasmI32, true)]);
-  builder.addFunction('createStruct', makeSig([kWasmI32], [kWasmAnyRef]))
+  builder.addFunction('createStruct', makeSig([kWasmI32], [kWasmEqRef]))
     .addBody([
       kExprLocalGet, 0,
       kGCPrefix, kExprStructNew, struct])

test/mjsunit/wasm/wasm-gc-js-roundtrip.js

@@ -27,7 +27,7 @@ let instance = (() => {
       ])
       .exportFunc();
 
-  builder.addFunction('i31_producer', makeSig([], [kWasmI31Ref]))
+  builder.addFunction('i31_as_eq_producer', makeSig([], [kWasmEqRef]))
       .addBody([kExprI32Const, 5, kGCPrefix, kExprI31New])
       .exportFunc();
 
@@ -36,7 +36,6 @@ let instance = (() => {
       .exportFunc();
 
   let test_types = {
-    i31: kWasmI31Ref,
     struct: kWasmDataRef,
     array: kWasmDataRef,
     raw_struct: struct,
@@ -68,26 +67,16 @@ let instance = (() => {
 // Wasm-exposed null is the same as JS null.
 assertEquals(instance.exports.struct_null(), null);
 
-// We can roundtrip an i31.
-instance.exports.i31_id(instance.exports.i31_producer());
-// We can roundtrip any null as i31.
-instance.exports.i31_id(instance.exports.i31_null());
-instance.exports.i31_id(instance.exports.struct_null());
-// We cannot roundtrip a struct as i31.
-assertThrows(
-    () => instance.exports.i31_id(instance.exports.struct_producer()),
-    TypeError, 'type incompatibility when transforming from/to JS');
-
 // We can roundtrip a struct as dataref.
 instance.exports.data_id(instance.exports.struct_producer());
 // We can roundtrip an array as dataref.
 instance.exports.data_id(instance.exports.array_producer());
-// We can roundtrip any null as dataref.
+// We can roundtrip null as dataref.
 instance.exports.data_id(instance.exports.data_null());
-instance.exports.data_id(instance.exports.i31_null());
 // We cannot roundtrip an i31 as dataref.
 assertThrows(
-    () => instance.exports.data_id(instance.exports.i31_producer()), TypeError,
+    () => instance.exports.data_id(instance.exports.i31_as_eq_producer()),
+    TypeError,
     'type incompatibility when transforming from/to JS');
 
 // We can roundtrip a struct as eqref.
@@ -95,35 +84,27 @@ instance.exports.eq_id(instance.exports.struct_producer());
 // We can roundtrip an array as eqref.
 instance.exports.eq_id(instance.exports.array_producer());
 // We can roundtrip an i31 as eqref.
-instance.exports.eq_id(instance.exports.i31_producer());
+instance.exports.eq_id(instance.exports.i31_as_eq_producer());
 // We can roundtrip any null as eqref.
 instance.exports.eq_id(instance.exports.data_null());
-instance.exports.eq_id(instance.exports.i31_null());
+instance.exports.eq_id(instance.exports.eq_null());
 instance.exports.eq_id(instance.exports.func_null());
 // We cannot roundtrip a func as eqref.
 assertThrows(
     () => instance.exports.eq_id(instance.exports.func_producer()), TypeError,
     'type incompatibility when transforming from/to JS');
 
-// We can roundtrip a struct as anyref.
-instance.exports.any_id(instance.exports.struct_producer());
-// We can roundtrip an array as anyref.
-instance.exports.any_id(instance.exports.array_producer());
-// We can roundtrip an i31 as anyref.
-instance.exports.any_id(instance.exports.i31_producer());
-// We can roundtrip a func as anyref.
-instance.exports.any_id(instance.exports.func_producer());
-// We can roundtrip any null as anyref.
-instance.exports.any_id(instance.exports.data_null());
-instance.exports.any_id(instance.exports.i31_null());
-instance.exports.any_id(instance.exports.func_null());
-// We can roundtrip a JS object as anyref.
-instance.exports.any_id(instance);
+// Anyref is not allowed at the JS interface.
+assertThrows(
+    () => instance.exports.any_null(), TypeError,
+    'type incompatibility when transforming from/to JS');
+assertThrows(
+    () => instance.exports.any_id(), TypeError,
+    'type incompatibility when transforming from/to JS');
 
 // We can roundtrip a typed function.
 instance.exports.typed_func_id(instance.exports.func_producer());
 // We can roundtrip any null as typed funcion.
-instance.exports.typed_func_id(instance.exports.i31_null());
 instance.exports.typed_func_id(instance.exports.struct_null());
 // We cannot roundtrip a struct as typed funcion.
 assertThrows(
@@ -133,11 +114,11 @@ assertThrows(
 // We can roundtrip a func.
 instance.exports.func_id(instance.exports.func_producer());
 // We can roundtrip any null as func.
-instance.exports.func_id(instance.exports.i31_null());
 instance.exports.func_id(instance.exports.struct_null());
 // We cannot roundtrip an i31 as func.
 assertThrows(
-    () => instance.exports.func_id(instance.exports.i31_producer()), TypeError,
+    () => instance.exports.func_id(instance.exports.i31_as_eq_producer()),
+    TypeError,
     'type incompatibility when transforming from/to JS');
 
 // We cannot directly roundtrip structs or arrays.