Validate runtime limits to 32 bits, unsigned pointer arrays (part of #275)

packages/quickjs-emscripten-core/src/context.ts

@@ -12,7 +12,6 @@ import type {
   EitherFFI,
   UInt32Pointer,
   JSValuePointerPointerPointer,
-  JSVoidPointer,
   HostRefId,
 } from "@jitl/quickjs-ffi-types"
 import type { JSPromiseState } from "./deferred-promise"
@@ -127,9 +126,11 @@ class ContextMemory extends ModuleMemory implements Disposable {
   }
 
   consumeJSCharPointer(ptr: JSBorrowedCharPointer): string {
-    const str = this.module.UTF8ToString(ptr)
-    this.ffi.QTS_FreeCString(this.ctx.value, ptr)
-    return str
+    try {
+      return this.module.UTF8ToString(ptr)
+    } finally {
+      this.ffi.QTS_FreeCString(this.ctx.value, ptr)
+    }
   }
 
   heapValueHandle(ptr: JSValuePointer, extraDispose?: () => void): JSValue {
@@ -448,6 +449,13 @@ export class QuickJSContext
     const array = new Uint8Array(buffer)
     const handle = this.memory.newHeapBufferPointer(array)
     const ptr = this.ffi.QTS_NewArrayBuffer(this.ctx.value, handle.value.pointer, array.length)
+    const errorPtr = this.ffi.QTS_ResolveException(this.ctx.value, ptr)
+    if (errorPtr) {
+      // QuickJS only takes ownership of the host buffer when it succeeds in creating the ArrayBuffer.
+      handle.dispose()
+      this.ffi.QTS_FreeValuePointer(this.ctx.value, ptr)
+      return this.unwrapResult(this.fail(this.memory.heapValueHandle(errorPtr)))
+    }
     return this.memory.heapValueHandle(ptr)
   }
 
@@ -487,9 +495,9 @@ export class QuickJSContext
         mutablePointerArray.value.ptr,
       )
       const promiseHandle = this.memory.heapValueHandle(promisePtr)
-      const [resolveHandle, rejectHandle] = Array.from(mutablePointerArray.value.typedArray).map(
-        (jsvaluePtr) => this.memory.heapValueHandle(jsvaluePtr as any),
-      )
+      const [resolveHandle, rejectHandle] = Array.from(
+        mutablePointerArray.value.typedArray.value,
+      ).map((jsvaluePtr) => this.memory.heapValueHandle(jsvaluePtr as any))
       return new QuickJSDeferredPromise({
         context: this,
         promiseHandle,
@@ -806,7 +814,9 @@ export class QuickJSContext
   }
 
   /**
-   * Coverts `handle` to a JavaScript ArrayBuffer
+   * Coverts `handle` to a JavaScript ArrayBuffer.
+   *
+   * The returned bytes are a copy that stays valid however much the WASM heap grows afterwards.
    */
   getArrayBuffer(handle: QuickJSHandle): Lifetime<Uint8Array> {
     this.runtime.assertOwned(handle)
@@ -815,9 +825,12 @@ export class QuickJSContext
     if (!ptr) {
       throw new Error("Couldn't allocate memory to get ArrayBuffer")
     }
-    return new Lifetime(this.module.HEAPU8.subarray(ptr, ptr + len), undefined, () =>
-      this.module._free(ptr),
-    )
+    try {
+      // A view into the heap would be detached by the next memory growth, so copy out.
+      return new Lifetime<Uint8Array>(this.module.HEAPU8.slice(ptr, ptr + len))
+    } finally {
+      this.module._free(ptr)
+    }
   }
 
   /**
@@ -994,7 +1007,7 @@ export class QuickJSContext
     if (status < 0) {
       return undefined
     }
-    return this.uint32Out.value.typedArray[0]
+    return this.uint32Out.value.typedArray.value[0]
   }
 
   /**
@@ -1052,13 +1065,14 @@ export class QuickJSContext
       if (errorPtr) {
         return this.fail(this.memory.heapValueHandle(errorPtr))
       }
-      const len = this.uint32Out.value.typedArray[0]
-      const ptr = outPtr.value.typedArray[0]
-      const pointerArray = new Uint32Array(this.module.HEAP8.buffer, ptr, len)
+      const len = this.uint32Out.value.typedArray.value[0]
+      const ptr = outPtr.value.typedArray.value[0]
+      const pointerArray = new Uint32Array(this.module.HEAPU8.buffer, ptr, len)
       const handles = Array.from(pointerArray).map((ptr) =>
         this.memory.heapValueHandle(ptr as JSValuePointer),
       )
-      this.ffi.QTS_FreeVoidPointer(this.ctx.value, ptr as JSVoidPointer)
+      // The C side allocated this array with plain malloc, not js_malloc.
+      this.module._free(ptr)
       return this.success(createDisposableArray(handles))
     })
   }

packages/quickjs-emscripten-core/src/host-ref.test.ts

@@ -110,6 +110,27 @@ describe("HostRefMap", () => {
   })
 
   describe("ID recycling (freelist)", () => {
+    it("does not hand out an id twice if it is deleted twice while its group is non-empty", () => {
+      const map = new HostRefMap()
+      const id1 = map.put({ a: 1 })
+      map.put({ b: 2 }) // keeps id1's group alive
+      map.delete(id1)
+      assert.throws(() => map.delete(id1), QuickJSHostRefInvalid)
+      const reused = map.put({ c: 3 })
+      const fresh = map.put({ d: 4 })
+      assert.strictEqual(reused, id1)
+      assert.notStrictEqual(fresh, id1)
+    })
+
+    it("keeps FIFO order across compaction of the freelist", () => {
+      const map = new HostRefMap()
+      const ids = Array.from({ length: 5000 }, (_, i) => map.put({ i }))
+      ids.forEach((id) => map.delete(id))
+      const reused = ids.map((_, i) => map.put({ i }))
+      assert.deepStrictEqual(reused, ids)
+      assert.ok(!ids.includes(map.put({ fresh: true })), "falls back to new ids once drained")
+    })
+
     it("reuses deleted IDs", () => {
       const map = new HostRefMap()
       const id1 = map.put({ first: true })

packages/quickjs-emscripten-core/src/host-ref.ts

@@ -17,7 +17,9 @@ function getGroupId(id: HostRefId): number {
  */
 export class HostRefMap {
   private nextId = INT32_MIN
+  /** FIFO queue of ids to reuse: `freelist[freelistHead..]`. */
   private freelist: HostRefId[] = []
+  private freelistHead = 0
   private groups = new Map<number, Map<HostRefId, object>>()
 
   put(value: object): HostRefId {
@@ -62,7 +64,10 @@ export class HostRefMap {
       throw new QuickJSHostRefInvalid(`host reference id ${id} is not defined`)
     }
 
-    group.delete(id)
+    if (!group.delete(id)) {
+      // Recycling an already-deleted id again would hand it out twice.
+      throw new QuickJSHostRefInvalid(`host reference id ${id} is not defined`)
+    }
     if (group.size === 0) {
       this.groups.delete(groupId)
     }
@@ -71,8 +76,14 @@ export class HostRefMap {
   }
 
   private allocateId(): HostRefId {
-    if (this.freelist.length > 0) {
-      return this.freelist.shift()!
+    if (this.freelistHead < this.freelist.length) {
+      const id = this.freelist[this.freelistHead++]
+      // Drop consumed entries occasionally; Array#shift per allocation is O(n).
+      if (this.freelistHead >= 1024 && this.freelistHead * 2 >= this.freelist.length) {
+        this.freelist = this.freelist.slice(this.freelistHead)
+        this.freelistHead = 0
+      }
+      return id
     }
 
     if (this.nextId === INVALID_HOST_REF_ID) {

packages/quickjs-emscripten-core/src/memory.ts

@@ -29,9 +29,39 @@ export interface TypedArrayConstructor<T> {
   BYTES_PER_ELEMENT: number
 }
 
+/**
+ * A TypedArray view into WASM memory that automatically refreshes when memory grows.
+ *
+ * When Emscripten's WASM memory grows (due to -sALLOW_MEMORY_GROWTH), all existing
+ * TypedArray views become detached because the underlying ArrayBuffer is replaced.
+ * This class lazily recreates the view when the buffer changes.
+ *
+ * @private
+ */
+export class RefreshableTypedArray<T extends TypedArray> {
+  private cachedArray: T | undefined
+  private lastBuffer: ArrayBufferLike | undefined
+
+  constructor(
+    private readonly module: EitherModule,
+    private readonly kind: TypedArrayConstructor<T>,
+    private readonly ptr: number,
+    private readonly length: number,
+  ) {}
+
+  get value(): T {
+    const currentBuffer = this.module.HEAPU8.buffer
+    if (this.cachedArray === undefined || this.lastBuffer !== currentBuffer) {
+      this.cachedArray = new this.kind(currentBuffer, this.ptr, this.length)
+      this.lastBuffer = currentBuffer
+    }
+    return this.cachedArray
+  }
+}
+
 /** @private */
 export type HeapTypedArray<JS extends TypedArray, C extends number> = Lifetime<{
-  typedArray: JS
+  typedArray: RefreshableTypedArray<JS>
   ptr: C
 }>
 
@@ -41,10 +71,19 @@ export type HeapTypedArray<JS extends TypedArray, C extends number> = Lifetime<{
 export class ModuleMemory {
   constructor(public module: EitherModule) {}
 
+  /** `_malloc` that throws instead of returning a null pointer we would then write through. */
+  private malloc<T extends number>(numBytes: number): T {
+    const ptr = this.module._malloc(numBytes)
+    if (!ptr && numBytes > 0) {
+      throw new Error(`Failed to allocate ${numBytes} bytes in the WASM heap (out of memory)`)
+    }
+    return ptr as T
+  }
+
   toPointerArray(handleArray: QuickJSHandle[]): Lifetime<JSValueConstPointerPointer> {
-    const typedArray = new Int32Array(handleArray.map((handle) => handle.value))
+    const typedArray = new Uint32Array(handleArray.map((handle) => handle.value))
     const numBytes = typedArray.length * typedArray.BYTES_PER_ELEMENT
-    const ptr = this.module._malloc(numBytes) as JSValueConstPointerPointer
+    const ptr = this.malloc<JSValueConstPointerPointer>(numBytes)
     const heapBytes = new Uint8Array(this.module.HEAPU8.buffer, ptr, numBytes)
     heapBytes.set(new Uint8Array(typedArray.buffer))
     return new Lifetime(ptr, undefined, (ptr) => this.module._free(ptr))
@@ -54,41 +93,42 @@ export class ModuleMemory {
     kind: TypedArrayConstructor<JS>,
     length: number,
   ): HeapTypedArray<JS, C> {
-    const zeros = new kind(new Array(length).fill(0))
-    const numBytes = zeros.length * zeros.BYTES_PER_ELEMENT
-    const ptr = this.module._malloc(numBytes) as C
-    const typedArray = new kind(this.module.HEAPU8.buffer, ptr, length)
-    typedArray.set(zeros)
+    const numBytes = length * kind.BYTES_PER_ELEMENT
+    const ptr = this.malloc<C>(numBytes)
+    const typedArray = new RefreshableTypedArray(this.module, kind, ptr, length)
+    typedArray.value.fill(0)
     return new Lifetime({ typedArray, ptr }, undefined, (value) => this.module._free(value.ptr))
   }
 
-  // TODO: shouldn't this be Uint32 instead of Int32?
+  /** Pointers are unsigned: heap addresses of 2 GiB and above must not read back negative. */
   newMutablePointerArray<T extends number>(
     length: number,
-  ): Lifetime<{ typedArray: Int32Array<ArrayBuffer>; ptr: T }> {
-    return this.newTypedArray(Int32Array, length)
+  ): Lifetime<{ typedArray: RefreshableTypedArray<Uint32Array<ArrayBuffer>>; ptr: T }> {
+    return this.newTypedArray(Uint32Array, length)
   }
 
   newHeapCharPointer(string: string): Lifetime<{ ptr: OwnedHeapCharPointer; strlen: number }> {
     const strlen = this.module.lengthBytesUTF8(string)
     const dataBytes = strlen + 1
-    const ptr: OwnedHeapCharPointer = this.module._malloc(dataBytes) as OwnedHeapCharPointer
+    const ptr = this.malloc<OwnedHeapCharPointer>(dataBytes)
     this.module.stringToUTF8(string, ptr, dataBytes)
     return new Lifetime({ ptr, strlen }, undefined, (value) => this.module._free(value.ptr))
   }
 
   newHeapBufferPointer(buffer: Uint8Array): Lifetime<HeapUint8Array> {
     const numBytes = buffer.byteLength
-    const ptr: JSVoidPointer = this.module._malloc(numBytes) as JSVoidPointer
+    const ptr = this.malloc<JSVoidPointer>(numBytes)
     this.module.HEAPU8.set(buffer, ptr)
     return new Lifetime({ pointer: ptr, numBytes }, undefined, (value) =>
       this.module._free(value.pointer),
     )
   }
 
   consumeHeapCharPointer(ptr: OwnedHeapCharPointer): string {
-    const str = this.module.UTF8ToString(ptr)
-    this.module._free(ptr)
-    return str
+    try {
+      return this.module.UTF8ToString(ptr)
+    } finally {
+      this.module._free(ptr)
+    }
   }
 }

packages/quickjs-emscripten-core/src/module.ts

@@ -433,6 +433,9 @@ export class QuickJSWASMModule {
    * Retrieve the WebAssembly memory used by this QuickJS module.
    * Use this access very carefully - you are responsible for safe interaction with the memory.
    *
+   * Do not call `memory.grow()` on the returned memory yourself: Emscripten's `HEAP*` views
+   * would not be refreshed and the module would stop working. Let the module grow its own memory.
+   *
    * To supply a custom, pre-initialized memory to QuickJS, create a new variant
    * and provide the {@link CustomizeVariantOptions#wasmMemory} option.
    *

packages/quickjs-emscripten-core/src/runtime.ts

@@ -43,6 +43,8 @@ export type ExecutePendingJobsResult = DisposableResult<
   }
 >
 
+const UINT32_MAX = 0xffffffff
+
 /**
  * A runtime represents a Javascript runtime corresponding to an object heap.
  * Several runtimes can exist at the same time but they cannot exchange objects.
@@ -251,7 +253,7 @@ export class QuickJSRuntime extends UsingDisposable implements Disposable {
       ctxPtrOut.value.ptr,
     )
 
-    const ctxPtr = ctxPtrOut.value.typedArray[0] as JSContextPointer
+    const ctxPtr = ctxPtrOut.value.typedArray.value[0] as JSContextPointer
     ctxPtrOut.dispose()
     if (ctxPtr === 0) {
       // No jobs executed.
@@ -285,6 +287,10 @@ export class QuickJSRuntime extends UsingDisposable implements Disposable {
     if (limitBytes < 0 && limitBytes !== -1) {
       throw new Error("Cannot set memory limit to negative number. To unset, pass -1")
     }
+    if (limitBytes !== -1 && !(limitBytes <= UINT32_MAX)) {
+      // NaN, Infinity and values above 2^32 - 1 would silently wrap in the 32-bit C `size_t`.
+      throw new RangeError(`Memory limit must be a number of bytes no greater than ${UINT32_MAX}`)
+    }
 
     this.ffi.QTS_RuntimeSetMemoryLimit(this.rt.value, limitBytes)
   }
@@ -317,7 +323,10 @@ export class QuickJSRuntime extends UsingDisposable implements Disposable {
    */
   setMaxStackSize(stackSize: number) {
     if (stackSize < 0) {
-      throw new Error("Cannot set memory limit to negative number. To unset, pass 0.")
+      throw new Error("Cannot set max stack size to negative number. To unset, pass 0.")
+    }
+    if (!(stackSize <= UINT32_MAX)) {
+      throw new RangeError(`Max stack size must be a number of bytes no greater than ${UINT32_MAX}`)
     }
 
     this.ffi.QTS_RuntimeSetMaxStackSize(this.rt.value, stackSize)