jsc: mark ArrayBuffer no-copy constructors unsafe

Merged
RO
5mo ago
+236-121

Fixes #31970

Problem

make_array_buffer_with_bytes_no_copy and make_typed_array_with_bytes_no_copy in src/jsc/array_buffer.rs were safe pub fns accepting an arbitrary raw pointer, length, deallocator fn pointer, and deallocator context. They forward those values to JSC, which adopts the memory as the backing store of a JS-visible ArrayBuffer/TypedArray: every JS read/write dereferences the pointer, and GC later invokes deallocator(ptr, ctx).

Safe Rust could therefore mint a JS object backed by a dangling pointer, and get an arbitrary fn-pointer/context pair invoked at GC time, without writing unsafe:

let backing = core::ptr::NonNull::<c_void>::dangling().as_ptr();
let _ = make_array_buffer_with_bytes_no_copy(global, backing, 1, None, ptr::null_mut());

The FFI imports were declared safe fn with a comment deferring the validity obligation to "the wrapper layer", but the wrappers were themselves safe and public, so the obligation was never discharged at an unsafe boundary.

Fix

  • Mark both free functions pub unsafe fn with a documented # Safety contract (ptr valid for reads and writes of len bytes until the deallocator runs; deallocator/ctx a pair sound to invoke exactly once on the JS thread at GC; ptr null only when len == 0).
  • Mark ArrayBuffer::to_js_with_context pub unsafe fn as well. It forwards a caller-supplied deallocator/ctx pair straight to the functions above (same class), and clippy's deny-level not_unsafe_ptr_arg_deref flags it once the callees are unsafe. to_js and to_js_unchecked take no raw arguments and stay safe; the soundness of the ArrayBuffer descriptor struct itself (from_bytes accepting arbitrary &mut [u8]) is tracked separately per the issue and is being fixed in #31174.
  • Drop the safe marker from the Bun__makeArrayBufferWithBytesNoCopy / Bun__makeTypedArrayWithBytesNoCopy externs so the obligation lives in the wrappers' contracts instead of being asserted away in the extern block.
  • Add unsafe {} blocks with per-site SAFETY justifications at every caller: ArrayBuffer::to_js_unchecked / to_js / to_js_with_context, MarkedArrayBuffer::to_js, Bun.mmapFile (BunObject.rs), the zlib/libdeflate sync paths (BunObject.rs), Blob Share/Transfer lifetimes (Blob.rs), bun:ffi toArrayBuffer (FFIObject.rs), and the image codec delivery path (Image.rs). All in-tree callers already passed valid pointers; this change is API contract only, no behavior change.

Test

No test file: the change is contract-only with zero behavior delta, and the enforcement is the compiler itself. Reverting any signature to safe trips unused_unsafe (denied workspace-wide) at every call site, and clippy's deny-level not_unsafe_ptr_arg_deref re-fires on to_js_with_context. A source-lint test under test/internal/ was originally included and removed at maintainer request.

Verification

  • cargo clippy -p bun_jsc -p bun_runtime clean (was failing on to_js_with_context mid-fix, which is what pulled it into scope)
  • bun run rust:check-all: 10 ok, 0 failed
  • bun bd test on suites covering every touched call site: test/js/bun/util/mmap.test.js (8 pass), test/regression/issue/18413-all-compressions.test.ts + 17793.test.ts (zlib paths), test/js/web/fetch/blob.test.ts + blob-cow.test.ts (27 pass), test/js/bun/ffi/ffi.test.js, test/js/bun/image/image.test.ts (93 pass)