Commit 8df26597 authored by Manos Koukoutos's avatar Manos Koukoutos Committed by V8 LUCI CQ

[wasm-gc] Fixes for the JS/Wasm boundary

- i31s should not be packed in {WasmWrapperGraphBuilder::ToJS}.
- anyref should be able to hold any JS value (except null if non
  nullable).
- Restructure TypeCheckJSObject.

Bug: v8:7748
Change-Id: I51ab6b84e89a70e565ce56de7a41f8693aa28e5b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3871073Reviewed-by: 's avatarMatthias Liedtke <mliedtke@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82992}
parent 6811cb9f
...@@ -6304,7 +6304,19 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { ...@@ -6304,7 +6304,19 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
WasmInternalFunction::kExternalOffset)); WasmInternalFunction::kExternalOffset));
} }
} }
case wasm::HeapType::kEq: case wasm::HeapType::kEq: {
// TODO(7748): Update this when JS interop is settled.
auto done = gasm_->MakeLabel(MachineRepresentation::kTaggedPointer);
// Do not wrap i31s.
gasm_->GotoIf(IsSmi(node), &done, node);
if (type.kind() == wasm::kRefNull) {
// Do not wrap {null}.
gasm_->GotoIf(IsNull(node), &done, node);
}
gasm_->Goto(&done, BuildAllocateObjectWrapper(node, context));
gasm_->Bind(&done);
return done.PhiAt(0);
}
case wasm::HeapType::kData: case wasm::HeapType::kData:
case wasm::HeapType::kArray: case wasm::HeapType::kArray:
// TODO(7748): Update this when JS interop is settled. // TODO(7748): Update this when JS interop is settled.
......
...@@ -2334,9 +2334,10 @@ Handle<AsmWasmData> AsmWasmData::New( ...@@ -2334,9 +2334,10 @@ Handle<AsmWasmData> AsmWasmData::New(
namespace wasm { namespace wasm {
bool TryUnpackObjectWrapper(Isolate* isolate, Handle<Object>& in_out_value) { bool TryUnpackObjectWrapper(Isolate* isolate, Handle<Object>& in_out_value) {
if (in_out_value->IsUndefined(isolate)) return false; if (in_out_value->IsUndefined(isolate) || in_out_value->IsNull(isolate) ||
if (in_out_value->IsNull(isolate)) return true; !in_out_value->IsJSObject()) {
if (!in_out_value->IsJSObject()) return false; return false;
}
Handle<Name> key = isolate->factory()->wasm_wrapped_object_symbol(); Handle<Name> key = isolate->factory()->wasm_wrapped_object_symbol();
LookupIterator it(isolate, in_out_value, key, LookupIterator it(isolate, in_out_value, key,
LookupIterator::OWN_SKIP_INTERCEPTOR); LookupIterator::OWN_SKIP_INTERCEPTOR);
...@@ -2345,6 +2346,8 @@ bool TryUnpackObjectWrapper(Isolate* isolate, Handle<Object>& in_out_value) { ...@@ -2345,6 +2346,8 @@ bool TryUnpackObjectWrapper(Isolate* isolate, Handle<Object>& in_out_value) {
return true; return true;
} }
// TODO(7748): Unify this with transforming from JS to wasm representation, and
// use it as the single interface for the JS->wasm boundary.
bool TypecheckJSObject(Isolate* isolate, const WasmModule* module, bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
Handle<Object> value, ValueType expected, Handle<Object> value, ValueType expected,
const char** error_message) { const char** error_message) {
...@@ -2369,6 +2372,10 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module, ...@@ -2369,6 +2372,10 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
} }
V8_FALLTHROUGH; V8_FALLTHROUGH;
case kRef: { case kRef: {
// TODO(7748): Follow any changes in proposed JS API. In particular,
// finalize the v8_flags.wasm_gc_js_interop situation.
// TODO(7748): Allow all in-range numbers for i31.
// TODO(7748): Streamline interaction of undefined and (ref any).
HeapType::Representation repr = expected.heap_representation(); HeapType::Representation repr = expected.heap_representation();
switch (repr) { switch (repr) {
case HeapType::kFunc: { case HeapType::kFunc: {
...@@ -2382,44 +2389,54 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module, ...@@ -2382,44 +2389,54 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
return true; return true;
} }
case HeapType::kExtern: case HeapType::kExtern:
return true; if (!value->IsNull(isolate)) return true;
case HeapType::kData: *error_message = "null is not allowed for (ref extern)";
case HeapType::kArray: return false;
case HeapType::kAny: case HeapType::kAny:
case HeapType::kEq:
case HeapType::kI31: {
// TODO(7748): Change this when we have a decision on the JS API for
// structs/arrays.
// TODO(7748): Reiterate isSmi() check for i31refs once spec work is
// done: Probably all JS number objects shall be allowed if
// representable as a 31 bit SMI.
if (!v8_flags.wasm_gc_js_interop) { if (!v8_flags.wasm_gc_js_interop) {
if (!value->IsSmi() && !value->IsString() && TryUnpackObjectWrapper(isolate, value);
!TryUnpackObjectWrapper(isolate, value)) {
*error_message =
"eqref/dataref/i31ref object must be null (if nullable) or "
"wrapped with the wasm object wrapper";
return false;
}
} }
if (!value->IsNull(isolate)) return true;
if (repr == HeapType::kI31) { *error_message = "null is not allowed for (ref any)";
if (!value->IsSmi()) { return false;
*error_message = "i31ref-typed object cannot be a heap object"; case HeapType::kData: {
return false; if (v8_flags.wasm_gc_js_interop
} ? value->IsWasmStruct() || value->IsWasmArray()
: TryUnpackObjectWrapper(isolate, value)) {
return true; return true;
} }
*error_message =
if (!(((repr == HeapType::kEq || repr == HeapType::kAny) && "dataref object must be null (if nullable) or a wasm "
value->IsSmi()) || "struct/array";
(repr == HeapType::kAny && value->IsString()) || return false;
(repr != HeapType::kArray && value->IsWasmStruct()) || }
value->IsWasmArray())) { case HeapType::kArray: {
*error_message = "object incompatible with wasm type"; if ((v8_flags.wasm_gc_js_interop ||
return false; TryUnpackObjectWrapper(isolate, value)) &&
value->IsWasmArray()) {
return true;
} }
return true; *error_message =
"arrayref object must be null (if nullable) or a wasm array";
return false;
}
case HeapType::kEq: {
if (value->IsSmi() ||
(v8_flags.wasm_gc_js_interop
? value->IsWasmStruct() || value->IsWasmArray()
: TryUnpackObjectWrapper(isolate, value))) {
return true;
}
*error_message =
"eqref object must be null (if nullable) or a wasm "
"i31/struct/array";
return false;
}
case HeapType::kI31: {
if (value->IsSmi()) return true;
*error_message =
"i31ref object must be null (if nullable) or a wasm i31";
return false;
} }
case HeapType::kString: case HeapType::kString:
if (value->IsString()) return true; if (value->IsString()) return true;
...@@ -2525,11 +2542,6 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module, ...@@ -2525,11 +2542,6 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
} }
} }
case kRtt: case kRtt:
// TODO(7748): Implement when the JS API for rtts is decided on.
*error_message =
"passing rtts between Webassembly and Javascript is not supported "
"yet.";
return false;
case kI8: case kI8:
case kI16: case kI16:
case kI32: case kI32:
......
...@@ -344,8 +344,12 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js"); ...@@ -344,8 +344,12 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
anyref_global.value = 12345; anyref_global.value = 12345;
assertEquals(12345, wasm.get_extern()); assertEquals(12345, wasm.get_extern());
assertThrows(() => anyref_global.value = {}, TypeError); let o = {};
assertThrows(() => anyref_global.value = undefined, TypeError); anyref_global.value = o;
assertEquals(o, anyref_global.value);
assertEquals(o, wasm.get_extern());
anyref_global.value = undefined;
assertEquals(undefined, anyref_global.value);
})(); })();
(function TestEqRefGlobalFromJS() { (function TestEqRefGlobalFromJS() {
......
...@@ -284,8 +284,8 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js'); ...@@ -284,8 +284,8 @@ d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
/Argument 1 must be specified for non-nullable element type/); /Argument 1 must be specified for non-nullable element type/);
wasmTable.grow(1, instance.exports.create_struct(33)); wasmTable.grow(1, instance.exports.create_struct(33));
assertEquals(33, instance.exports.struct_getter(4)); assertEquals(33, instance.exports.struct_getter(4));
assertThrows(() => wasmTable.set(4, undefined), TypeError, // undefined is ok for (ref any), but not null.
/Argument 1 is invalid/); wasmTable.set(4, undefined);
assertThrows(() => wasmTable.set(4, null), TypeError, assertThrows(() => wasmTable.set(4, null), TypeError,
/Argument 1 is invalid/); /Argument 1 is invalid/);
})(); })();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment