Skip to content

Commit

Permalink
[WASM-Function-References] call_ref should subtype-check its arguments
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264649

Reviewed by Justin Michaud.

Fixes a few issues with call_ref, mainly that it should subtype check its
arguments. Also fixes other GC-related issues with call_ref such as needing
to check that the target typedef is a function type, and using subtyping
for the funcref argument itself.

Also adds FIXME comments for a related bug for the runtime part.

* JSTests/wasm/function-references/call_ref.js:
(async invalidTypeIndex):
* JSTests/wasm/gc/call_ref.js: Added.
(testRefSubtyping):
(testArgSubtyping):
(testTypeDefCheck):
* Source/JavaScriptCore/wasm/WasmFormat.h:
(JSC::Wasm::isSubtypeIndex):
* Source/JavaScriptCore/wasm/WasmFunctionParser.h:
(JSC::Wasm::FunctionParser<Context>::parseExpression):
* Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:
(JSC::LLInt::doWasmCallIndirect):
(JSC::LLInt::doWasmCallRef):

Canonical link: https://commits.webkit.org/271780@main
  • Loading branch information
takikawa committed Dec 9, 2023
1 parent 9937d7e commit c64fed2
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 6 deletions.
2 changes: 1 addition & 1 deletion JSTests/wasm/function-references/call_ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ async function invalidTypeIndex() {
assert.throws(
() => new WebAssembly.Instance(module("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x0a\x02\x60\x01\x7f\x01\x7f\x60\x00\x01\x7f\x03\x03\x02\x00\x01\x07\x08\x01\x04\x6d\x61\x69\x6e\x00\x01\x09\x05\x01\x03\x00\x01\x00\x0a\x12\x02\x07\x00\x20\x00\x41\x13\x6a\x0b\x08\x00\x41\x0a\xd2\x00\x14\x01\x0b")),
WebAssembly.CompileError,
"invalid type index for call_ref value"
"invalid type for call_ref value, expected (ref null <func:1>) got (ref <func:0>), in function at index 1"
);
}

Expand Down
46 changes: 46 additions & 0 deletions JSTests/wasm/gc/call_ref.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//@ runWebAssemblySuite("--useWebAssemblyTypedFunctionReferences=true", "--useWebAssemblyGC=true")

import * as assert from "../assert.js";
import { compile, instantiate } from "./wast-wrapper.js";

function testRefSubtyping() {
// A call to a subtype should validate.
instantiate(`
(module
(type (sub (func (param i32))))
(type (sub 0 (func (param i32))))
(global (ref 1) (ref.func 0))
(func (type 1))
(func (call_ref 0 (i32.const 3) (global.get 0)))
)
`);
}

function testArgSubtyping() {
// Ensure that call_ref uses subtyping for arguments.
instantiate(`
(module
(func (param eqref))
(global (ref 0) (ref.func 0))
(func (call_ref 0 (ref.i31 (i32.const 42)) (global.get 0)))
)
`);
}

function testTypeDefCheck() {
// Non-func typedefs are invalid.
assert.throws(
() => instantiate(`
(module
(type (struct))
(func (call_ref 0 (ref.null func)))
)
`),
WebAssembly.CompileError,
"WebAssembly.Module doesn't validate: invalid type index (not a function signature) for call_ref, got 0, in function at index 0"
);
}

testRefSubtyping();
testArgSubtyping();
testTypeDefCheck();
4 changes: 4 additions & 0 deletions Source/JavaScriptCore/wasm/WasmFormat.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ inline bool isSubtypeIndex(TypeIndex sub, TypeIndex parent)
if (sub == parent)
return true;

// When Wasm GC is off, RTTs are not registered and there is no subtyping on typedefs.
if (!Options::useWebAssemblyGC())
return false;

auto subRTT = TypeInformation::tryGetCanonicalRTT(sub);
auto parentRTT = TypeInformation::tryGetCanonicalRTT(parent);
ASSERT(subRTT.has_value() && parentRTT.has_value());
Expand Down
12 changes: 7 additions & 5 deletions Source/JavaScriptCore/wasm/WasmFunctionParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -2820,12 +2820,14 @@ FOR_EACH_WASM_MEMORY_STORE_OP(CREATE_CASE)
WASM_VALIDATOR_FAIL_IF(typeIndex >= m_info.typeCount(), "call_ref index ", typeIndex, " is out of bounds");

WASM_PARSER_FAIL_IF(m_expressionStack.isEmpty(), "can't call_ref on empty expression stack");
WASM_VALIDATOR_FAIL_IF(!isRefWithTypeIndex(m_expressionStack.last().type()), "non-funcref call_ref value ", m_expressionStack.last().type().kind);
WASM_VALIDATOR_FAIL_IF(m_expressionStack.last().type().index != m_info.typeSignatures[typeIndex]->index(), "invalid type index for call_ref value");

const TypeIndex calleeTypeIndex = m_expressionStack.last().type().index;
const TypeDefinition& typeDefinition = TypeInformation::get(calleeTypeIndex);
const TypeDefinition& typeDefinition = m_info.typeSignatures[typeIndex];
const TypeIndex calleeTypeIndex = typeDefinition.index();
WASM_VALIDATOR_FAIL_IF(!typeDefinition.expand().is<FunctionSignature>(), "invalid type index (not a function signature) for call_ref, got ", typeIndex);
const auto& calleeSignature = *typeDefinition.expand().as<FunctionSignature>();
Type calleeType = Type { TypeKind::RefNull, calleeTypeIndex };
WASM_VALIDATOR_FAIL_IF(!isSubtype(m_expressionStack.last().type(), calleeType), "invalid type for call_ref value, expected ", calleeType, " got ", m_expressionStack.last().type());

size_t argumentCount = calleeSignature.argumentCount() + 1; // Add the callee's value.
WASM_PARSER_FAIL_IF(argumentCount > m_expressionStack.size(), "call_ref expects ", argumentCount, " arguments, but the expression stack currently holds ", m_expressionStack.size(), " values");

Expand All @@ -2837,7 +2839,7 @@ FOR_EACH_WASM_MEMORY_STORE_OP(CREATE_CASE)
size_t stackIndex = m_expressionStack.size() - i - 1;
TypedExpression arg = m_expressionStack.at(stackIndex);
if (i > 0)
WASM_VALIDATOR_FAIL_IF(arg.type() != calleeSignature.argumentType(argumentCount - i - 1), "argument type mismatch in call_indirect, got ", arg.type(), ", expected ", calleeSignature.argumentType(argumentCount - i - 1));
WASM_VALIDATOR_FAIL_IF(!isSubtype(arg.type(), calleeSignature.argumentType(argumentCount - i - 1)), "argument type mismatch in call_ref, got ", arg.type(), ", expected ", calleeSignature.argumentType(argumentCount - i - 1));
args[args.size() - i - 1] = arg;
m_context.didPopValueFromStack(arg, "CallRef"_s);
}
Expand Down
2 changes: 2 additions & 0 deletions Source/JavaScriptCore/wasm/WasmSlowPaths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,7 @@ inline UGPRPair doWasmCallIndirect(CallFrame* callFrame, Wasm::Instance* instanc
WASM_THROW(Wasm::ExceptionType::NullTableEntry);

const auto& callSignature = CALLEE()->signature(typeIndex);
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=260820
if (callSignature.index() != function.m_function.typeIndex)
WASM_THROW(Wasm::ExceptionType::BadSignature);

Expand Down Expand Up @@ -656,6 +657,7 @@ inline UGPRPair doWasmCallRef(CallFrame* callFrame, Wasm::Instance* callerInstan
Wasm::WasmToWasmImportableFunction function = wasmFunction->importableFunction();
Wasm::Instance* calleeInstance = &wasmFunction->instance()->instance();

// FIXME: https://bugs.webkit.org/show_bug.cgi?id=260820
ASSERT(function.typeIndex == CALLEE()->signature(typeIndex).index());
UNUSED_PARAM(typeIndex);
WASM_CALL_RETURN(calleeInstance, function.entrypointLoadLocation->taggedPtr(), WasmEntryPtrTag);
Expand Down

0 comments on commit c64fed2

Please sign in to comment.