diff --git a/package.json b/package.json index 89323b7..58692ad 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "scripts": { "build": "make app", "build:dev": "make dev", - "test": "node test.mjs && node test-contract.mjs", + "test": "node test.mjs && node test-contract.mjs && node test-bugs.mjs", "test:examples": "node test-examples.mjs", "prepublishOnly": "npm run build && npm test && npm run test:examples", "serve": "python3 -m http.server 8080" diff --git a/src/main.c b/src/main.c index 7759773..e40f382 100644 --- a/src/main.c +++ b/src/main.c @@ -8,6 +8,7 @@ */ #include +#include #include #include #include @@ -368,6 +369,52 @@ EMSCRIPTEN_KEEPALIVE int64_t get_data_byte_size(ray_t* obj) { * Atom constructors * ============================================================================ */ +/* Index, length and symbol-id parameters that cross the JS boundary are + * declared `ray_jsidx_t` (double), never int64_t. + * + * The build links with `-s WASM_BIGINT=0`, which legalizes every i64 parameter + * into two i32 words (lo, hi). The SDK's cwrap arg lists declare one 'number' + * per C parameter, so an i64 parameter that is not last shifts every parameter + * after it: `vec_set_idx(obj, idx, val)` received `val` as idx's high word and + * 0 as `val`, which tripped the `if (!obj || !val)` guard and returned + * RAY_NULL_OBJ. A trailing i64 appeared to work only because the missing high + * word defaults to 0 -- it still truncated at 2^32. + * + * A double is passed as one f64, needs no legalization, matches the JS Number + * domain exactly, and represents every integer up to 2^53 -- far beyond any + * index reachable in a 4 GiB wasm32 heap. Call sites in JS stay unchanged. */ +typedef double ray_jsidx_t; + +/* Values arriving as f64 are untrusted: JS (or a raw `_fill_i32_vec(p, d, -1)` + * probe) can hand us NaN, Infinity, a fraction or a negative. Casting those + * straight to int64_t is undefined for NaN/Inf and, for a negative, yields a + * negative count whose byte size wraps to nearly UINT32_MAX once memcpy's + * 32-bit size_t truncates it on wasm32. Narrow only after proving the value + * is finite, integral, non-negative and inside the exact-integer range of a + * double. */ +#define RAY_JSIDX_MAX 9007199254740991.0 /* 2^53 - 1 */ + +static bool jsidx_to_i64(ray_jsidx_t v, int64_t* out) { + if (!isfinite(v) || v < 0.0 || v > RAY_JSIDX_MAX || v != floor(v)) return false; + *out = (int64_t)v; + return true; +} + +/* String-length arguments cross the boundary as f64 as well. Every JS call + * site marshals its argument through cwrap's 'string', which hands us a + * NUL-terminated copy, so we can do better than trusting the number: validate + * it, then clamp to the buffer's actual extent. A bogus length then neither + * wraps the size_t cast nor reads past the copy. */ +static bool jsidx_to_strlen(const char* s, ray_jsidx_t len_f, size_t* out) { + int64_t n; + if (!s || !jsidx_to_i64(len_f, &n) || (uint64_t)n > (uint64_t)SIZE_MAX) return false; + /* Open-coded strnlen: that is POSIX, and this TU builds with -std=c17. */ + size_t max = (size_t)n, i = 0; + while (i < max && s[i]) i++; + *out = i; + return true; +} + EMSCRIPTEN_KEEPALIVE ray_t* init_b8(bool val) { return ray_bool(val); } EMSCRIPTEN_KEEPALIVE ray_t* init_u8(uint8_t val) { return ray_u8(val); } EMSCRIPTEN_KEEPALIVE ray_t* init_i16(int16_t val) { return ray_i16(val); } @@ -379,12 +426,16 @@ EMSCRIPTEN_KEEPALIVE ray_t* init_date(int64_t days) { return ray_date(days EMSCRIPTEN_KEEPALIVE ray_t* init_time(int64_t ms) { return ray_time(ms); } EMSCRIPTEN_KEEPALIVE ray_t* init_timestamp(int64_t ns) { return ray_timestamp(ns); } -EMSCRIPTEN_KEEPALIVE ray_t* init_symbol_str(const char* s, int64_t len) { - return ray_sym(ray_sym_intern(s, (size_t)len)); +EMSCRIPTEN_KEEPALIVE ray_t* init_symbol_str(const char* s, ray_jsidx_t len) { + size_t n; + if (!jsidx_to_strlen(s, len, &n)) return ray_error("length", "init_symbol_str: invalid length"); + return ray_sym(ray_sym_intern(s, n)); } -EMSCRIPTEN_KEEPALIVE ray_t* init_string_str(const char* s, int64_t len) { - return ray_str(s, (size_t)len); +EMSCRIPTEN_KEEPALIVE ray_t* init_string_str(const char* s, ray_jsidx_t len) { + size_t n; + if (!jsidx_to_strlen(s, len, &n)) return ray_error("length", "init_string_str: invalid length"); + return ray_str(s, n); } /* ============================================================================ @@ -419,7 +470,12 @@ EMSCRIPTEN_KEEPALIVE int64_t read_symbol_id(ray_t* obj) { * thread-local scratch buffer so the returned pointer survives until the * next call (Emscripten's UTF8ToString already copies on the JS side * before the next call lands). */ -EMSCRIPTEN_KEEPALIVE const char* symbol_to_str(int64_t id) { +EMSCRIPTEN_KEEPALIVE const char* symbol_to_str(ray_jsidx_t id_f) { + int64_t id; + if (!jsidx_to_i64(id_f, &id)) { + g_sym_to_str_buf[0] = '\0'; + return g_sym_to_str_buf; + } ray_t* s = ray_sym_str(id); if (!s) { g_sym_to_str_buf[0] = '\0'; @@ -436,12 +492,13 @@ EMSCRIPTEN_KEEPALIVE const char* symbol_to_str(int64_t id) { /* Resolve a symbol vector cell through the vector's own domain. CSV/splayed * columns may use a file-local dictionary whose positions are not runtime * symbol IDs. */ -EMSCRIPTEN_KEEPALIVE const char* symbol_vec_get(ray_t* vec, int64_t idx) { - if (!vec || ray_type(vec) != RAY_SYM || idx < 0 || idx >= ray_len(vec)) { +EMSCRIPTEN_KEEPALIVE const char* symbol_vec_get(ray_t* vec, ray_jsidx_t idx) { + int64_t i; + if (!vec || ray_type(vec) != RAY_SYM || !jsidx_to_i64(idx, &i) || i >= ray_len(vec)) { g_sym_to_str_buf[0] = '\0'; return g_sym_to_str_buf; } - ray_t* s = ray_sym_vec_cell(vec, idx); /* borrowed from the domain */ + ray_t* s = ray_sym_vec_cell(vec, i); /* borrowed from the domain */ if (!s) { g_sym_to_str_buf[0] = '\0'; return g_sym_to_str_buf; @@ -467,13 +524,14 @@ EMSCRIPTEN_KEEPALIVE int64_t str_atom_len(ray_t* s) { /* Per-cell read of a RAY_STR vector. Copies into a thread-local scratch * buffer; lifetime as for symbol_to_str. */ -EMSCRIPTEN_KEEPALIVE const char* str_vec_get(ray_t* vec, int64_t idx) { - if (!vec) { +EMSCRIPTEN_KEEPALIVE const char* str_vec_get(ray_t* vec, ray_jsidx_t idx) { + int64_t i; + if (!vec || !jsidx_to_i64(idx, &i)) { g_str_vec_buf[0] = '\0'; return g_str_vec_buf; } size_t n = 0; - const char* p = ray_str_vec_get(vec, idx, &n); + const char* p = ray_str_vec_get(vec, i, &n); if (!p) { g_str_vec_buf[0] = '\0'; return g_str_vec_buf; @@ -494,13 +552,17 @@ EMSCRIPTEN_KEEPALIVE const char* str_vec_get(ray_t* vec, int64_t idx) { * typed-array view and have the engine see the elements. We bridge that * by setting v->len = capacity after allocation; data starts zero-filled * because mmap'd pages are zero-init. */ -EMSCRIPTEN_KEEPALIVE ray_t* init_vector(int8_t type, int64_t len) { +EMSCRIPTEN_KEEPALIVE ray_t* init_vector(int8_t type, ray_jsidx_t len_f) { + int64_t len; + if (!jsidx_to_i64(len_f, &len)) return ray_error("length", "init_vector: invalid length"); ray_t* v = (type == RAY_SYM) ? ray_sym_vec_new(RAY_SYM_W64, len) : ray_vec_new(type, len); if (v && !RAY_IS_ERR(v)) v->len = len; return v; } -EMSCRIPTEN_KEEPALIVE ray_t* init_list(int64_t len) { +EMSCRIPTEN_KEEPALIVE ray_t* init_list(ray_jsidx_t len_f) { + int64_t len; + if (!jsidx_to_i64(len_f, &len)) return ray_error("length", "init_list: invalid length"); ray_t* l = ray_list_new(len); if (l && !RAY_IS_ERR(l)) { /* Slots default to RAY_NULL_OBJ so iteration / drop is safe before @@ -564,8 +626,10 @@ static ray_t* box_vec_element(int8_t vec_type, const void* p) { } } -EMSCRIPTEN_KEEPALIVE ray_t* vec_at_idx(ray_t* obj, int64_t idx) { - if (!obj) return RAY_NULL_OBJ; +EMSCRIPTEN_KEEPALIVE ray_t* vec_at_idx(ray_t* obj, ray_jsidx_t idx_f) { + int64_t idx; + /* An unusable index reads like an out-of-range one: RAY_NULL_OBJ. */ + if (!obj || !jsidx_to_i64(idx_f, &idx)) return RAY_NULL_OBJ; int8_t t = ray_type(obj); if (t == RAY_LIST) { ray_t* item = ray_list_get(obj, idx); @@ -581,11 +645,14 @@ EMSCRIPTEN_KEEPALIVE ray_t* vec_at_idx(ray_t* obj, int64_t idx) { return box_vec_element(t, p); } -EMSCRIPTEN_KEEPALIVE ray_t* vec_set_idx(ray_t* obj, int64_t idx, ray_t* val) { +EMSCRIPTEN_KEEPALIVE ray_t* vec_set_idx(ray_t* obj, ray_jsidx_t idx_f, ray_t* val) { + int64_t idx; if (!obj || !val) return RAY_NULL_OBJ; + if (!jsidx_to_i64(idx_f, &idx)) return ray_error("index", "vec_set_idx: invalid index"); int8_t t = ray_type(obj); if (t == RAY_LIST) { - ray_retain(val); + /* ray_list_set retains `val` itself — the caller's ref stays its own. + * Retaining here too would strand one ref per element. */ return ray_list_set(obj, idx, val); } if (t == RAY_STR) { @@ -601,7 +668,7 @@ EMSCRIPTEN_KEEPALIVE ray_t* vec_push(ray_t* obj, ray_t* val) { if (!obj || !val) return RAY_NULL_OBJ; int8_t t = ray_type(obj); if (t == RAY_LIST) { - ray_retain(val); + /* ray_list_append retains `val` itself — see vec_set_idx. */ return ray_list_append(obj, val); } if (t == RAY_STR) { @@ -613,11 +680,13 @@ EMSCRIPTEN_KEEPALIVE ray_t* vec_push(ray_t* obj, ray_t* val) { return ray_vec_append(obj, sp); } -EMSCRIPTEN_KEEPALIVE ray_t* vec_insert(ray_t* obj, int64_t idx, ray_t* val) { +EMSCRIPTEN_KEEPALIVE ray_t* vec_insert(ray_t* obj, ray_jsidx_t idx_f, ray_t* val) { + int64_t idx; if (!obj || !val) return RAY_NULL_OBJ; + if (!jsidx_to_i64(idx_f, &idx)) return ray_error("index", "vec_insert: invalid index"); int8_t t = ray_type(obj); if (t == RAY_LIST) { - ray_retain(val); + /* ray_list_insert_at retains `val` itself — see vec_set_idx. */ return ray_list_insert_at(obj, idx, val); } if (t == RAY_STR) { @@ -637,22 +706,28 @@ EMSCRIPTEN_KEEPALIVE ray_t* vec_insert(ray_t* obj, int64_t idx, ray_t* val) { * the JS wrappers must build vectors fresh before filling. * ============================================================================ */ -EMSCRIPTEN_KEEPALIVE void fill_i64_vec(ray_t* obj, int64_t* data, int64_t len) { - if (!obj || !data || obj->type != RAY_I64) return; +EMSCRIPTEN_KEEPALIVE void fill_i64_vec(ray_t* obj, int64_t* data, ray_jsidx_t len_f) { + int64_t len; + if (!obj || !data || obj->type != RAY_I64 || !jsidx_to_i64(len_f, &len)) return; int64_t copy_len = len < ray_len(obj) ? len : ray_len(obj); - memcpy(ray_data(obj), data, copy_len * sizeof(int64_t)); + if (copy_len <= 0) return; + memcpy(ray_data(obj), data, (size_t)copy_len * sizeof(int64_t)); } -EMSCRIPTEN_KEEPALIVE void fill_i32_vec(ray_t* obj, int32_t* data, int64_t len) { - if (!obj || !data || obj->type != RAY_I32) return; +EMSCRIPTEN_KEEPALIVE void fill_i32_vec(ray_t* obj, int32_t* data, ray_jsidx_t len_f) { + int64_t len; + if (!obj || !data || obj->type != RAY_I32 || !jsidx_to_i64(len_f, &len)) return; int64_t copy_len = len < ray_len(obj) ? len : ray_len(obj); - memcpy(ray_data(obj), data, copy_len * sizeof(int32_t)); + if (copy_len <= 0) return; + memcpy(ray_data(obj), data, (size_t)copy_len * sizeof(int32_t)); } -EMSCRIPTEN_KEEPALIVE void fill_f64_vec(ray_t* obj, double* data, int64_t len) { - if (!obj || !data || obj->type != RAY_F64) return; +EMSCRIPTEN_KEEPALIVE void fill_f64_vec(ray_t* obj, double* data, ray_jsidx_t len_f) { + int64_t len; + if (!obj || !data || obj->type != RAY_F64 || !jsidx_to_i64(len_f, &len)) return; int64_t copy_len = len < ray_len(obj) ? len : ray_len(obj); - memcpy(ray_data(obj), data, copy_len * sizeof(double)); + if (copy_len <= 0) return; + memcpy(ray_data(obj), data, (size_t)copy_len * sizeof(double)); } /* ============================================================================ @@ -694,8 +769,8 @@ EMSCRIPTEN_KEEPALIVE ray_t* dict_get(ray_t* d, ray_t* key) { * * v2 builds tables column-by-column; init_table replays the v1 contract by * iterating the JS-supplied (sym-vec, list-of-cols) and chaining add_col. - * Each col is retained once because ray_table_add_col → ray_list_append - * consumes the ref. + * ray_table_add_col retains each col itself, so the caller's refs (held by + * the JS wrappers) survive the call untouched. * ============================================================================ */ EMSCRIPTEN_KEEPALIVE ray_t* init_table(ray_t* keys, ray_t* vals) { @@ -711,9 +786,10 @@ EMSCRIPTEN_KEEPALIVE ray_t* init_table(ray_t* keys, ray_t* vals) { int64_t* key_ids = (int64_t*)ray_data(keys); for (int64_t i = 0; i < n; i++) { - ray_t* col = ray_list_get(vals, i); + ray_t* col = ray_list_get(vals, i); /* borrowed */ if (!col) continue; - ray_retain(col); + /* ray_table_add_col retains col_vec itself; an extra retain here would + * strand one ref per column. */ tbl = ray_table_add_col(tbl, key_ids[i], col); if (RAY_IS_ERR(tbl)) return tbl; } @@ -737,23 +813,30 @@ EMSCRIPTEN_KEEPALIVE ray_t* table_vals(ray_t* t) { ray_t* lst = ray_list_new(n); for (int64_t i = 0; i < n; i++) { ray_t* col = ray_table_get_col_idx(t, i); /* borrowed */ - if (col) ray_retain(col); + /* ray_list_append takes the ref the returned list needs; retaining here + * too would strand one per column on every .values() call. */ lst = ray_list_append(lst, col ? col : RAY_NULL_OBJ); } return lst; } -EMSCRIPTEN_KEEPALIVE ray_t* table_col(ray_t* t, const char* name, int64_t len) { +EMSCRIPTEN_KEEPALIVE ray_t* table_col(ray_t* t, const char* name, ray_jsidx_t len) { + size_t n; if (!t || ray_type(t) != RAY_TABLE) return RAY_NULL_OBJ; - int64_t id = ray_sym_intern(name, (size_t)len); + if (!jsidx_to_strlen(name, len, &n)) return RAY_NULL_OBJ; + int64_t id = ray_sym_intern(name, n); ray_t* col = ray_table_get_col(t, id); /* borrowed */ if (col) ray_retain(col); return col ? col : RAY_NULL_OBJ; } /* Build a {col_name: col[idx]} dict for one row. */ -EMSCRIPTEN_KEEPALIVE ray_t* table_row(ray_t* t, int64_t idx) { +EMSCRIPTEN_KEEPALIVE ray_t* table_row(ray_t* t, ray_jsidx_t idx) { + int64_t row; if (!t || ray_type(t) != RAY_TABLE) return RAY_NULL_OBJ; + /* Checked here rather than left to vec_at_idx, so a bad index fails as a + * row instead of yielding a dict of nulls. */ + if (!jsidx_to_i64(idx, &row)) return RAY_NULL_OBJ; int64_t n = ray_table_ncols(t); ray_t* keys = ray_sym_vec_new(RAY_SYM_W64, n); ray_t* vals = ray_list_new(n); @@ -811,8 +894,11 @@ EMSCRIPTEN_KEEPALIVE ray_t* table_upsert(ray_t* t, ray_t* match_count, ray_t* da * Symbol / global env / type name * ============================================================================ */ -EMSCRIPTEN_KEEPALIVE int64_t intern_symbol(const char* s, int64_t len) { - return ray_sym_intern(s, (size_t)len); +/* Returns -1 for an invalid length; interned IDs are non-negative slots. */ +EMSCRIPTEN_KEEPALIVE int64_t intern_symbol(const char* s, ray_jsidx_t len) { + size_t n; + if (!jsidx_to_strlen(s, len, &n)) return -1; + return ray_sym_intern(s, n); } EMSCRIPTEN_KEEPALIVE ray_t* global_set(ray_t* name, ray_t* val) { diff --git a/src/rayforce.sdk.d.ts b/src/rayforce.sdk.d.ts index e5ccec0..6b6887f 100644 --- a/src/rayforce.sdk.d.ts +++ b/src/rayforce.sdk.d.ts @@ -482,7 +482,13 @@ export declare class RayforceSDK { * Set a global variable */ set(name: string, value: RayObject | any): void; - + + /** + * Delete a global binding, releasing the engine's ref to its value. + * Deleting an absent name is a no-op. + */ + unset(name: string): void; + /** * Get a global variable */ diff --git a/src/rayforce.sdk.js b/src/rayforce.sdk.js index fa250eb..9095e00 100644 --- a/src/rayforce.sdk.js +++ b/src/rayforce.sdk.js @@ -86,6 +86,21 @@ const TYPED_ARRAY_MAP = { [Types.SYM]: BigInt64Array, }; +// Keys in decoded objects come from user data (dict keys, column names), so a +// plain `obj[key] = v` silently drops `__proto__`: it hits Object.prototype's +// setter instead of creating an own property, and an object value would even +// re-point the result's prototype. defineProperty makes every key round-trip +// as an own enumerable data property while keeping Object.prototype available +// on the result (so `.hasOwnProperty`, spread and JSON.stringify behave). +function setOwn(obj, key, value) { + Object.defineProperty(obj, key, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +} + // ============================================================================ // Main SDK Class // ============================================================================ @@ -492,8 +507,15 @@ class RayforceSDK { keyView[i] = BigInt(this._internSymbol(keys[i], keys[i].length)); } - const valList = this.list(Object.values(obj).map(v => this._toRayObject(v))); - return new Dict(this, this._initDict(keyVec._ptr, valList._ptr)); + // Pass raw values: List.set converts and drops the temporaries itself. + const valList = this.list(Object.values(obj)); + try { + // init_dict retains both sides, so our two wrappers are still ours. + return new Dict(this, this._initDict(keyVec._ptr, valList._ptr)); + } finally { + valList.drop(); + keyVec.drop(); + } } /** @@ -511,10 +533,21 @@ class RayforceSDK { const valList = this.list(); for (const name of colNames) { - valList.push(this._arrayToVector(columns[name])); + const col = this._arrayToVector(columns[name]); + try { + valList.push(col); + } finally { + col.drop(); // the list holds its own ref now + } } - return new Table(this, this._initTable(keyVec._ptr, valList._ptr)); + try { + // init_table retains every column it adopts; keys/vals stay ours. + return new Table(this, this._initTable(keyVec._ptr, valList._ptr)); + } finally { + valList.drop(); + keyVec.drop(); + } } /** @@ -541,8 +574,8 @@ class RayforceSDK { } else if (first instanceof Date) { type = Types.TIMESTAMP; } else { - // Default to list for mixed types - return this.list(arr.map(v => this._toRayObject(v))); + // Default to list for mixed types (List.set owns the conversions) + return this.list(arr); } const vec = this.vector(type, arr.length); @@ -599,8 +632,42 @@ class RayforceSDK { */ set(name, value) { const sym = this.symbol(name); - const val = value instanceof RayObject ? value : this._toRayObject(value); - this._globalSet(sym._ptr, val._ptr); + const temp = !(value instanceof RayObject); + const val = temp ? this._toRayObject(value) : value; + try { + // ray_env_set retains `val` into the binding, so both of our handles + // are still ours to drop. The pointer global_set returns is borrowed + // from that binding — never wrap or drop it. + const ptr = this._globalSet(sym._ptr, val._ptr); + // A full env table (or a reserved name) comes back as an error block. + // Swallowing it turned a failed bind into a baffling "name undefined" + // from whatever read the binding next. + if (ptr && this._isObjError(ptr)) { + throw new Error(`set(${name}) failed: ${this._getErrorMessage(ptr)}`); + } + } finally { + if (temp) val.drop(); + sym.drop(); + } + } + + /** + * Delete a global binding, releasing the engine's ref to its value. + * Deleting an absent name is a no-op. + * @param {string} name + */ + unset(name) { + const sym = this.symbol(name); + try { + // global_set forwards a NULL value to ray_env_set, whose documented + // contract for NULL is "delete the slot and release its value". + const ptr = this._globalSet(sym._ptr, 0); + if (ptr && this._isObjError(ptr)) { + throw new Error(`unset(${name}) failed: ${this._getErrorMessage(ptr)}`); + } + } finally { + sym.drop(); + } } /** @@ -1204,8 +1271,15 @@ class List extends RayObject { */ set(idx, value) { if (idx < 0) idx = this.length + idx; - const obj = value instanceof RayObject ? value : this._sdk._toRayObject(value); - this._ptr = this._sdk._vecSetIdx(this._ptr, idx, obj._ptr); + const temp = !(value instanceof RayObject); + const obj = temp ? this._sdk._toRayObject(value) : value; + try { + this._rebind(this._sdk._vecSetIdx(this._ptr, idx, obj._ptr), 'set'); + } finally { + // The list took its own ref; wrappers we minted here are ours to drop. + // Caller-supplied RayObjects stay owned by the caller. + if (temp) obj.drop(); + } } /** @@ -1213,8 +1287,30 @@ class List extends RayObject { * new) parent pointer returned by ray_list_append. */ push(value) { - const obj = value instanceof RayObject ? value : this._sdk._toRayObject(value); - this._ptr = this._sdk._vecPush(this._ptr, obj._ptr); + const temp = !(value instanceof RayObject); + const obj = temp ? this._sdk._toRayObject(value) : value; + try { + this._rebind(this._sdk._vecPush(this._ptr, obj._ptr), 'push'); + } finally { + if (temp) obj.drop(); + } + } + + /** + * Adopt the pointer returned by a COW list op. Rebinding unconditionally + * would let a failed op replace a live list with null or an error object, + * turning a binding fault into silent data loss — surface it instead. + * @param {number} ptr + * @param {string} op + */ + _rebind(ptr, op) { + if (!ptr || this._sdk._isObjNull(ptr)) { + throw new Error(`List.${op}() failed: the engine returned null`); + } + if (this._sdk._isObjError(ptr)) { + throw new Error(`List.${op}() failed: ${this._sdk._getErrorMessage(ptr)}`); + } + this._ptr = ptr; } /** @@ -1224,11 +1320,19 @@ class List extends RayObject { toJS() { const result = []; for (let i = 0; i < this.length; i++) { - result.push(this.at(i).toJS()); + // at() hands back an owned ref (vec_at_idx retains list slots and boxes + // everything else fresh); reading through it doesn't consume it. + const el = this.at(i); + try { + result.push(el.toJS()); + } finally { + el.drop(); + } } return result; } + // Yielded elements are owned handles and become the consumer's to drop. *[globalThis.Symbol.iterator]() { for (let i = 0; i < this.length; i++) { yield this.at(i); @@ -1263,9 +1367,16 @@ class Dict extends RayObject { * @returns {RayObject} */ get(key) { - const keyObj = typeof key === 'string' ? this._sdk.symbol(key) : key; - const ptr = this._sdk._dictGet(this._ptr, keyObj._ptr); - return this._sdk._wrapPtr(ptr); + const temp = typeof key === 'string'; + const keyObj = temp ? this._sdk.symbol(key) : key; + try { + // ray_dict_get hands back an owned ref, so the wrapper we return owns + // it; the key we minted to look it up is a separate handle. + const ptr = this._sdk._dictGet(this._ptr, keyObj._ptr); + return this._sdk._wrapPtr(ptr); + } finally { + if (temp) keyObj.drop(); + } } /** @@ -1274,7 +1385,12 @@ class Dict extends RayObject { * @returns {boolean} */ has(key) { - return !this.get(key).isNull; + const value = this.get(key); + try { + return !value.isNull; + } finally { + value.drop(); // get() returns an owned ref we never hand out + } } /** @@ -1283,23 +1399,43 @@ class Dict extends RayObject { */ toJS() { const result = {}; + // keys()/values() go through dict_keys/dict_vals, which retain before + // handing the borrowed slot back — both wrappers are ours to drop. const keys = this.keys(); const vals = this.values(); - - for (let i = 0; i < keys.length; i++) { - const keyStr = this._sdk._symbolToStr(Number(keys.at(i))); - result[keyStr] = vals.at(i).toJS(); + + try { + for (let i = 0; i < keys.length; i++) { + const val = vals.at(i); + try { + // keys.at() already decodes SYM cells to strings via symbol_vec_get; + // re-wrapping in Number() gave NaN -> symbol_to_str(NaN) -> "", so every + // key collapsed to the same empty string and entries overwrote each other. + setOwn(result, keys.at(i), val.toJS()); + } finally { + val.drop(); + } + } + } finally { + vals.drop(); + keys.drop(); } - + return result; } + // The yielded values are owned handles — the consumer drops those. The + // finally still runs if the consumer abandons the loop early. *[globalThis.Symbol.iterator]() { const keys = this.keys(); const vals = this.values(); - for (let i = 0; i < keys.length; i++) { - const keyStr = this._sdk._symbolToStr(Number(keys.at(i))); - yield [keyStr, vals.at(i)]; + try { + for (let i = 0; i < keys.length; i++) { + yield [keys.at(i), vals.at(i)]; + } + } finally { + vals.drop(); + keys.drop(); } } } @@ -1322,13 +1458,17 @@ class Table extends RayObject { * @returns {string[]} */ columnNames() { - const cols = this.columns(); - const names = []; - for (let i = 0; i < cols.length; i++) { - // at() already converts symbol IDs to strings - names.push(cols.at(i)); + const cols = this.columns(); // table_keys builds a fresh sym vector + try { + const names = []; + for (let i = 0; i < cols.length; i++) { + // at() already converts symbol IDs to strings + names.push(cols.at(i)); + } + return names; + } finally { + cols.drop(); } - return names; } /** @@ -1391,12 +1531,16 @@ class Table extends RayObject { insert(data) { let insertData; if (Array.isArray(data)) { - insertData = this._sdk.list(data.map(v => this._sdk._toRayObject(v))); + insertData = this._sdk.list(data); } else { insertData = this._sdk.dict(data); } - const newPtr = this._sdk._tableInsert(this._ptr, insertData._ptr); - return this._sdk._wrapPtr(newPtr); + try { + const newPtr = this._sdk._tableInsert(this._ptr, insertData._ptr); + return this._sdk._wrapPtr(newPtr); + } finally { + insertData.drop(); + } } /** @@ -1406,12 +1550,21 @@ class Table extends RayObject { toJS() { const result = {}; const names = this.columnNames(); - const vals = this.values(); - - for (let i = 0; i < names.length; i++) { - result[names[i]] = vals.at(i).toJS(); + const vals = this.values(); // table_vals builds a fresh owned list + + try { + for (let i = 0; i < names.length; i++) { + const col = vals.at(i); + try { + setOwn(result, names[i], col.toJS()); + } finally { + col.drop(); + } + } + } finally { + vals.drop(); } - + return result; } @@ -1423,19 +1576,28 @@ class Table extends RayObject { const names = this.columnNames(); const count = this.rowCount; const rows = []; - - for (let i = 0; i < count; i++) { - const row = {}; - for (const name of names) { - row[name] = this.col(name).at(i); - if (typeof row[name] === 'bigint') { - const n = Number(row[name]); - row[name] = Number.isSafeInteger(n) ? n : row[name]; + + // Each col() retains the column, so the old per-cell lookup leaked one + // ref per cell — and re-resolved the name rows × columns times. Resolve + // each column once, read every row through it, then drop. + const cols = names.map(name => this.col(name)); + try { + for (let i = 0; i < count; i++) { + const row = {}; + for (let c = 0; c < names.length; c++) { + let cell = cols[c].at(i); + if (typeof cell === 'bigint') { + const n = Number(cell); + if (Number.isSafeInteger(n)) cell = n; + } + setOwn(row, names[c], cell); } + rows.push(row); } - rows.push(row); + } finally { + for (const col of cols) col.drop(); } - + return rows; } } @@ -1628,16 +1790,29 @@ class SelectQuery { * * v2's ray_select_fn evaluates the dict body as an AST node — its `from:` * slot expects an unevaluated symbol/expression, not a raw ray_t* stuffed - * in as an i64 atom. We bind the live table to a temporary global, - * render the query as a Rayfall string, eval, then leave the binding for - * the caller to clean up (eval's name resolution catches it). + * in as an i64 atom. We bind the live table to a temporary global, render + * the query as a Rayfall string, eval, then unbind. + * + * The unbind is not optional: the global env is a fixed 1024-slot table, so + * abandoning one binding per query used to pin every queried table in memory + * and then fail outright — around the 700th call ray_env_set returned OOM + * and the next eval reported the freshly-made name as undefined. * @returns {Table|RayError} */ execute() { const sdk = this._sdk; const tableSym = `__rfq_${++sdk._cmdCounter}`; sdk.set(tableSym, this._table); + try { + return this._run(tableSym); + } finally { + sdk.unset(tableSym); + } + } + /** @param {string} tableSym */ + _run(tableSym) { + const sdk = this._sdk; const parts = [`from: ${tableSym}`]; if (this._selectCols && this._selectCols.length) { diff --git a/test-bugs.mjs b/test-bugs.mjs new file mode 100644 index 0000000..64441b3 --- /dev/null +++ b/test-bugs.mjs @@ -0,0 +1,635 @@ +// Regression tests for the SDK bugs reported against 0.2.1. +// +// Bug 1 - int64_t params in non-final position silently return RAY_NULL. +// `-s WASM_BIGINT=0` legalizes every int64_t parameter into two i32 +// words, but the cwrap arg lists for `vec_set_idx` / `vec_insert` +// declare only three args, so `val` is eaten as the index high word +// and the real `val` arrives as 0. +// Bug 2 - Dict.toJS() and the dict iterator wrap an already-decoded symbol +// string in Number(), collapsing every key to "". +// Bug 3 - list/dict/table construction retained every element twice. +// Bug 4 - the reader family dropped the owned handles it read through. +// Bug 5 - the f64 index/length params introduced by the bug 1 fix were +// narrowed with an unchecked cast, so a negative length reached +// memcpy as a ~4 GiB byte count. +// +// Each group failed against 0.2.1 and passes against a fixed build. Cases +// marked "control" passed all along and are here to keep the diagnosis +// honest: they pin down the paths that are *not* broken, so a fix that +// regresses them is caught. +// +// Run with: node test-bugs.mjs + +import assert from 'node:assert/strict'; +import { init, Types } from './dist/index.js'; + +const rf = await init({ singleton: false }); + +const results = []; + +function check(bug, name, fn) { + try { + fn(); + results.push({ bug, name, ok: true }); + } catch (error) { + results.push({ bug, name, ok: false, error }); + } +} + +// ============================================================================ +// Bug 1 - list / dict construction +// ============================================================================ + +check('bug1', 'control: rf.list([]) builds an empty list', () => { + const list = rf.list([]); + assert.equal(list.type, Types.LIST); + assert.equal(rf.format(list), '()'); +}); + +check('bug1', 'control: (list 1 2.5) via eval builds a list', () => { + const list = rf.eval('(list 1 2.5)'); + assert.equal(list.type, Types.LIST); + assert.equal(rf.format(list), '(1 2.5)'); +}); + +check('bug1', 'rf.list([1, 2.5]) returns a LIST, not RAY_NULL', () => { + const list = rf.list([1, 2.5]); + assert.equal(list.type, Types.LIST, `got ${rf.typeName(list.type)} (${list.type})`); + assert.equal(rf.format(list), '(1 2.5)'); +}); + +check('bug1', 'rf.list([1, "two", 3.0]) preserves mixed types', () => { + const list = rf.list([1, 'two', 3.0]); + assert.equal(list.type, Types.LIST); + assert.equal(list.length, 3); + assert.deepEqual(list.toJS(), [1, 'two', 3.0]); +}); + +check('bug1', 'control: List.push() appends (no int64_t param on vec_push)', () => { + const list = rf.list([]); + list.push(1); + list.push(2.5); + assert.equal(list.type, Types.LIST); + assert.equal(rf.format(list), '(1 2.5)'); +}); + +check('bug1', 'List.set() replaces an element instead of destroying the list', () => { + const list = rf.eval('(list 1 2)'); + list.set(0, rf.i64(9)); + assert.equal(list.type, Types.LIST, `got ${rf.typeName(list.type)} (${list.type})`); + assert.equal(rf.format(list), '(9 2)'); +}); + +check('bug1', 'List.set() with a negative index writes from the end', () => { + const list = rf.eval('(list 1 2)'); + list.set(-1, rf.i64(9)); + assert.equal(rf.format(list), '(1 9)'); +}); + +check('bug1', 'rf.dict({x:1, y:2.5}) has non-null values', () => { + const dict = rf.dict({ x: 1, y: 2.5 }); + assert.equal(rf.format(dict), '{x:1 y:2.5}'); +}); + +check('bug1', 'Dict.values() of a natively built dict is a List', () => { + const dict = rf.dict({ x: 1, y: 2.5 }); + const vals = dict.values(); + assert.equal(vals.type, Types.LIST, `got ${rf.typeName(vals.type)} (${vals.type})`); + assert.deepEqual(vals.toJS(), [1, 2.5]); +}); + +check('bug1', 'Dict.toJS() of a natively built dict round-trips', () => { + // Currently throws TypeError: vals.at is not a function, because values() + // is a RayNull rather than a List. + assert.deepEqual(rf.dict({ x: 1, y: 2.5 }).toJS(), { x: 1, y: 2.5 }); +}); + +// --- Binding-level proof: the legalized wasm signature takes four i32 args. --- + +const wasm = rf._wasm; + +check('bug1', 'raw vec_set_idx(obj, idx, val) is not mis-legalized', () => { + // The SDK calls the export this way (3 args). With WASM_BIGINT=0 the wasm + // signature is (obj, idx_lo, idx_hi, val), so `val` lands in idx_hi and the + // `if (!obj || !val) return RAY_NULL_OBJ;` guard in main.c fires. + const ptr = wasm._vec_set_idx(rf._initList(2), 0, rf.i64(7)._ptr); + assert.notEqual(rf._getObjType(ptr), Types.NULL, 'vec_set_idx returned RAY_NULL'); + assert.equal(rf.format(ptr), '(7 null)'); +}); + +check('bug1', 'vec_set_idx takes exactly one arg per C parameter', () => { + // The pre-fix ABI needed the index split into (lo, hi). Now that the index + // is a single f64 there is no high word, so the old 4-arg call shifts `val` + // off the end. Pinning this keeps a future WASM_BIGINT change from silently + // reintroducing the split. + const ptr = wasm._vec_set_idx(rf._initList(2), 0, 0, rf.i64(7)._ptr); + assert.equal(rf._getObjType(ptr), Types.NULL, 'the split-index form should no longer apply'); +}); + +check('bug1', 'an out-of-range index errors instead of wrapping to 0', () => { + // The truncation this replaces was the dangerous part: index 2^32 used to + // alias index 0 and silently overwrite the wrong element. + const set = wasm._vec_set_idx(rf._initList(2), 2 ** 32, rf.i64(7)._ptr); + assert.equal(rf._getObjType(set), Types.ERR, `got ${rf.format(set)}`); + const insert = wasm._vec_insert(rf.eval('(list 1 2)')._ptr, 2 ** 32, rf.i64(9)._ptr); + assert.equal(rf._getObjType(insert), Types.ERR, `got ${rf.format(insert)}`); +}); + +check('bug1', 'raw vec_insert(obj, idx, val) is not mis-legalized', () => { + const ptr = wasm._vec_insert(rf.eval('(list 1 2)')._ptr, 1, rf.i64(9)._ptr); + assert.notEqual(rf._getObjType(ptr), Types.NULL, 'vec_insert returned RAY_NULL'); + assert.equal(rf.format(ptr), '(1 9 2)'); +}); + +check('bug1', 'List.set() surfaces a failed COW rebind instead of nulling itself', () => { + // The guard added alongside the binding fix: a list must never be silently + // replaced by null or an error object. + const list = rf.eval('(list 1 2)'); + assert.throws(() => list.set(2 ** 32, rf.i64(9)), /List\.set\(\) failed/); + assert.equal(rf.format(list), '(1 2)', 'the list survived the failed set'); +}); + +check('bug1', 'a trailing index param is not truncated to 32 bits', () => { + // vec_at_idx takes its index last, so the pre-fix int64_t signature appeared + // to work: the omitted high word defaulted to 0. Index 2^32 truncated to 0 + // and returned element 0 instead of null. + // (Vector.at() bounds-checks in JS, so this only shows at the export level.) + const list = rf.eval('(list 10 20)')._ptr; + assert.equal(rf.format(wasm._vec_at_idx(list, 0)), '10'); + assert.equal(rf.format(wasm._vec_at_idx(list, 2)), 'null'); + assert.equal( + rf.format(wasm._vec_at_idx(list, 2 ** 32)), + 'null', + 'index 2^32 truncated to 0 - the high word is being dropped', + ); +}); + +// ============================================================================ +// Bug 2 - Dict.toJS() and the dict iterator return empty keys +// ============================================================================ + +const dict = rf.eval('(dict [x y] (list 1 2.5))'); + +check('bug2', 'control: the dict itself formats correctly', () => { + assert.equal(rf.format(dict), '{x:1 y:2.5}'); +}); + +check('bug2', 'control: Dict.keys().toJS() decodes symbols to strings', () => { + assert.deepEqual(dict.keys().toJS(), ['x', 'y']); +}); + +check('bug2', 'control: Dict.values().toJS() is correct', () => { + assert.deepEqual(dict.values().toJS(), [1, 2.5]); +}); + +check('bug2', 'Dict.toJS() keeps the keys', () => { + // Vector.at() on a SYM vector already returns a string; toJS() re-wraps it + // in Number(), so every key becomes symbol_to_str(NaN) === "". + assert.deepEqual(dict.toJS(), { x: 1, y: 2.5 }); +}); + +check('bug2', 'Dict.toJS() does not collapse distinct keys into one entry', () => { + const wide = rf.eval('(dict [a b c] (list 1 2 3))'); + assert.equal(Object.keys(wide.toJS()).length, 3); +}); + +check('bug2', 'the dict iterator yields real keys', () => { + assert.deepEqual( + [...dict].map(([key, value]) => [key, value.toJS()]), + [['x', 1], ['y', 2.5]], + ); +}); + +check('bug2', 'Dict.get() still resolves a key read back from toJS()', () => { + const [key] = Object.keys(dict.toJS()); + assert.equal(dict.get(key).toJS(), 1); +}); + +check('bug2', 'Dict.toJS() round-trips a __proto__ key', () => { + // Plain `result[key] = value` hits Object.prototype's __proto__ setter + // instead of defining an own property, so the entry vanished - and an + // object-valued one silently re-pointed the result's prototype. + const proto = rf.eval('(dict [__proto__ x] (list 1 2))'); + const js = proto.toJS(); + assert.deepEqual(Object.keys(js), ['__proto__', 'x']); + assert.equal(js.__proto__, 1); + assert.equal(Object.getPrototypeOf(js), Object.prototype); +}); + +check('bug2', 'an object-valued __proto__ entry does not re-point the prototype', () => { + const proto = rf.eval('(dict [__proto__ x] (list (dict [a] (list 1)) 2))'); + const js = proto.toJS(); + assert.equal(Object.getPrototypeOf(js), Object.prototype); + assert.deepEqual(js.__proto__, { a: 1 }); +}); + +check('bug2', 'Table.toJS()/toRows() round-trip a __proto__ column', () => { + const table = rf.eval('(table [__proto__ x] (list [1 2] [3 4]))'); + assert.deepEqual(Object.keys(table.toJS()), ['__proto__', 'x']); + const rows = table.toRows(); + assert.deepEqual(rows.map(row => Object.keys(row)), [['__proto__', 'x'], ['__proto__', 'x']]); + assert.equal(rows[0].__proto__, 1); + assert.equal(Object.getPrototypeOf(rows[0]), Object.prototype); +}); + +// ============================================================================ +// Bug 3 - every list/dict element was retained twice +// +// vec_set_idx / vec_push / vec_insert called ray_retain(val) before handing +// the item to ray_list_set / _append / _insert_at, which retain it too. An +// element therefore went 1 -> 3, and dropping both the list and the caller's +// handle left one ref stranded forever. The SDK compounded it by minting +// temporary wrappers for raw JS values and never dropping them. +// ============================================================================ + +check('bug3', 'a list takes exactly one ref on push', () => { + const atom = rf.i64(42); + assert.equal(atom.refCount, 1, 'fresh atom should start at rc=1'); + const list = rf.list(); + list.push(atom); + assert.equal(atom.refCount, 2, 'list should hold exactly one ref'); + list.drop(); + assert.equal(atom.refCount, 1, 'dropping the list should return the ref'); + atom.drop(); +}); + +check('bug3', 'a list takes exactly one ref on set', () => { + const atom = rf.i64(7); + const list = rf.list([0, 0]); + list.set(0, atom); + assert.equal(atom.refCount, 2); + list.drop(); + assert.equal(atom.refCount, 1); + atom.drop(); +}); + +check('bug3', 'push does not steal the caller\'s ref', () => { + const atom = rf.i64(1); + const list = rf.list(); + list.push(atom); + list.drop(); + assert.equal(Number(atom.value), 1, 'the caller\'s handle must stay alive'); + atom.drop(); +}); + +// The engine grows the WASM heap in large steps, so a leak only shows after +// enough cycles to exhaust the current slack. Pre-fix these loops took the +// heap from 68 MB to 320 MB; 50k iterations alone showed nothing, so keep the +// counts high enough to stay diagnostic. +const CYCLES = 400_000; + +check('bug3', 'building and dropping lists does not grow the heap', () => { + for (let i = 0; i < 10_000; i++) rf.list([i, i + 1, 'tag']).drop(); // settle + const before = rf._wasm.HEAPU8.length; + for (let i = 0; i < CYCLES; i++) rf.list([i, i + 1, 'tag']).drop(); + assert.equal(rf._wasm.HEAPU8.length, before, `heap grew across ${CYCLES} cycles`); +}); + +check('bug3', 'building and dropping dicts does not grow the heap', () => { + for (let i = 0; i < 10_000; i++) rf.dict({ a: i, b: i + 0.5 }).drop(); + const before = rf._wasm.HEAPU8.length; + for (let i = 0; i < CYCLES; i++) rf.dict({ a: i, b: i + 0.5 }).drop(); + assert.equal(rf._wasm.HEAPU8.length, before, `heap grew across ${CYCLES} cycles`); +}); + +check('bug3', 'building and dropping tables does not grow the heap', () => { + const build = i => rf.table({ id: [i, i + 1], name: ['a', 'b'] }); + for (let i = 0; i < 10_000; i++) build(i).drop(); + const before = rf._wasm.HEAPU8.length; + for (let i = 0; i < CYCLES; i++) build(i).drop(); + assert.equal(rf._wasm.HEAPU8.length, before, `heap grew across ${CYCLES} cycles`); +}); + +// rf.set() and Dict.get()/has() minted symbol wrappers for their string +// arguments and dropped none of them; ray_env_set and ray_dict_get both take +// their own refs, so every call stranded one. +check('bug3', 'rf.set() does not leak its symbol/value wrappers', () => { + for (let i = 0; i < 10_000; i++) rf.set('g', i); + const before = rf._wasm.HEAPU8.length; + for (let i = 0; i < CYCLES; i++) rf.set('g', i); + assert.equal(rf._wasm.HEAPU8.length, before, `heap grew across ${CYCLES} cycles`); + assert.equal(Number(rf.eval('g').toJS()), CYCLES - 1, 'the binding must still be readable'); +}); + +check('bug3', 'rf.set() keeps a caller-supplied value alive', () => { + const atom = rf.i64(99); + rf.set('kept', atom); + assert.equal(Number(atom.value), 99, 'the caller\'s handle must survive set()'); + assert.equal(Number(rf.eval('kept').toJS()), 99); + atom.drop(); + assert.equal(Number(rf.eval('kept').toJS()), 99, 'the binding holds its own ref'); +}); + +check('bug3', 'Dict.get()/has() do not leak the looked-up key or value', () => { + const dict3 = rf.dict({ a: 1, b: 2 }); + for (let i = 0; i < 10_000; i++) { dict3.get('a').drop(); dict3.has('b'); } + const before = rf._wasm.HEAPU8.length; + for (let i = 0; i < CYCLES; i++) { dict3.get('a').drop(); dict3.has('b'); } + assert.equal(rf._wasm.HEAPU8.length, before, `heap grew across ${CYCLES} cycles`); + assert.equal(dict3.get('a').toJS(), 1, 'lookups must still work'); + assert.equal(dict3.has('b'), true); + assert.equal(dict3.has('zz'), false); + dict3.drop(); +}); + +// ============================================================================ +// Bug 4 - readers discarded the owned handles they read through +// +// dict_keys/dict_vals/table_keys/table_vals/table_col/vec_at_idx all hand +// back an owned ref. The toJS()/iterator/columnNames() family read one +// field off each and dropped it on the floor, so simply *inspecting* a +// container leaked. Table.toRows() was worst: one column ref per cell. +// +// Separately, SelectQuery.execute() bound the table to a fresh __rfq_N +// global per call and never unbound it. The env table is a fixed 1024 +// slots, so this pinned every queried table and then broke queries outright. +// ============================================================================ + +// These readers hand back refs to objects that already exist, so a leaked +// handle strands a refcount without allocating anything — a heap probe cannot +// see it (an earlier version of these tests passed against the unfixed SDK for +// exactly that reason). Probe the refcount of the underlying object instead: +// take a handle, read its rc, drop it. A reader that leaks moves that number. +function rcProbe(makeHandle) { + return () => { + const h = makeHandle(); + try { return h.refCount; } finally { h.drop(); } + }; +} + +function assertNoLeak(label, probe, fn, iterations = 200) { + const before = probe(); + for (let i = 0; i < iterations; i++) fn(); + assert.equal(probe(), before, `${label} stranded a ref (+${probe() - before})`); +} + +// Fresh-allocation paths (table_keys/table_vals build new objects) can still +// be checked by heap growth, which catches leaks refcounts alone would miss. +function assertNoGrowth(label, fn, iterations = 100_000) { + for (let i = 0; i < 10_000; i++) fn(); // settle + const before = rf._wasm.HEAPU8.length; + for (let i = 0; i < iterations; i++) fn(); + assert.equal(rf._wasm.HEAPU8.length, before, `${label} grew the heap`); +} + +check('bug4', 'List.toJS() does not leak its elements', () => { + const list = rf.list([1, 'two', 3.5]); + assertNoLeak('List.toJS()', rcProbe(() => list.at(0)), () => list.toJS()); + assert.deepEqual(list.toJS(), [1, 'two', 3.5]); + list.drop(); +}); + +check('bug4', 'Dict.toJS() and its iterator do not leak keys/values', () => { + const d = rf.dict({ a: 1, b: 2.5 }); + const valsRc = rcProbe(() => d.values()); + const keysRc = rcProbe(() => d.keys()); + const elemRc = rcProbe(() => d.values().at(0)); + + assertNoLeak('Dict.toJS() vals', valsRc, () => d.toJS()); + assertNoLeak('Dict.toJS() keys', keysRc, () => d.toJS()); + assertNoLeak('Dict.toJS() elements', elemRc, () => d.toJS()); + assertNoLeak('Dict iterator', valsRc, () => { for (const [, v] of d) v.drop(); }); + + assert.deepEqual(d.toJS(), { a: 1, b: 2.5 }); + d.drop(); +}); + +check('bug4', 'abandoning the Dict iterator early still releases it', () => { + const d = rf.dict({ a: 1, b: 2, c: 3 }); + assertNoLeak('abandoned Dict iterator', rcProbe(() => d.values()), () => { + for (const [, v] of d) { v.drop(); break; } + }); + d.drop(); +}); + +check('bug4', 'Table.columnNames()/toJS()/toRows() do not leak', () => { + const t = rf.table({ id: [1, 2, 3], name: ['a', 'b', 'c'] }); + const colRc = rcProbe(() => t.col('id')); + + // toRows() leaked one column ref per cell; toJS()/columnNames() leaked the + // fresh sym-vector and list that table_keys/table_vals allocate. + assertNoLeak('Table.toRows()', colRc, () => t.toRows()); + assertNoLeak('Table.toJS()', colRc, () => t.toJS()); + assertNoGrowth('columnNames()', () => t.columnNames()); + assertNoGrowth('Table.toJS()', () => t.toJS()); + assertNoGrowth('Table.toRows()', () => t.toRows()); + + assert.deepEqual(t.columnNames(), ['id', 'name']); + assert.deepEqual(t.toJS(), { id: [1, 2, 3], name: ['a', 'b', 'c'] }); + assert.deepEqual(t.toRows(), [ + { id: 1, name: 'a' }, { id: 2, name: 'b' }, { id: 3, name: 'c' }, + ]); + t.drop(); +}); + +check('bug4', 'repeated queries do not exhaust the 1024-slot global env', () => { + const t = rf.table({ id: [1, 2, 3, 4, 5], v: [10, 20, 30, 40, 50] }); + for (let i = 0; i < 3000; i++) { + const result = t.where(rf.col('v').gt(20)).execute(); + assert.equal(result.isError, false, `query #${i} failed: ${result.toString()}`); + if (i === 2999) assert.equal(result.rowCount, 3, 'query must still be correct'); + result.drop(); + } + t.drop(); +}); + +check('bug4', 'rf.unset() removes a binding and releases its value', () => { + // NB: eval() returns an owned ref — an env lookup retains the bound value, + // so holding one of these would itself show up in refCount below. + const read = name => { const r = rf.eval(name); try { return r.toJS(); } finally { r.drop(); } }; + const isUnbound = name => { const r = rf.eval(name); try { return r.isError; } finally { r.drop(); } }; + + const atom = rf.i64(5); + rf.set('tmpbind', atom); + assert.equal(Number(read('tmpbind')), 5); + assert.equal(atom.refCount, 2, 'the binding should hold one ref'); + rf.unset('tmpbind'); + assert.equal(atom.refCount, 1, 'unset must release the binding\'s ref'); + assert.equal(isUnbound('tmpbind'), true, 'the name should be gone'); + rf.unset('tmpbind'); // deleting an absent name is a no-op + atom.drop(); +}); + +// ============================================================================ +// Bug 5 - f64 index/length params were narrowed without validation +// +// The fix for bug 1 moved every index/length parameter to `double` +// (ray_jsidx_t) to dodge i64 legalization, but each one was then narrowed +// with a bare `(int64_t)` cast. A double that is negative, fractional, NaN +// or Infinity has no defined narrowing, and a negative length reached +// memcpy as a byte count that wraps to nearly UINT32_MAX on wasm32: +// `_fill_i32_vec(vec, buf, -1)` was a linear-memory overwrite. +// +// The SDK normalizes and bounds-checks indices before it calls in, so these +// only bite at the export level - which is exactly where an embedder or a +// fuzzer reaches. Every export must now reject the value instead of +// narrowing it: no trap, no corruption, no garbage handle. +// ============================================================================ + +// Everything JS can put in a `double` that is not a usable index or length. +const BAD_IDX = [-1, NaN, Infinity, -Infinity, 2.5, -0.5, 2 ** 53 + 2]; + +// Copy a JS string into a NUL-terminated buffer, as cwrap's 'string' does. +function withCStr(s, fn) { + const size = wasm.lengthBytesUTF8(s) + 1; + const p = wasm._malloc(size); + wasm.stringToUTF8(s, p, size); + try { return fn(p); } finally { wasm._free(p); } +} + +for (const [name, fn, type, cell] of [ + ['fill_i64_vec', '_fill_i64_vec', Types.I64, 7n], + ['fill_i32_vec', '_fill_i32_vec', Types.I32, 7], + ['fill_f64_vec', '_fill_f64_vec', Types.F64, 7], +]) { + check('bug5', `${name}() with a bogus length is a no-op, not a wrapped memcpy`, () => { + for (const bad of BAD_IDX) { + const vec = rf.vector(type, [cell, cell, cell, cell]); + const before = String(Array.from(vec.typedArray)); + const scratch = wasm._malloc(64); + wasm[fn](vec._ptr, scratch, bad); + assert.equal( + String(Array.from(vec.typedArray)), before, + `length ${bad} wrote through to the vector`, + ); + wasm._free(scratch); + vec.drop(); + } + }); +} + +check('bug5', 'control: the heap is intact after the fill probes', () => { + // A canary, not a diagnostic: pre-fix this passed too, because on this + // Emscripten runtime the ~4 GiB copy trapped ("memory access out of + // bounds") before it could scribble. A runtime that clamps instead of + // trapping would corrupt linear memory here rather than throwing above. + assert.equal(rf.eval('(sum (til 100))').toJS(), 4950); +}); + +check('bug5', 'init_vector() / init_list() reject a bogus length', () => { + for (const bad of BAD_IDX) { + const vec = wasm._init_vector(Types.I64, bad); + assert.equal(wasm._is_obj_error(vec), 1, `init_vector accepted length ${bad}`); + const list = wasm._init_list(bad); + assert.equal(wasm._is_obj_error(list), 1, `init_list accepted length ${bad}`); + } +}); + +check('bug5', 'the string constructors reject a bogus length', () => { + withCStr('hello', p => { + for (const bad of BAD_IDX) { + assert.equal( + wasm._is_obj_error(wasm._init_string_str(p, bad)), 1, + `init_string_str accepted length ${bad}`, + ); + assert.equal( + wasm._is_obj_error(wasm._init_symbol_str(p, bad)), 1, + `init_symbol_str accepted length ${bad}`, + ); + assert.equal(wasm._intern_symbol(p, bad), -1, `intern_symbol accepted length ${bad}`); + } + }); +}); + +check('bug5', 'an overlong string length clamps to the NUL instead of over-reading', () => { + // Every call site marshals through cwrap's 'string', so the buffer is + // NUL-terminated and the real extent is knowable. + withCStr('hi', p => { + const atom = wasm._init_string_str(p, 4096); + assert.equal(wasm._is_obj_error(atom), 0, 'a merely-too-large length is still usable'); + assert.equal(wasm.UTF8ToString(wasm._str_atom_ptr(atom)), 'hi'); + assert.equal(wasm._str_atom_len(atom), 2, 'the length must be the clamped one'); + }); +}); + +check('bug5', 'the string readers return "" for a bogus index', () => { + const syms = rf.vector(Types.SYM, ['a', 'b']); + const strs = rf.eval('("aa";"bb")'); + for (const bad of BAD_IDX) { + assert.equal(wasm.UTF8ToString(wasm._symbol_to_str(bad)), '', `symbol_to_str(${bad})`); + assert.equal( + wasm.UTF8ToString(wasm._symbol_vec_get(syms._ptr, bad)), '', + `symbol_vec_get(${bad})`, + ); + assert.equal( + wasm.UTF8ToString(wasm._str_vec_get(strs._ptr, bad)), '', + `str_vec_get(${bad})`, + ); + } + syms.drop(); + strs.drop(); +}); + +check('bug5', 'the index ops reject a bogus index and leave the vector intact', () => { + const vec = rf.vector(Types.I64, [1n, 2n, 3n]); + const val = rf.i64(9); + for (const bad of BAD_IDX) { + assert.equal(wasm._is_obj_null(wasm._vec_at_idx(vec._ptr, bad)), 1, `vec_at_idx(${bad})`); + assert.equal( + wasm._is_obj_error(wasm._vec_set_idx(vec._ptr, bad, val._ptr)), 1, + `vec_set_idx(${bad})`, + ); + assert.equal( + wasm._is_obj_error(wasm._vec_insert(vec._ptr, bad, val._ptr)), 1, + `vec_insert(${bad})`, + ); + } + assert.equal(String(Array.from(vec.typedArray)), '1,2,3', 'the vector was mutated'); + vec.drop(); + val.drop(); +}); + +check('bug5', 'table_row() / table_col() reject a bogus index', () => { + const t = rf.table({ id: [1, 2, 3], name: ['a', 'b', 'c'] }); + for (const bad of BAD_IDX) { + // table_row forwards its index to vec_at_idx per column: without its own + // check it would return a dict of nulls rather than fail. + assert.equal(wasm._is_obj_null(wasm._table_row(t._ptr, bad)), 1, `table_row(${bad})`); + withCStr('id', p => { + assert.equal(wasm._is_obj_null(wasm._table_col(t._ptr, p, bad)), 1, `table_col(${bad})`); + }); + } + t.drop(); +}); + +check('bug5', 'control: the validated paths still work', () => { + const vec = rf.vector(Types.I64, [1n, 2n, 3n]); + assert.equal(vec.at(1), 2); + assert.equal(rf.format(wasm._vec_at_idx(vec._ptr, 2)), '3'); + vec.drop(); + + const t = rf.table({ id: [1, 2, 3], name: ['a', 'b', 'c'] }); + assert.deepEqual(t.toRows()[2], { id: 3, name: 'c' }); + assert.deepEqual(t.col('name').toJS(), ['a', 'b', 'c']); + t.drop(); + + assert.deepEqual(rf.vector(Types.SYM, ['x', 'y']).toJS(), ['x', 'y']); + assert.equal(rf.string('hello').toJS(), 'hello'); + assert.equal(rf.eval('(sum (til 1000))').toJS(), 499500); +}); + +// ============================================================================ +// Report +// ============================================================================ + +const failed = results.filter(r => !r.ok); + +for (const { bug, name, ok, error } of results) { + if (ok) { + console.log(` ok [${bug}] ${name}`); + } else { + console.log(` FAIL [${bug}] ${name}`); + console.log(` ${String(error.message).split('\n').join('\n ')}`); + } +} + +console.log( + `\n${results.length - failed.length}/${results.length} passed, ${failed.length} failed ` + + `(bug1: ${failed.filter(r => r.bug === 'bug1').length}, ` + + `bug2: ${failed.filter(r => r.bug === 'bug2').length}, ` + + `bug3: ${failed.filter(r => r.bug === 'bug3').length}, ` + + `bug4: ${failed.filter(r => r.bug === 'bug4').length}, ` + + `bug5: ${failed.filter(r => r.bug === 'bug5').length})`, +); + +if (failed.length > 0) process.exitCode = 1;