[wasm-gc] Change MatchesSignature to use canonical types

Specifically, the methods in {WasmJSFunction} and {WasmCapiFunction}.
Drive-by:
- Fix a bug in {WasmCapiFunction::GetSignature}.
- Fix a bug in wasm-module-builder.js.

Bug: v8:7748
Change-Id: I7408d07766536ed37f23b97ad210212b986412bf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4079097
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84728}
This commit is contained in:
Manos Koukoutos 2022-12-08 12:55:25 +01:00 committed by V8 LUCI CQ
parent 350f57281a
commit ba6be821c9
6 changed files with 107 additions and 84 deletions

View File

@ -7967,7 +7967,7 @@ WasmImportData ResolveWasmImportCall(
if (WasmJSFunction::IsWasmJSFunction(*callable)) {
auto js_function = Handle<WasmJSFunction>::cast(callable);
suspend = js_function->GetSuspend();
if (!js_function->MatchesSignature(expected_sig)) {
if (!js_function->MatchesSignature(expected_canonical_type_index)) {
return {WasmImportCallKind::kLinkError, callable, wasm::kNoSuspend};
}
// Resolve the short-cut to the underlying callable and continue.
@ -7975,7 +7975,7 @@ WasmImportData ResolveWasmImportCall(
}
if (WasmCapiFunction::IsWasmCapiFunction(*callable)) {
auto capi_function = Handle<WasmCapiFunction>::cast(callable);
if (!capi_function->MatchesSignature(expected_sig)) {
if (!capi_function->MatchesSignature(expected_canonical_type_index)) {
return {WasmImportCallKind::kLinkError, callable, wasm::kNoSuspend};
}
return {WasmImportCallKind::kWasmToCapi, callable, wasm::kNoSuspend};

View File

@ -2031,7 +2031,10 @@ void WebAssemblyFunction(const v8::FunctionCallbackInfo<v8::Value>& args) {
bool is_wasm_js_function = i::WasmJSFunction::IsWasmJSFunction(*callable);
if (is_wasm_exported_function && !suspend && !promise) {
if (*i::Handle<i::WasmExportedFunction>::cast(callable)->sig() == *sig) {
uint32_t canonical_sig_index =
i::wasm::GetWasmEngine()->type_canonicalizer()->AddRecursiveGroup(sig);
if (i::Handle<i::WasmExportedFunction>::cast(callable)->MatchesSignature(
canonical_sig_index)) {
args.GetReturnValue().Set(Utils::ToLocal(callable));
return;
}
@ -2043,7 +2046,10 @@ void WebAssemblyFunction(const v8::FunctionCallbackInfo<v8::Value>& args) {
}
if (is_wasm_js_function && !suspend && !promise) {
if (i::Handle<i::WasmJSFunction>::cast(callable)->MatchesSignature(sig)) {
uint32_t canonical_sig_index =
i::wasm::GetWasmEngine()->type_canonicalizer()->AddRecursiveGroup(sig);
if (i::Handle<i::WasmJSFunction>::cast(callable)->MatchesSignature(
canonical_sig_index)) {
args.GetReturnValue().Set(Utils::ToLocal(callable));
return;
}

View File

@ -1609,7 +1609,7 @@ Handle<WasmTagObject> WasmTagObject::New(Isolate* isolate,
return tag_wrapper;
}
// TODO(9495): Update this if function type variance is introduced.
// TODO(7748): Integrate this with type canonicalization.
bool WasmTagObject::MatchesSignature(const wasm::FunctionSig* sig) {
DCHECK_EQ(0, sig->return_count());
DCHECK_LE(sig->parameter_count(), std::numeric_limits<int>::max());
@ -1623,10 +1623,11 @@ bool WasmTagObject::MatchesSignature(const wasm::FunctionSig* sig) {
return true;
}
const wasm::FunctionSig* WasmCapiFunction::GetSignature(Zone* zone) {
const wasm::FunctionSig* WasmCapiFunction::GetSignature(Zone* zone) const {
WasmCapiFunctionData function_data = shared().wasm_capi_function_data();
auto serialized_sig = function_data.serialized_signature();
auto sig_size = serialized_sig.Size() - 1;
PodArray<wasm::ValueType> serialized_sig =
function_data.serialized_signature();
int sig_size = serialized_sig.length() - 1;
wasm::ValueType* types = zone->NewArray<wasm::ValueType>(sig_size);
int returns_size = 0;
int index = 0;
@ -1634,7 +1635,7 @@ const wasm::FunctionSig* WasmCapiFunction::GetSignature(Zone* zone) {
types[index] = serialized_sig.get(index);
index++;
}
returns_size = index - 1;
returns_size = index;
while (index < sig_size) {
types[index] = serialized_sig.get(index + 1);
index++;
@ -1644,25 +1645,19 @@ const wasm::FunctionSig* WasmCapiFunction::GetSignature(Zone* zone) {
types);
}
bool WasmCapiFunction::MatchesSignature(const wasm::FunctionSig* sig) const {
// TODO(jkummerow): Unify with "SignatureHelper" in c-api.cc.
int param_count = static_cast<int>(sig->parameter_count());
int result_count = static_cast<int>(sig->return_count());
PodArray<wasm::ValueType> serialized_sig =
shared().wasm_capi_function_data().serialized_signature();
if (param_count + result_count + 1 != serialized_sig.length()) return false;
int serialized_index = 0;
for (int i = 0; i < result_count; i++, serialized_index++) {
if (sig->GetReturn(i) != serialized_sig.get(serialized_index)) {
return false;
}
}
if (serialized_sig.get(serialized_index) != wasm::kWasmVoid) return false;
serialized_index++;
for (int i = 0; i < param_count; i++, serialized_index++) {
if (sig->GetParam(i) != serialized_sig.get(serialized_index)) return false;
}
return true;
bool WasmCapiFunction::MatchesSignature(
uint32_t other_canonical_sig_index) const {
AccountingAllocator allocator;
Zone zone(&allocator, ZONE_NAME);
const wasm::FunctionSig* sig = GetSignature(&zone);
#if DEBUG
// TODO(7748): Change this if indexed types are allowed.
for (wasm::ValueType type : sig->all()) CHECK(!type.has_index());
#endif
// TODO(7748): Check for subtyping instead if C API functions can define
// signature supertype.
return wasm::GetWasmEngine()->type_canonicalizer()->AddRecursiveGroup(sig) ==
other_canonical_sig_index;
}
// static
@ -2136,7 +2131,7 @@ wasm::Suspend WasmJSFunction::GetSuspend() const {
.suspend());
}
const wasm::FunctionSig* WasmJSFunction::GetSignature(Zone* zone) {
const wasm::FunctionSig* WasmJSFunction::GetSignature(Zone* zone) const {
WasmJSFunctionData function_data = shared().wasm_js_function_data();
int sig_size = function_data.serialized_signature().length();
wasm::ValueType* types = zone->NewArray<wasm::ValueType>(sig_size);
@ -2148,21 +2143,19 @@ const wasm::FunctionSig* WasmJSFunction::GetSignature(Zone* zone) {
return zone->New<wasm::FunctionSig>(return_count, parameter_count, types);
}
// TODO(9495): Update this if function type variance is introduced.
bool WasmJSFunction::MatchesSignature(const wasm::FunctionSig* sig) {
DCHECK_LE(sig->all().size(), kMaxInt);
int sig_size = static_cast<int>(sig->all().size());
int return_count = static_cast<int>(sig->return_count());
int parameter_count = static_cast<int>(sig->parameter_count());
DisallowHeapAllocation no_alloc;
WasmJSFunctionData function_data = shared().wasm_js_function_data();
if (return_count != function_data.serialized_return_count() ||
parameter_count != function_data.serialized_parameter_count()) {
return false;
}
if (sig_size == 0) return true; // Prevent undefined behavior.
const wasm::ValueType* expected = sig->all().begin();
return function_data.serialized_signature().matches(expected, sig_size);
bool WasmJSFunction::MatchesSignature(
uint32_t other_canonical_sig_index) const {
AccountingAllocator allocator;
Zone zone(&allocator, ZONE_NAME);
const wasm::FunctionSig* sig = GetSignature(&zone);
#if DEBUG
// TODO(7748): Change this if indexed types are allowed.
for (wasm::ValueType type : sig->all()) CHECK(!type.has_index());
#endif
// TODO(7748): Check for subtyping instead if WebAssembly.Function can define
// signature supertype.
return wasm::GetWasmEngine()->type_canonicalizer()->AddRecursiveGroup(sig) ==
other_canonical_sig_index;
}
PodArray<wasm::ValueType> WasmCapiFunction::GetSerializedSignature() const {
@ -2378,16 +2371,8 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
}
return WasmInternalFunction::FromExternal(value, isolate);
} else if (WasmJSFunction::IsWasmJSFunction(*value)) {
AccountingAllocator allocator;
Zone zone(&allocator, ZONE_NAME);
// Since a WasmJSFunction cannot refer to indexed types (definable
// only in a module), we do not need full function subtyping.
// TODO(7748): Change this if wasm types can be exported.
const FunctionSig* real_sig =
WasmJSFunction::cast(*value).GetSignature(&zone);
uint32_t real_canonical_index =
type_canonicalizer->AddRecursiveGroup(real_sig);
if (real_canonical_index != expected_canonical.ref_index()) {
if (!Handle<WasmJSFunction>::cast(value)->MatchesSignature(
expected_canonical.ref_index())) {
*error_message =
"assigned WebAssembly.Function has to be a subtype of the "
"expected type";
@ -2395,20 +2380,8 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
}
return WasmInternalFunction::FromExternal(value, isolate);
} else if (WasmCapiFunction::IsWasmCapiFunction(*value)) {
// Since a WasmCapiFunction cannot refer to indexed types
// (definable only in a module), we do not need full function
// subtyping.
// TODO(7748): Change this if wasm types can be exported.
AccountingAllocator allocator;
Zone zone(&allocator, ZONE_NAME);
// Since a WasmJSFunction cannot refer to indexed types (definable
// only in a module), we do not need full function subtyping.
// TODO(7748): Change this if wasm types can be exported.
const FunctionSig* real_sig =
WasmCapiFunction::cast(*value).GetSignature(&zone);
uint32_t real_canonical_index =
type_canonicalizer->AddRecursiveGroup(real_sig);
if (real_canonical_index != expected_canonical.ref_index()) {
if (!Handle<WasmCapiFunction>::cast(value)->MatchesSignature(
expected_canonical.ref_index())) {
*error_message =
"assigned C API function has to be a subtype of the expected "
"type";

View File

@ -643,8 +643,8 @@ class WasmJSFunction : public JSFunction {
wasm::Suspend GetSuspend() const;
// Deserializes the signature of this function using the provided zone. Note
// that lifetime of the signature is hence directly coupled to the zone.
const wasm::FunctionSig* GetSignature(Zone* zone);
bool MatchesSignature(const wasm::FunctionSig* sig);
const wasm::FunctionSig* GetSignature(Zone* zone) const;
bool MatchesSignature(uint32_t other_canonical_sig_index) const;
DECL_CAST(WasmJSFunction)
OBJECT_CONSTRUCTORS(WasmJSFunction, JSFunction);
@ -662,8 +662,8 @@ class WasmCapiFunction : public JSFunction {
PodArray<wasm::ValueType> GetSerializedSignature() const;
// Checks whether the given {sig} has the same parameter types as the
// serialized signature stored within this C-API function object.
bool MatchesSignature(const wasm::FunctionSig* sig) const;
const wasm::FunctionSig* GetSignature(Zone* zone);
bool MatchesSignature(uint32_t other_canonical_sig_index) const;
const wasm::FunctionSig* GetSignature(Zone* zone) const;
DECL_CAST(WasmCapiFunction)
OBJECT_CONSTRUCTORS(WasmCapiFunction, JSFunction);

View File

@ -157,18 +157,61 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
return builder.instantiate({});
})();
var instance = (function () {
var builder = new WasmModuleBuilder();
// These should canonicalize to the same types as the exporting instance.
let super_struct = builder.addStruct([makeField(kWasmI32, true)]);
let sub_struct = builder.addStruct(
[makeField(kWasmI32, true), makeField(kWasmI64, true)], super_struct);
let super_sig = builder.addType(makeSig([wasmRefNullType(sub_struct)],
[kWasmI32]))
builder.addImport("m", "f", super_sig);
// Import is a function of the declared type.
return builder.instantiate({m: {f:
exporting_instance.exports.exported_function}});
})();
(function TestJSFunctionCanonicallyDifferent() {
print(arguments.callee.name);
let imp = new WebAssembly.Function({parameters: ["i32"], results: ["i32"]},
x => x + 1);
(function () {
var builder = new WasmModuleBuilder();
// These should canonicalize to the same types as the exporting instance.
let super_struct = builder.addStruct([makeField(kWasmI32, true)]);
let sub_struct = builder.addStruct(
[makeField(kWasmI32, true), makeField(kWasmI64, true)], super_struct);
let super_sig = builder.addType(makeSig([wasmRefNullType(sub_struct)],
[kWasmI32]))
builder.addImport("m", "f", super_sig);
let sig = builder.addType(kSig_i_i);
builder.addImport("m", "f", sig);
// This succeeds
builder.instantiate({m: {f: imp}});
})();
(function () {
var builder = new WasmModuleBuilder();
let sig = builder.addType(kSig_i_i);
let sig_sub = builder.addType(kSig_i_i, sig);
builder.addImport("m", "f", sig_sub);
// Import is a function of the declared type.
return builder.instantiate({m: {f:
exporting_instance.exports.exported_function}});
assertThrows(() => builder.instantiate({m: {f: imp}}),
WebAssembly.LinkError,
/imported function does not match the expected type/);
})();
(function () {
var builder = new WasmModuleBuilder();
builder.startRecGroup();
let sig_in_group = builder.addType(kSig_i_i);
builder.addType(kSig_i_v);
builder.endRecGroup();
builder.addImport("m", "f", sig_in_group);
// Import is a function of the declared type.
assertThrows(() => builder.instantiate({m: {f: imp}}),
WebAssembly.LinkError,
/imported function does not match the expected type/);
})();
})();

View File

@ -1340,8 +1340,9 @@ class WasmModuleBuilder {
addType(type, supertype_idx = kNoSuperType) {
var pl = type.params.length; // should have params
var rl = type.results.length; // should have results
type.supertype = supertype_idx;
this.types.push(type);
var type_copy = {params: type.params, results: type.results,
supertype: supertype_idx};
this.types.push(type_copy);
return this.types.length - 1;
}