Box JSException storage in a class to fit the direct typed-error convention#13
Draft
krodak wants to merge 1 commit into
Draft
Box JSException storage in a class to fit the direct typed-error convention#13krodak wants to merge 1 commit into
krodak wants to merge 1 commit into
Conversation
30a9c43 to
aa02b44
Compare
1744fd4 to
583c12b
Compare
aa02b44 to
982e9fb
Compare
583c12b to
da5ff9c
Compare
da5ff9c to
89de6cd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
An intermediate solution for the async closure reject path broken by swiftlang/swift#89320, until the IRGen fix in swiftlang/swift#89715 ships.
JSException(~36 bytes) exceeds the direct typed-error convention on wasm32, so anasync throws(JSException)closure value takes the indirect-error path that swiftlang/swift#89320 miscompiles: when the closure is captureless, the thrown error is corrupted across the async unwind and the rejectedPromisereceives garbage instead of theJSException. BoxingJSException's storage into a single class reference (~4 bytes) moves it into the direct convention, so the broken path is never taken.The function-export variant of the same bug was fixed separately at the codegen level in swiftwasm#760 (already on
main); this PR addresses the closure side, which codegen cannot reach (the closure value is constructed by user code).1. Boxing, conditional on
compiler(>=6.3).thrownValue,description, andstackmove into a private final class; the struct holds one stored reference. The public surface is unchanged (computed accessors, same initializers, equivalentEquatable); the cost is one heap allocation per thrown exception, on the error path only. The boxed layout crashes the Swift 6.1 wasm IRGen (Invalid InsertValueInst operands), so older compilers keep the original stored properties and today's behavior. 6.2 is unverified and conservatively kept on the legacy layout; the threshold can be lowered after verification.2. Reject tests gated by capability, not blanket. A
@JSprobe (asyncThrowsClosureRejectSupported) reports whether the boxed layout is active; the Swift-to-JSasync throws(JSException)closure reject assertions run whenever it is. On Swift 6.3 and later they run and pass; on 6.1 they are skipped (status quo there). The blanketASYNC_THROWS_CLOSURE_REJECT_BLOCKEDgate from the base PR is removed.3. Base-PR mitigations removed. With boxing active on current toolchains, the build-time warning and most doc callouts from the base PR are dropped; the support table notes the reject path requires Swift 6.3 or later.
4.
JSClosure.asyncreject path. The same bug shape existed inJSClosure.asyncand was untested; this PR adds the missing reject-path test (gated oncompiler(>=6.3)), which passes with boxing.Notes