Skip to content

Conversation

@takikawa
Copy link
Contributor

@takikawa takikawa commented Nov 15, 2022

febd681

[Wasm-GC] Fix refcounts for compound type definitions
https://bugs.webkit.org/show_bug.cgi?id=247874

Reviewed by Yusuke Suzuki.

Compound type definitions that use type indices to refer to other types
need to ref/deref the referred type definitions in order to keep them
live (since the module's type signature list will not necessarily hold
these types). Type definitions that only hold value types such as
functions or arrays are fine as-is.

* JSTests/wasm/gc/bug247874.js: Added.
(i.instantiate.module.type.struct.type.sub.0.struct.field.i32.global.import.string_appeared_here.string_appeared_here):
(i.instantiate.module.rec.type.struct.type.func.global.import.string_appeared_here.string_appeared_here):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.cpp:
(JSC::Wasm::Subtype::cleanup):
(JSC::Wasm::Projection::cleanup):
(JSC::Wasm::RecursionGroup::cleanup):
(JSC::Wasm::TypeDefinition::cleanup):
(JSC::Wasm::RecursionGroupParameterTypes::translate):
(JSC::Wasm::ProjectionParameterTypes::translate):
(JSC::Wasm::SubtypeParameterTypes::translate):
(JSC::Wasm::TypeInformation::tryCleanup):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.h:

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

13c577c

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

@takikawa takikawa self-assigned this Nov 15, 2022
@takikawa takikawa added the WebAssembly For bugs in JavaScript WebAssembly label Nov 15, 2022
@takikawa takikawa marked this pull request as ready for review November 16, 2022 02:26
@takikawa takikawa requested a review from a team as a code owner November 16, 2022 02:26
Copy link
Member

@Constellation Constellation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

@Constellation Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 17, 2022
https://bugs.webkit.org/show_bug.cgi?id=247874

Reviewed by Yusuke Suzuki.

Compound type definitions that use type indices to refer to other types
need to ref/deref the referred type definitions in order to keep them
live (since the module's type signature list will not necessarily hold
these types). Type definitions that only hold value types such as
functions or arrays are fine as-is.

* JSTests/wasm/gc/bug247874.js: Added.
(i.instantiate.module.type.struct.type.sub.0.struct.field.i32.global.import.string_appeared_here.string_appeared_here):
(i.instantiate.module.rec.type.struct.type.func.global.import.string_appeared_here.string_appeared_here):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.cpp:
(JSC::Wasm::Subtype::cleanup):
(JSC::Wasm::Projection::cleanup):
(JSC::Wasm::RecursionGroup::cleanup):
(JSC::Wasm::TypeDefinition::cleanup):
(JSC::Wasm::RecursionGroupParameterTypes::translate):
(JSC::Wasm::ProjectionParameterTypes::translate):
(JSC::Wasm::SubtypeParameterTypes::translate):
(JSC::Wasm::TypeInformation::tryCleanup):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.h:

Canonical link: https://commits.webkit.org/256800@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Wasm-GC-Fix-refcounts-for-compound-type-definitions branch from 13c577c to febd681 Compare November 17, 2022 20:29
@webkit-commit-queue
Copy link
Collaborator

Committed 256800@main (febd681): https://commits.webkit.org/256800@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit febd681 into WebKit:main Nov 17, 2022
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebAssembly For bugs in JavaScript WebAssembly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants