[Wasm-GC] Update element segments to account for typed funcrefs and GC types #21647

takikawa · 2023-12-11T20:49:00Z

`7e71e72`

[Wasm-GC] Update element segments to account for typed funcrefs and GC types
https://bugs.webkit.org/show_bug.cgi?id=251874

Reviewed by Justin Michaud.

Fix element segment parsing to allow more kinds of ref types (this is actually
a requirement of the reference types proposal, predating both typed funcref and
GC).

Also add support for general const expr initialization for elements.

For now, initialization steps for element segments are done each time on table
init or array.new_elem. This could be done earlier in module init instead (to
avoid duplciated work for shared elements), but that requires a larger change
to create a runtime representation of elements that can strongly hold
references.

Add missing spec tests and re-enable tests that now succeed as well. A small
validation fix was needed for the br_table test.

* JSTests/wasm/function-references-spec-tests/br_table.wast.js: Added.
* JSTests/wasm/function-references-spec-tests/ref.wast.js: Added.
* JSTests/wasm/function-references-spec-tests/ref_is_null.wast.js: Added.
* JSTests/wasm/function-references-spec-tests/table-sub.wast.js: Added.
* JSTests/wasm/function-references-spec-tests/unreached-valid.wast.js: Added.
* JSTests/wasm/gc-spec-tests/array.wast.js:
* JSTests/wasm/gc-spec-tests/binary-gc.wast.js: Added.
* JSTests/wasm/gc-spec-tests/ref_eq.wast.js: Added.
* JSTests/wasm/gc-spec-tests/ref_test.wast.js: Added.
* JSTests/wasm/gc/array_new_elem.js:
(testTypeMismatch):
(testAllElementSegmentKinds):
(testNullFunctionIndex):
* JSTests/wasm/gc/const-exprs.js:
(async testElementConstExprs):
* JSTests/wasm/gc/wast-wrapper.js:
* JSTests/wasm/references/element_active_mod.js:
(refNullExternInElemsSection):
* JSTests/wasm/v8/regress/regress-1046472.js:
* Source/JavaScriptCore/wasm/WasmConstExprGenerator.cpp:
(JSC::Wasm::evaluateExtendedConstExpr):
* Source/JavaScriptCore/wasm/WasmConstExprGenerator.h:
* Source/JavaScriptCore/wasm/WasmEntryPlan.cpp:
(JSC::Wasm::EntryPlan::prepare):
* Source/JavaScriptCore/wasm/WasmFormat.h:
(JSC::Wasm::Element::Element):
(JSC::Wasm::Element::length const):
(JSC::Wasm::Element::isNullFuncIndex): Deleted.
* Source/JavaScriptCore/wasm/WasmFunctionParser.h:
(JSC::Wasm::FunctionParser<Context>::addReferencedFunctions):
(JSC::Wasm::FunctionParser<Context>::parseExpression):
* Source/JavaScriptCore/wasm/WasmInstance.cpp:
(JSC::Wasm::Instance::initElementSegment):
(JSC::Wasm::Instance::copyElementSegment):
(JSC::Wasm::Instance::evaluateConstantExpression):
* Source/JavaScriptCore/wasm/WasmInstance.h:
* Source/JavaScriptCore/wasm/WasmSectionParser.cpp:
(JSC::Wasm::SectionParser::parseTableHelper):
(JSC::Wasm::SectionParser::parseElement):
(JSC::Wasm::SectionParser::validateElementTableIdx):
(JSC::Wasm::SectionParser::parseElementSegmentVectorOfExpressions):
(JSC::Wasm::SectionParser::parseElementSegmentVectorOfIndexes):
* Source/JavaScriptCore/wasm/WasmSectionParser.h:
* Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:
(JSC::WebAssemblyModuleRecord::evaluateConstantExpression):
(JSC::WebAssemblyModuleRecord::evaluate):

Canonical link: https://commits.webkit.org/271952@main

a032f5e

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
~~🛠 🧪 jsc~~	✅ 🧪 api-ios	✅ 🧪 mac-wk2	~~🧪 gtk-wk2~~
~~🛠 🧪 jsc-arm64~~	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-12-11T20:49:46Z

EWS run on previous version of this PR (hash cefa447)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 wincairo
	✅ 🛠 ios-sim	⏳ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	⏳ 🧪 api-mac	⏳ 🧪 api-wpe
	⏳ 🧪 ios-wk2-wpt	⏳ 🧪 mac-wk1	⏳ 🛠 gtk
⏳ 🛠 🧪 jsc	~~🧪 api-ios~~	⏳ 🧪 mac-wk2	⏳ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc-arm64	⏳ 🛠 tv	⏳ 🧪 mac-AS-debug-wk2	⏳ 🧪 api-gtk
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2-stress	✅ 🛠 jsc-armv7
	⏳ 🛠 watch		~~🧪 jsc-armv7-tests~~
	⏳ 🛠 watch-sim

takikawa · 2023-12-11T20:52:08Z

This will also close https://bugs.webkit.org/show_bug.cgi?id=260542

webkit-early-warning-system · 2023-12-11T20:56:58Z

EWS run on previous version of this PR (hash 8570de3)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 gtk
⏳ 🛠 🧪 jsc	✅ 🧪 api-ios	~~🧪 mac-wk2~~	~~🧪 gtk-wk2~~
⏳ 🛠 🧪 jsc-arm64	⏳ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	⏳ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
	✅ 🛠 watch		❌ 🧪 jsc-armv7-tests
	⏳ 🛠 watch-sim

justinmichaud · 2023-12-11T21:38:09Z

Source/JavaScriptCore/wasm/WasmInstance.cpp

Can this be fixed?

Oh whoops, thanks for catching this. Currently it's not expected that this should ever fail, so I'll put in a RELEASE_ASSERT but once I fix https://bugs.webkit.org/show_bug.cgi?id=264454 it will change to throw an exception in rare cases (ditto for the other call of evaluateConstantExpression).

takikawa · 2023-12-11T22:07:07Z

BTW there is a test failure in armv7, which just looks to be the result of error message mismatch (test still fails correctly). I'll update the expected message when I push in a bit.

webkit-early-warning-system · 2023-12-11T22:17:39Z

EWS run on previous version of this PR (hash 818914a)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
❌ 🛠 🧪 merge	✅ 🛠 watch		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch-sim

justinmichaud

r=me

webkit-commit-queue · 2023-12-12T18:51:12Z

No reviewer information in commit message, blocking PR #21647

webkit-early-warning-system · 2023-12-12T19:55:30Z

EWS run on current version of this PR (hash a032f5e)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
⏳ 🧪 style	🛠 ios	🛠 mac	🛠 wpe	🛠 wincairo
	🛠 ios-sim	⏳ 🛠 mac-AS-debug	🧪 wpe-wk2
🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	🧪 api-wpe
	🧪 ios-wk2-wpt	🧪 mac-wk1	🛠 gtk
⏳ 🛠 🧪 jsc	🧪 api-ios	🧪 mac-wk2	🧪 gtk-wk2
⏳ 🛠 🧪 jsc-arm64	⏳ 🛠 tv	⏳ 🧪 mac-AS-debug-wk2	🧪 api-gtk
🧪 services	⏳ 🛠 tv-sim	🧪 mac-wk2-stress	🛠 jsc-armv7
	⏳ 🛠 watch		🧪 jsc-armv7-tests
	⏳ 🛠 watch-sim

webkit-early-warning-system · 2023-12-12T19:55:30Z

EWS run on current version of this PR (hash a032f5e)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
~~🛠 🧪 jsc~~	✅ 🧪 api-ios	✅ 🧪 mac-wk2	~~🧪 gtk-wk2~~
~~🛠 🧪 jsc-arm64~~	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch-sim

…C types https://bugs.webkit.org/show_bug.cgi?id=251874 Reviewed by Justin Michaud. Fix element segment parsing to allow more kinds of ref types (this is actually a requirement of the reference types proposal, predating both typed funcref and GC). Also add support for general const expr initialization for elements. For now, initialization steps for element segments are done each time on table init or array.new_elem. This could be done earlier in module init instead (to avoid duplciated work for shared elements), but that requires a larger change to create a runtime representation of elements that can strongly hold references. Add missing spec tests and re-enable tests that now succeed as well. A small validation fix was needed for the br_table test. * JSTests/wasm/function-references-spec-tests/br_table.wast.js: Added. * JSTests/wasm/function-references-spec-tests/ref.wast.js: Added. * JSTests/wasm/function-references-spec-tests/ref_is_null.wast.js: Added. * JSTests/wasm/function-references-spec-tests/table-sub.wast.js: Added. * JSTests/wasm/function-references-spec-tests/unreached-valid.wast.js: Added. * JSTests/wasm/gc-spec-tests/array.wast.js: * JSTests/wasm/gc-spec-tests/binary-gc.wast.js: Added. * JSTests/wasm/gc-spec-tests/ref_eq.wast.js: Added. * JSTests/wasm/gc-spec-tests/ref_test.wast.js: Added. * JSTests/wasm/gc/array_new_elem.js: (testTypeMismatch): (testAllElementSegmentKinds): (testNullFunctionIndex): * JSTests/wasm/gc/const-exprs.js: (async testElementConstExprs): * JSTests/wasm/gc/wast-wrapper.js: * JSTests/wasm/references/element_active_mod.js: (refNullExternInElemsSection): * JSTests/wasm/v8/regress/regress-1046472.js: * Source/JavaScriptCore/wasm/WasmConstExprGenerator.cpp: (JSC::Wasm::evaluateExtendedConstExpr): * Source/JavaScriptCore/wasm/WasmConstExprGenerator.h: * Source/JavaScriptCore/wasm/WasmEntryPlan.cpp: (JSC::Wasm::EntryPlan::prepare): * Source/JavaScriptCore/wasm/WasmFormat.h: (JSC::Wasm::Element::Element): (JSC::Wasm::Element::length const): (JSC::Wasm::Element::isNullFuncIndex): Deleted. * Source/JavaScriptCore/wasm/WasmFunctionParser.h: (JSC::Wasm::FunctionParser<Context>::addReferencedFunctions): (JSC::Wasm::FunctionParser<Context>::parseExpression): * Source/JavaScriptCore/wasm/WasmInstance.cpp: (JSC::Wasm::Instance::initElementSegment): (JSC::Wasm::Instance::copyElementSegment): (JSC::Wasm::Instance::evaluateConstantExpression): * Source/JavaScriptCore/wasm/WasmInstance.h: * Source/JavaScriptCore/wasm/WasmSectionParser.cpp: (JSC::Wasm::SectionParser::parseTableHelper): (JSC::Wasm::SectionParser::parseElement): (JSC::Wasm::SectionParser::validateElementTableIdx): (JSC::Wasm::SectionParser::parseElementSegmentVectorOfExpressions): (JSC::Wasm::SectionParser::parseElementSegmentVectorOfIndexes): * Source/JavaScriptCore/wasm/WasmSectionParser.h: * Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp: (JSC::WebAssemblyModuleRecord::evaluateConstantExpression): (JSC::WebAssemblyModuleRecord::evaluate): Canonical link: https://commits.webkit.org/271952@main

webkit-commit-queue · 2023-12-12T22:16:34Z

Committed 271952@main (7e71e72): https://commits.webkit.org/271952@main

Reviewed commits have been landed. Closing PR #21647 and removing active labels.

takikawa self-assigned this Dec 11, 2023

takikawa added the WebAssembly For bugs in JavaScript WebAssembly label Dec 11, 2023

takikawa requested a review from a team as a code owner December 11, 2023 20:49

takikawa force-pushed the eng/Wasm-GC-Update-element-segments-to-account-for-typed-funcrefs-and-GC-types branch from cefa447 to 8570de3 Compare December 11, 2023 20:56

justinmichaud reviewed Dec 11, 2023

View reviewed changes

takikawa force-pushed the eng/Wasm-GC-Update-element-segments-to-account-for-typed-funcrefs-and-GC-types branch from 8570de3 to 818914a Compare December 11, 2023 22:17

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 12, 2023

justinmichaud approved these changes Dec 12, 2023

View reviewed changes

takikawa added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Dec 12, 2023

webkit-commit-queue added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Dec 12, 2023

takikawa removed the merging-blocked Applied to prevent a change from being merged label Dec 12, 2023

takikawa force-pushed the eng/Wasm-GC-Update-element-segments-to-account-for-typed-funcrefs-and-GC-types branch from 818914a to a032f5e Compare December 12, 2023 19:55

takikawa added the merge-queue Applied to send a pull request to merge-queue label Dec 12, 2023

webkit-commit-queue force-pushed the eng/Wasm-GC-Update-element-segments-to-account-for-typed-funcrefs-and-GC-types branch from a032f5e to 7e71e72 Compare December 12, 2023 22:16

webkit-commit-queue merged commit 7e71e72 into WebKit:main Dec 12, 2023

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 12, 2023

[Wasm-GC] Update element segments to account for typed funcrefs and GC types #21647

[Wasm-GC] Update element segments to account for typed funcrefs and GC types #21647

Uh oh!

Conversation

takikawa commented Dec 11, 2023 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

7e71e72

Uh oh!

webkit-early-warning-system commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takikawa commented Dec 11, 2023

Uh oh!

webkit-early-warning-system commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmichaud Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

takikawa Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

takikawa commented Dec 11, 2023

Uh oh!

webkit-early-warning-system commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmichaud left a comment

Choose a reason for hiding this comment

Uh oh!

webkit-commit-queue commented Dec 12, 2023

Uh oh!

webkit-early-warning-system commented Dec 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Dec 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-commit-queue commented Dec 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

takikawa commented Dec 11, 2023 •

edited by webkit-early-warning-system

Loading

`7e71e72`

webkit-early-warning-system commented Dec 11, 2023 •

edited

Loading

webkit-early-warning-system commented Dec 11, 2023 •

edited

Loading

webkit-early-warning-system commented Dec 11, 2023 •

edited

Loading

webkit-early-warning-system commented Dec 12, 2023 •

edited

Loading

webkit-early-warning-system commented Dec 12, 2023 •

edited

Loading