JSTypedArray: annotate swjs_create_typed_array for bounds-safety#749
Draft
airspeedswift wants to merge 1 commit into
Draft
JSTypedArray: annotate swjs_create_typed_array for bounds-safety#749airspeedswift wants to merge 1 commit into
airspeedswift wants to merge 1 commit into
Conversation
30bb265 to
34ab30a
Compare
Author
|
removed the guard checking for clang headers, that's probably overkill if you assume the minimum tools version, but can put it back if this repo's practice prefers it. |
…ributes On recent clang/Swift toolchains, bounds-safety annotations on `void *` parameters cause the Swift importer to synthesize an `UnsafeRawBufferPointer`-taking overload alongside the original. Adding these annotations to `swjs_create_typed_array` gives us that overload for free, and it lays the foundation for broader `Span` adoption in JavaScriptKit in follow-up PRs. Because `elements_ptr` is `void *`, the byte count is `length * sizeof(element)`. This PR adds a trailing `element_size` parameter and annotates `elements_ptr` as `__sized_by_or_null(length * element_size) __noescape`. Both annotations are correct given the JS implementation: it constructs a typed array view over WASM linear memory at `elementsPtr` with `length` elements and immediately calls `.slice()` to copy before returning. The view constructor reads exactly `length * constructor.BYTES_PER_ELEMENT` bytes, which equals `length * element_size` since Swift passes `MemoryLayout<Element>.stride` — matching `BYTES_PER_ELEMENT` for every `TypedArrayElement` type. And because only the `.slice()` copy is retained and returned, `elementsPtr` never escapes the call. `__sized_by_or_null` rather than `__sized_by` preserves the existing nullable contract: Swift's `Array.baseAddress` is `nil` for empty arrays, and the JS side already handles `length == 0` without dereferencing the pointer. The macros are defined by clang's `<ptrcheck.h>` and `<lifetimebound.h>` resource headers, both included explicitly: Apple's `<sys/cdefs.h>` pulls in `<ptrcheck.h>` transitively, but Linux's does not, so direct inclusion is required for cross-platform consistency. The only coordinated change is in `JSTypedArray.swift`'s `createTypedArray`, which passes `Int32(MemoryLayout<Element>.stride)` as the new argument; the JS dispatch entry is unchanged — extra WASM arguments are silently dropped.
34ab30a to
01d7a52
Compare
Author
|
hmm guess I was wrong about ptrcheck being quite so available... |
Author
|
Moving this to draft pending decision about how best to handle 6.1 toolchain support. Also needs some tests for calling the safe version. |
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.
On recent clang/Swift toolchains, bounds-safety annotations on
void *parameters cause the Swift importer to synthesize anUnsafeRawBufferPointer-taking overload alongside the original. Adding these annotations toswjs_create_typed_arraygives us that overload for free, and it lays the foundation for broaderSpanadoption in JavaScriptKit in follow-up PRs.Because
elements_ptrisvoid *, the byte count islength * sizeof(element). This PR adds a trailingelement_sizeparameter and annotateselements_ptras__sized_by_or_null(length * element_size) __noescape. Both annotations are correct given the JS implementation: it constructs a typed array view over WASM linear memory atelementsPtrwithlengthelements and immediately calls.slice()to copy before returning. The view constructor reads exactlylength * constructor.BYTES_PER_ELEMENTbytes, which equalslength * element_sizesince Swift passesMemoryLayout<Element>.stride— matchingBYTES_PER_ELEMENTfor everyTypedArrayElementtype. And because only the.slice()copy is retained and returned,elementsPtrnever escapes the call.__sized_by_or_nullrather than__sized_bypreserves the existing nullable contract: Swift'sArray.baseAddressisnilfor empty arrays, and the JS side already handleslength == 0without dereferencing the pointer.The only coordinated change is in
JSTypedArray.swift'screateTypedArray, which passesInt32(MemoryLayout<Element>.stride)as the new argument; the JS dispatch entry is unchanged — extra WASM arguments are silently dropped.The alternative is to redefine length as a byte count and drop element_size, collapsing the bounds expression to
__sized_by_or_null(byte_length);– this is a narrower change that doesn't update the typescript, but you could argue the alternative is cleaner.