Skip to content

Commit

Permalink
Re-land [WASM-Function-References] Fix block signature parsing for re…
Browse files Browse the repository at this point in the history
…ftypes

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

Reviewed by Justin Michaud.

This is a re-land of this patch, with some adjustment to hopefully improve
performance in benchmarks (previous attempt backed out due to JetStream2
regression).

* JSTests/wasm/function-references/block_signature.js: Added.
(module):
(async blockSignatureTest):
* JSTests/wasm/gc-spec-tests/type-equivalence.wast.js:
* Source/JavaScriptCore/wasm/WasmFormat.h:
* Source/JavaScriptCore/wasm/WasmFunctionParser.h:
(JSC::Wasm::FunctionParser<Context>::unify):
* Source/JavaScriptCore/wasm/WasmParser.h:
(JSC::Wasm::Parser<SuccessType>::parseBlockSignature):
(JSC::Wasm::Parser<SuccessType>::parseReftypeSignature):

Canonical link: https://commits.webkit.org/266847@main
  • Loading branch information
takikawa committed Aug 12, 2023
1 parent a4d9492 commit 5ff67f8
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 9 deletions.
62 changes: 62 additions & 0 deletions JSTests/wasm/function-references/block_signature.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//@ runWebAssemblySuite("--useWebAssemblyTypedFunctionReferences=true")
import * as assert from '../assert.js';

function module(bytes, valid = true) {
let buffer = new ArrayBuffer(bytes.length);
let view = new Uint8Array(buffer);
for (let i = 0; i < bytes.length; ++i) {
view[i] = bytes.charCodeAt(i);
}
return new WebAssembly.Module(buffer);
}

async function blockSignatureTest() {
/*
(module
(type (func))
(func (type 0))
(elem declare funcref (ref.func 0))
(func (result)
(block (result (ref 0))
(ref.func 0))
drop)
)
*/
new WebAssembly.Instance(module("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x84\x80\x80\x80\x00\x01\x60\x00\x00\x03\x83\x80\x80\x80\x00\x02\x00\x00\x09\x87\x80\x80\x80\x00\x01\x07\x70\x01\xd2\x00\x0b\x0a\x96\x80\x80\x80\x00\x02\x82\x80\x80\x80\x00\x00\x0b\x89\x80\x80\x80\x00\x00\x02\x6b\x00\xd2\x00\x0b\x1a\x0b"));

/*
(module
(type (func))
(func (result)
(block (result (ref null 0))
(ref.null 0))
drop)
)
*/
new WebAssembly.Instance(module("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x84\x80\x80\x80\x00\x01\x60\x00\x00\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x8f\x80\x80\x80\x00\x01\x89\x80\x80\x80\x00\x00\x02\x6c\x00\xd0\x00\x0b\x1a\x0b"));

// Both shorthand and Ref encodings tested below.
/*
(module
(func (result)
(block (result (ref null extern))
(ref.null extern))
drop)
)
*/
new WebAssembly.Instance(module("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x84\x80\x80\x80\x00\x01\x60\x00\x00\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x8e\x80\x80\x80\x00\x01\x88\x80\x80\x80\x00\x00\x02\x6f\xd0\x6f\x0b\x1a\x0b"));
new WebAssembly.Instance(module("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x84\x80\x80\x80\x00\x01\x60\x00\x00\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x8f\x80\x80\x80\x00\x01\x89\x80\x80\x80\x00\x00\x02\x6c\x6f\xd0\x6f\x0b\x1a\x0b"));

/*
(module
(func (result)
(block (result (ref null func))
(ref.null func))
drop)
)
*/
new WebAssembly.Instance(module("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x84\x80\x80\x80\x00\x01\x60\x00\x00\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x8e\x80\x80\x80\x00\x01\x88\x80\x80\x80\x00\x00\x02\x70\xd0\x70\x0b\x1a\x0b"));
new WebAssembly.Instance(module("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x84\x80\x80\x80\x00\x01\x60\x00\x00\x03\x82\x80\x80\x80\x00\x01\x00\x0a\x8f\x80\x80\x80\x00\x01\x89\x80\x80\x80\x00\x00\x02\x6c\x70\xd0\x70\x0b\x1a\x0b"));
}

assert.asyncTest(blockSignatureTest());
6 changes: 2 additions & 4 deletions JSTests/wasm/gc-spec-tests/type-equivalence.wast.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,11 @@ let $7 = instance("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x9e\x80\x80\x80\x00\x06\
// type-equivalence.wast:131
assert_return(() => call($7, "run", []));

// FIXME: https://bugs.webkit.org/show_bug.cgi?id=247383
// type-equivalence.wast:136
//let $8 = instance("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x8e\x80\x80\x80\x00\x03\x60\x00\x01\x6c\x00\x60\x00\x01\x6c\x01\x60\x00\x00\x03\x84\x80\x80\x80\x00\x03\x00\x01\x02\x04\x85\x80\x80\x80\x00\x01\x70\x01\x02\x02\x07\x87\x80\x80\x80\x00\x01\x03\x72\x75\x6e\x00\x02\x09\x8e\x80\x80\x80\x00\x01\x06\x00\x41\x00\x0b\x70\x02\xd2\x00\x0b\xd2\x01\x0b\x0a\xe4\x80\x80\x80\x00\x03\x84\x80\x80\x80\x00\x00\xd0\x00\x0b\x84\x80\x80\x80\x00\x00\xd0\x01\x0b\xcc\x80\x80\x80\x00\x00\x02\x6c\x00\x41\x00\x11\x00\x00\x0b\x02\x6c\x00\x41\x00\x11\x01\x00\x0b\x02\x6c\x01\x41\x00\x11\x00\x00\x0b\x02\x6c\x01\x41\x00\x11\x01\x00\x0b\x02\x6c\x00\x41\x01\x11\x00\x00\x0b\x02\x6c\x00\x41\x01\x11\x01\x00\x0b\x02\x6c\x01\x41\x01\x11\x00\x00\x0b\x02\x6c\x01\x41\x01\x11\x01\x00\x0b\x0c\x00\x0b");
let $8 = instance("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x8e\x80\x80\x80\x00\x03\x60\x00\x01\x6c\x00\x60\x00\x01\x6c\x01\x60\x00\x00\x03\x84\x80\x80\x80\x00\x03\x00\x01\x02\x04\x85\x80\x80\x80\x00\x01\x70\x01\x02\x02\x07\x87\x80\x80\x80\x00\x01\x03\x72\x75\x6e\x00\x02\x09\x8e\x80\x80\x80\x00\x01\x06\x00\x41\x00\x0b\x70\x02\xd2\x00\x0b\xd2\x01\x0b\x0a\xe4\x80\x80\x80\x00\x03\x84\x80\x80\x80\x00\x00\xd0\x00\x0b\x84\x80\x80\x80\x00\x00\xd0\x01\x0b\xcc\x80\x80\x80\x00\x00\x02\x6c\x00\x41\x00\x11\x00\x00\x0b\x02\x6c\x00\x41\x00\x11\x01\x00\x0b\x02\x6c\x01\x41\x00\x11\x00\x00\x0b\x02\x6c\x01\x41\x00\x11\x01\x00\x0b\x02\x6c\x00\x41\x01\x11\x00\x00\x0b\x02\x6c\x00\x41\x01\x11\x01\x00\x0b\x02\x6c\x01\x41\x01\x11\x00\x00\x0b\x02\x6c\x01\x41\x01\x11\x01\x00\x0b\x0c\x00\x0b");

// FIXME: https://bugs.webkit.org/show_bug.cgi?id=247383
// type-equivalence.wast:156
//assert_return(() => call($8, "run", []));
assert_return(() => call($8, "run", []));

// type-equivalence.wast:161
let $9 = instance("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\xac\x80\x80\x80\x00\x03\x4f\x03\x60\x02\x7f\x6b\x00\x00\x60\x02\x7f\x6b\x02\x00\x60\x02\x7f\x6b\x01\x00\x4f\x03\x60\x02\x7f\x6b\x03\x00\x60\x02\x7f\x6b\x05\x00\x60\x02\x7f\x6b\x04\x00\x60\x00\x00\x03\x85\x80\x80\x80\x00\x04\x00\x01\x02\x06\x04\x85\x80\x80\x80\x00\x01\x70\x01\x03\x03\x07\x87\x80\x80\x80\x00\x01\x03\x72\x75\x6e\x00\x03\x09\x91\x80\x80\x80\x00\x01\x06\x00\x41\x00\x0b\x70\x03\xd2\x00\x0b\xd2\x01\x0b\xd2\x02\x0b\x0a\xd3\x80\x80\x80\x00\x04\x82\x80\x80\x80\x00\x00\x0b\x82\x80\x80\x80\x00\x00\x0b\x82\x80\x80\x80\x00\x00\x0b\xb8\x80\x80\x80\x00\x00\x41\x01\xd2\x00\x41\x00\x11\x00\x00\x41\x01\xd2\x02\x41\x01\x11\x01\x00\x41\x01\xd2\x01\x41\x02\x11\x02\x00\x41\x01\xd2\x00\x41\x00\x11\x03\x00\x41\x01\xd2\x02\x41\x01\x11\x04\x00\x41\x01\xd2\x01\x41\x02\x11\x05\x00\x0b");
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/wasm/WasmFormat.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ struct CompilationContext;
struct ModuleInformation;
struct UnlinkedHandlerInfo;

using BlockSignature = const TypeDefinition*;
using BlockSignature = RefPtr<const TypeDefinition>;

enum class TableElementType : uint8_t {
Externref,
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/wasm/WasmFunctionParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -1465,7 +1465,7 @@ auto FunctionParser<Context>::checkBranchTarget(const ControlType& target) -> Pa
template<typename Context>
auto FunctionParser<Context>::unify(const ControlType& controlData) -> PartialResult
{
const TypeDefinition* typeDefinition = controlData.signature(); // just to avoid a weird compiler error with templates at the next line.
RefPtr<const TypeDefinition> typeDefinition = controlData.signature(); // just to avoid a weird compiler error with templates at the next line.
const FunctionSignature* signature = typeDefinition->as<FunctionSignature>();
WASM_VALIDATOR_FAIL_IF(signature->returnCount() != m_expressionStack.size(), " block with type: ", signature->toString(), " returns: ", signature->returnCount(), " but stack has: ", m_expressionStack.size(), " values");
for (unsigned i = 0; i < signature->returnCount(); ++i)
Expand Down
25 changes: 22 additions & 3 deletions Source/JavaScriptCore/wasm/WasmParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class Parser {
bool WARN_UNUSED_RETURN parseVarInt64(int64_t&);

PartialResult WARN_UNUSED_RETURN parseBlockSignature(const ModuleInformation&, BlockSignature&);
PartialResult WARN_UNUSED_RETURN parseReftypeSignature(const ModuleInformation&, BlockSignature&);
bool WARN_UNUSED_RETURN parseValueType(const ModuleInformation&, Type&);
bool WARN_UNUSED_RETURN parseRefType(const ModuleInformation&, Type&);
bool WARN_UNUSED_RETURN parseExternalKind(ExternalKind&);
Expand Down Expand Up @@ -310,9 +311,16 @@ ALWAYS_INLINE bool Parser<SuccessType>::parseVarUInt1(uint8_t& result)
template<typename SuccessType>
ALWAYS_INLINE typename Parser<SuccessType>::PartialResult Parser<SuccessType>::parseBlockSignature(const ModuleInformation& info, BlockSignature& result)
{
int8_t typeKind;
if (peekInt7(typeKind) && isValidTypeKind(typeKind)) {
Type type = { static_cast<TypeKind>(typeKind), 0 };
int8_t kindByte;
if (peekInt7(kindByte) && isValidTypeKind(kindByte)) {
TypeKind typeKind = static_cast<TypeKind>(kindByte);

if (UNLIKELY(Options::useWebAssemblyTypedFunctionReferences())) {
if ((isValidHeapTypeKind(typeKind) || typeKind == TypeKind::Ref || typeKind == TypeKind::RefNull))
return parseReftypeSignature(info, result);
}

Type type = { typeKind, 0 };
WASM_PARSER_FAIL_IF(!(isValueType(type) || type.isVoid()), "result type of block: ", makeString(type.kind), " is not a value type or Void");
result = m_typeInformation.thunkFor(type);
m_offset++;
Expand All @@ -328,6 +336,17 @@ ALWAYS_INLINE typename Parser<SuccessType>::PartialResult Parser<SuccessType>::p
return { };
}

template<typename SuccessType>
typename Parser<SuccessType>::PartialResult Parser<SuccessType>::parseReftypeSignature(const ModuleInformation& info, BlockSignature& result)
{
Type resultType;
WASM_PARSER_FAIL_IF(!parseValueType(info, resultType), "result type of block is not a valid ref type");
Vector<Type, 16> returnTypes { resultType };
result = TypeInformation::typeDefinitionForFunction(returnTypes, { });

return { };
}

template<typename SuccessType>
ALWAYS_INLINE bool Parser<SuccessType>::parseHeapType(const ModuleInformation& info, int32_t& result)
{
Expand Down

0 comments on commit 5ff67f8

Please sign in to comment.