Fix over-aligned key path indices (wasm LocalStorage<UInt64> trap) + wasm runtime smoke - #89
Merged
Merged
Conversation
A Swift compiler bug (present in 6.3.3 and 6.4.0) misplaces a key path's subscript index when the path is formed in generic code, the index's layout depends on a generic parameter, and the index is aligned beyond a pointer. The caller packs it at pointer-size offset; the generated argument-init thunk reads it at the offset rounded up to its alignment. SwiftModel formed such paths for storage ([_metadata: ContextStorage<V>]), preferences ([_preference: PreferenceStorage<V>]) and container elements ([cursor: ContainerCursor<ID, ...>]). On wasm32 that made LocalStorage<UInt64>/Int64/Double read garbage or trap in keypath_destroy under Context.willAccessStorage; on 64-bit, 16-byte-aligned types such as SIMD vectors did the same. Index the storage and preference stub paths by the non-generic storage key, box ContainerCursor's id, and drop the unused generic Model [_metadata:]/[_preference:] subscripts. OverAlignedKeyPathIndexTests reproduces all four paths natively with SIMD2<Double> (each crashed before). scripts/wasm-smoke builds a small executable for wasm32-unknown-wasip1 and runs it under wasmtime; it trapped with the downstream stack before the fix and passes after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test suite can't run on WASI yet, but a plain executable can, so the WASM job now installs wasmtime and runs scripts/wasm-smoke after the compile steps. Job timeout 10 -> 15 min for the extra build (the job took ~4 min; the smoke adds one more SwiftModel build). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The swift:6.3.0 container has no xz, so the action couldn't unpack the wasmtime .tar.xz. Also pin wasmtime to 49.0.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Problem
Reading a
LocalStorage<UInt64>on wasm32 trapped with "null function" inkeypath_destroy, called fromContext.willAccessStorage. imagien hit this in #2428 and worked around it in #2471 by switching toLocalStorage<Int>.The cause is a Swift compiler bug, reproduced on 6.3.3 and 6.4.0 with about 10 lines of plain Swift and no SwiftModel. It hits a key path that:
The call site packs the index into the key path's argument buffer at pointer-size offset, without aligning it. The generated
keypath_arg_initreads it back atroundUp(pointerSize, alignment). So the index is read as garbage: sometimes a silently wrong value, sometimes a trap later on destroy.UInt64,Int64,Double.Intworks, which is why the imagien workaround helped.SIMD4<Float>. This happens in native macOS builds too.SwiftModel formed three such key paths:
[_metadata: ContextStorage<V>]for local and environment storage,[_preference: PreferenceStorage<V>]for preferences,[cursor: ContainerCursor<ID, …>]for collection elements. A[Row]withUInt64ids crashed on wasm32 the same way.Fix
_ModelStateType[_metadata:]/[_preference:]stub subscripts are now indexed by the storage's non-genericAnyHashableSendablekey. They are now internal, andVappears only in the result type.ContainerCursorstores its id in a class box, so its layout no longer depends onID.Model[_metadata:]/Model[_preference:]subscripts are removed.Validation
OverAlignedKeyPathIndexTests(native) covers local, environment, preference and container-element paths withSIMD2<Double>, which is 16-byte aligned. That makes the bug reproduce on 64-bit hosts, so regular macOS and Linux CI catches a regression. All four tests crashed with SIGSEGV before the fix.scripts/wasm-smoke(new) buildsTests/WASMSmoke, a small separate package, forwasm32-unknown-wasip1and runs it under wasmtime. It checksUInt64/Int64/Doublestorage, environment, preference,[Row]withUInt64ids, and memoize. Againstorigin/mainit traps with exactly the downstream stack (keypath_destroy←Context.willAccessStorage). With this branch, all checks pass.bytecodealliance/actions/wasmtime/setup@v1and runsscripts/wasm-smokeafter the compile steps, so WASM code is executed in CI, not just compiled. The job timeout goes from 10 to 15 minutes; the job took about 4 minutes, and the smoke adds one more SwiftModel build. The suite itself still can't run on WASI, becauseGlobalTickScheduleris GCD-backed.scripts/test(parallel) andscripts/test --no-parallelare both green locally: 746 + 124 + 7 + 29 tests.swift build --build-testspasses with the 6.4.0 SDK (SWIFTPM_TARGET_WASI=1 OMIT_DYNAMIC_TEST_SUPPORT=1). The local 6.3.x toolchains can't compile this repo's manifest against the macOS 27 SDK, so the 6.3 lane is left to CI's Linux container.🤖 Generated with Claude Code