Skip to content

Commit

Permalink
[JSC] SIMD wasm functions should throw TypeError when it is called fr…
Browse files Browse the repository at this point in the history
…om JS

https://bugs.webkit.org/show_bug.cgi?id=248866

Reviewed by Alexey Shvayka.

This patch fixes the following two issues related to V128 in JS functions.

1. WebAssemblyWrapperFunction wrapping imported host functions should also throw TypeError when its signature includes V128.
2. WebAssemblyFunction should throw TypeError when V128 is included in return-type too. Previously we are only checking argument types.

* JSTests/wasm/v8/regress/regress-9447.js:
* Source/JavaScriptCore/wasm/WasmTypeDefinition.cpp:
(JSC::Wasm::TypeInformation::TypeInformation):
(JSC::Wasm::FunctionParameterTypes::translate):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.h:
(JSC::Wasm::FunctionSignature::FunctionSignature):
(JSC::Wasm::FunctionSignature::argumentsOrResultsIncludeV128 const):
(JSC::Wasm::FunctionSignature::setArgumentsOrResultsIncludeV128):
* Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/wasm/js/WebAssemblyWrapperFunction.cpp:
(JSC::WebAssemblyWrapperFunction::create):
(JSC::JSC_DEFINE_HOST_FUNCTION):

Canonical link: https://commits.webkit.org/257494@main
  • Loading branch information
Constellation committed Dec 7, 2022
1 parent e9b7bb9 commit ba93223
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 9 deletions.
5 changes: 2 additions & 3 deletions JSTests/wasm/v8/regress/regress-9447.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//@ skip
//@ skip if $architecture != "arm64"
//@ requireOptions("--useWebAssemblySIMD=1")
// Copyright 2019 the V8 project authors. All rights reserved.
Expand Down Expand Up @@ -38,6 +37,6 @@ var fun2 = (function GenerateFun2() {

// Both exported functions should throw, no matter how often they get wrapped.
assertThrows(fun1, TypeError,
/type incompatibility when transforming from\/to JS/);
/an exported wasm function cannot contain a v128 parameter or return value/);
assertThrows(fun2, TypeError,
/type incompatibility when transforming from\/to JS/);
/an exported wasm function cannot contain a v128 parameter or return value/);
6 changes: 6 additions & 0 deletions Source/JavaScriptCore/wasm/WasmTypeDefinition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@ TypeInformation::TypeInformation()
RefPtr<TypeDefinition> sig = TypeDefinition::tryCreateFunctionSignature(1, 0); \
sig->ref(); \
sig->as<FunctionSignature>()->getReturnType(0) = Types::type; \
if (Types::type.isV128()) \
sig->as<FunctionSignature>()->setArgumentsOrResultsIncludeV128(true); \
thunkTypes[linearizeType(TypeKind::type)] = sig.get(); \
m_typeSet.add(TypeHash { sig.releaseNonNull() }); \
} \
Expand Down Expand Up @@ -559,18 +561,22 @@ struct FunctionParameterTypes {
RefPtr<TypeDefinition> signature = TypeDefinition::tryCreateFunctionSignature(params.returnTypes.size(), params.argumentTypes.size());
RELEASE_ASSERT(signature);
bool hasRecursiveReference = false;
bool argumentsOrResultsIncludeV128 = false;

for (unsigned i = 0; i < params.returnTypes.size(); ++i) {
signature->as<FunctionSignature>()->getReturnType(i) = params.returnTypes[i];
hasRecursiveReference |= isRefWithRecursiveReference(params.returnTypes[i]);
argumentsOrResultsIncludeV128 |= params.returnTypes[i].isV128();
}

for (unsigned i = 0; i < params.argumentTypes.size(); ++i) {
signature->as<FunctionSignature>()->getArgumentType(i) = params.argumentTypes[i];
hasRecursiveReference |= isRefWithRecursiveReference(params.argumentTypes[i]);
argumentsOrResultsIncludeV128 |= params.argumentTypes[i].isV128();
}

signature->as<FunctionSignature>()->setHasRecursiveReference(hasRecursiveReference);
signature->as<FunctionSignature>()->setArgumentsOrResultsIncludeV128(argumentsOrResultsIncludeV128);

entry.key = WTFMove(signature);
}
Expand Down
6 changes: 4 additions & 2 deletions Source/JavaScriptCore/wasm/WasmTypeDefinition.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ class FunctionSignature {
: m_payload(payload)
, m_argCount(argumentCount)
, m_retCount(returnCount)
, m_hasRecursiveReference(false)
{
}

Expand All @@ -160,6 +159,8 @@ class FunctionSignature {
Type returnType(FunctionArgCount i) const { ASSERT(i < returnCount()); return const_cast<FunctionSignature*>(this)->getReturnType(i); }
bool returnsVoid() const { return !returnCount(); }
Type argumentType(FunctionArgCount i) const { return const_cast<FunctionSignature*>(this)->getArgumentType(i); }
bool argumentsOrResultsIncludeV128() const { return m_argumentsOrResultsIncludeV128; }
void setArgumentsOrResultsIncludeV128(bool value) { m_argumentsOrResultsIncludeV128 = value; }

size_t numVectors() const
{
Expand Down Expand Up @@ -205,7 +206,8 @@ class FunctionSignature {
Type* m_payload;
FunctionArgCount m_argCount;
FunctionArgCount m_retCount;
bool m_hasRecursiveReference;
bool m_hasRecursiveReference { false };
bool m_argumentsOrResultsIncludeV128 { false };
};

// FIXME auto-generate this. https://bugs.webkit.org/show_bug.cgi?id=165231
Expand Down
5 changes: 3 additions & 2 deletions Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ JSC_DEFINE_HOST_FUNCTION(callWebAssemblyFunction, (JSGlobalObject* globalObject,
JSWebAssemblyInstance* instance = wasmFunction->instance();
Wasm::Instance* wasmInstance = &instance->instance();

if (signature.argumentsOrResultsIncludeV128())
return throwVMTypeError(globalObject, scope, Wasm::errorMessageForExceptionType(Wasm::ExceptionType::TypeErrorInvalidV128Use));

for (unsigned argIndex = 0; argIndex < signature.argumentCount(); ++argIndex) {
if (signature.argumentType(argIndex).isV128())
return JSValue::encode(throwException(globalObject, scope, createTypeError(globalObject, Wasm::errorMessageForExceptionType(Wasm::ExceptionType::TypeErrorInvalidV128Use))));
uint64_t value = fromJSValue(globalObject, signature.argumentType(argIndex), callFrame->argument(argIndex));
RETURN_IF_EXCEPTION(scope, encodedJSValue());
boxedArgs.append(JSValue::decode(value));
Expand Down
18 changes: 16 additions & 2 deletions Source/JavaScriptCore/wasm/js/WebAssemblyWrapperFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ namespace JSC {
const ClassInfo WebAssemblyWrapperFunction::s_info = { "WebAssemblyWrapperFunction"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(WebAssemblyWrapperFunction) };

static JSC_DECLARE_HOST_FUNCTION(callWebAssemblyWrapperFunction);
static JSC_DECLARE_HOST_FUNCTION(callWebAssemblyWrapperFunctionIncludingV128);

WebAssemblyWrapperFunction::WebAssemblyWrapperFunction(VM& vm, NativeExecutable* executable, JSGlobalObject* globalObject, Structure* structure, Wasm::WasmToWasmImportableFunction importableFunction)
: Base(vm, executable, globalObject, structure, importableFunction)
Expand All @@ -45,10 +46,16 @@ WebAssemblyWrapperFunction::WebAssemblyWrapperFunction(VM& vm, NativeExecutable*
WebAssemblyWrapperFunction* WebAssemblyWrapperFunction::create(VM& vm, JSGlobalObject* globalObject, Structure* structure, JSObject* function, unsigned importIndex, JSWebAssemblyInstance* instance, Wasm::TypeIndex typeIndex)
{
ASSERT_WITH_MESSAGE(!function->inherits<WebAssemblyWrapperFunction>(), "We should never double wrap a wrapper function.");

String name = emptyString();
NativeExecutable* executable = vm.getHostFunction(callWebAssemblyWrapperFunction, ImplementationVisibility::Public, NoIntrinsic, callHostFunctionAsConstructor, nullptr, name);
WebAssemblyWrapperFunction* result = new (NotNull, allocateCell<WebAssemblyWrapperFunction>(vm)) WebAssemblyWrapperFunction(vm, executable, globalObject, structure, Wasm::WasmToWasmImportableFunction { typeIndex, &instance->instance().importFunctionInfo(importIndex)->wasmToEmbedderStub } );
const auto& signature = Wasm::TypeInformation::getFunctionSignature(typeIndex);
NativeExecutable* executable = nullptr;
if (signature.argumentsOrResultsIncludeV128())
executable = vm.getHostFunction(callWebAssemblyWrapperFunctionIncludingV128, ImplementationVisibility::Public, NoIntrinsic, callHostFunctionAsConstructor, nullptr, name);
else
executable = vm.getHostFunction(callWebAssemblyWrapperFunction, ImplementationVisibility::Public, NoIntrinsic, callHostFunctionAsConstructor, nullptr, name);

WebAssemblyWrapperFunction* result = new (NotNull, allocateCell<WebAssemblyWrapperFunction>(vm)) WebAssemblyWrapperFunction(vm, executable, globalObject, structure, Wasm::WasmToWasmImportableFunction { typeIndex, &instance->instance().importFunctionInfo(importIndex)->wasmToEmbedderStub });
result->finishCreation(vm, executable, signature.argumentCount(), name, function, instance);
return result;
}
Expand Down Expand Up @@ -89,6 +96,13 @@ JSC_DEFINE_HOST_FUNCTION(callWebAssemblyWrapperFunction, (JSGlobalObject* global
RELEASE_AND_RETURN(scope, JSValue::encode(call(globalObject, function, callData, jsUndefined(), ArgList(callFrame))));
}

JSC_DEFINE_HOST_FUNCTION(callWebAssemblyWrapperFunctionIncludingV128, (JSGlobalObject* globalObject, CallFrame*))
{
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
return throwVMTypeError(globalObject, scope, Wasm::errorMessageForExceptionType(Wasm::ExceptionType::TypeErrorInvalidV128Use));
}

} // namespace JSC

#endif // ENABLE(WEBASSEMBLY)

0 comments on commit ba93223

Please sign in to comment.