Skip to content

Commit

Permalink
Wasm element segment vector items should allow more constant expressions
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260542

Reviewed by Justin Michaud.

This patch enables the test cases in the extended constant expression proposal
spec tests that were blocked by the item limitation.

It also fixes a bug uncovered by these tests, which is that the function table
initialization path for global/constant expression cases needs to be handled
explicitly.

The spec test tests the global.get case. The constant expression case is
difficult to test in practice, because currently you can only get a function
result from a constant expression via ref.null, ref.func, and global.get which
are all fast-path special cases and avoid the full constant expression parsing.

* JSTests/wasm/extended-const-spec-tests/elem.wast.js:
* Source/JavaScriptCore/wasm/WasmInstance.cpp:
(JSC::Wasm::Instance::initElementSegment):

Canonical link: https://commits.webkit.org/273002@main
  • Loading branch information
takikawa committed Jan 13, 2024
1 parent 65e9ef2 commit f546b9f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 14 deletions.
6 changes: 2 additions & 4 deletions JSTests/wasm/extended-const-spec-tests/elem.wast.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,8 @@ let $module4 = $30;
// elem.wast:689
register("module4", $module4)

// FIXME: https://bugs.webkit.org/show_bug.cgi?id=260542
// elem.wast:691
//let $31 = instance("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x85\x80\x80\x80\x00\x01\x60\x00\x01\x7f\x02\x8e\x80\x80\x80\x00\x01\x07\x6d\x6f\x64\x75\x6c\x65\x34\x01\x66\x03\x70\x00\x03\x82\x80\x80\x80\x00\x01\x00\x04\x84\x80\x80\x80\x00\x01\x70\x00\x0a\x07\x96\x80\x80\x80\x00\x01\x12\x63\x61\x6c\x6c\x5f\x69\x6d\x70\x6f\x72\x74\x65\x64\x5f\x65\x6c\x65\x6d\x00\x00\x09\x89\x80\x80\x80\x00\x01\x04\x41\x00\x0b\x01\x23\x00\x0b\x0a\x8d\x80\x80\x80\x00\x01\x87\x80\x80\x80\x00\x00\x41\x00\x11\x00\x00\x0b");
let $31 = instance("\x00\x61\x73\x6d\x01\x00\x00\x00\x01\x85\x80\x80\x80\x00\x01\x60\x00\x01\x7f\x02\x8e\x80\x80\x80\x00\x01\x07\x6d\x6f\x64\x75\x6c\x65\x34\x01\x66\x03\x70\x00\x03\x82\x80\x80\x80\x00\x01\x00\x04\x84\x80\x80\x80\x00\x01\x70\x00\x0a\x07\x96\x80\x80\x80\x00\x01\x12\x63\x61\x6c\x6c\x5f\x69\x6d\x70\x6f\x72\x74\x65\x64\x5f\x65\x6c\x65\x6d\x00\x00\x09\x89\x80\x80\x80\x00\x01\x04\x41\x00\x0b\x01\x23\x00\x0b\x0a\x8d\x80\x80\x80\x00\x01\x87\x80\x80\x80\x00\x00\x41\x00\x11\x00\x00\x0b");

// FIXME: https://bugs.webkit.org/show_bug.cgi?id=260542
// elem.wast:701
//assert_return(() => call($31, "call_imported_elem", []), 42);
assert_return(() => call($31, "call_imported_elem", []), 42);
31 changes: 21 additions & 10 deletions Source/JavaScriptCore/wasm/WasmInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,18 +276,29 @@ void Instance::initElementSegment(uint32_t tableIndex, const Element& segment, u
continue;
}

if (initType == Element::InitializationType::FromGlobal) {
jsTable->set(dstIndex, JSValue::decode(loadI64Global(initialBitsOrIndex)));
continue;
JSValue initValue;
if (initType == Element::InitializationType::FromGlobal)
initValue = JSValue::decode(loadI64Global(initialBitsOrIndex));
else {
ASSERT(initType == Element::InitializationType::FromExtendedExpression);
uint64_t result;
bool success = evaluateConstantExpression(initialBitsOrIndex, segment.elementType, result);
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=264454
// Currently this should never fail, as the parse phase already validated it.
RELEASE_ASSERT(success);
initValue = JSValue::decode(result);
}

ASSERT(initType == Element::InitializationType::FromExtendedExpression);
uint64_t result;
bool success = evaluateConstantExpression(initialBitsOrIndex, segment.elementType, result);
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=264454
// Currently this should never fail, as the parse phase already validated it.
RELEASE_ASSERT(success);
jsTable->set(dstIndex, JSValue::decode(result));
if (jsTable->table()->isExternrefTable())
jsTable->set(dstIndex, initValue);
else {
// Validation should guarantee that the table is for funcs, and the value is a func as well.
ASSERT(jsTable->table()->isFuncrefTable());
ASSERT(initValue.getObject());
WebAssemblyFunctionBase* func = jsDynamicCast<WebAssemblyFunctionBase*>(initValue.getObject());
ASSERT(func);
jsTable->set(dstIndex, func);
}
}
}

Expand Down

0 comments on commit f546b9f

Please sign in to comment.