add audit
This commit is contained in:
@@ -188,11 +188,11 @@ JSC_CCALL(vector_normalize,
|
||||
double len = sqrt(x*x + y*y);
|
||||
if (len > 0) {
|
||||
JS_FRAME(js);
|
||||
JS_LOCAL(result, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, result, "x", number2js(js, x/len));
|
||||
JS_SetPropertyStr(js, result, "y", number2js(js, y/len));
|
||||
JS_ROOT(result, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, result.val, "x", number2js(js, x/len));
|
||||
JS_SetPropertyStr(js, result.val, "y", number2js(js, y/len));
|
||||
JS_RestoreFrame(_js_ctx, _js_gc_frame, _js_local_frame);
|
||||
ret = result;
|
||||
ret = result.val;
|
||||
}
|
||||
)
|
||||
|
||||
@@ -262,7 +262,13 @@ The module file (`rtree.c`) includes the library header and uses `cell.h` as usu
|
||||
|
||||
## GC Safety
|
||||
|
||||
ƿit uses a Cheney copying garbage collector. Any JS allocation — `JS_NewObject`, `JS_NewString`, `JS_SetPropertyStr`, etc. — can trigger GC, which **moves** heap objects to new addresses. C locals holding JSValue become stale after any allocating call.
|
||||
ƿit uses a **Cheney copying garbage collector**. Any JS allocation — `JS_NewObject`, `JS_NewString`, `JS_NewInt32`, `JS_SetPropertyStr`, `js_new_blob_stoned_copy`, etc. — can trigger GC, which **moves** heap objects to new addresses. Bare C locals holding `JSValue` become **dangling pointers** after any allocating call. This is not a theoretical concern — it causes real crashes that are difficult to reproduce because they depend on heap pressure.
|
||||
|
||||
### Checklist (apply to EVERY C function you write or modify)
|
||||
|
||||
1. Count the `JS_New*`, `JS_SetProperty*`, and `js_new_blob*` calls in the function
|
||||
2. If there are **2 or more**, the function **MUST** use `JS_FRAME` / `JS_ROOT` / `JS_RETURN`
|
||||
3. Every `JSValue` held in a C local across an allocating call must be rooted
|
||||
|
||||
### When you need rooting
|
||||
|
||||
@@ -274,22 +280,70 @@ JSC_CCALL(mymod_name,
|
||||
)
|
||||
```
|
||||
|
||||
If a function holds a heap object across further allocating calls, you must root it:
|
||||
If a function creates an object and then sets properties on it, you **must** root it — each `JS_SetPropertyStr` call is an allocating call that can trigger GC:
|
||||
|
||||
```c
|
||||
JSC_CCALL(vector_normalize,
|
||||
double x = js2number(js, argv[0]);
|
||||
double y = js2number(js, argv[1]);
|
||||
double len = sqrt(x*x + y*y);
|
||||
if (len > 0) {
|
||||
JS_FRAME(js);
|
||||
JS_LOCAL(result, JS_NewObject(js));
|
||||
// result is rooted — GC can update it through these calls:
|
||||
JS_SetPropertyStr(js, result, "x", number2js(js, x/len));
|
||||
JS_SetPropertyStr(js, result, "y", number2js(js, y/len));
|
||||
JS_RestoreFrame(_js_ctx, _js_gc_frame, _js_local_frame);
|
||||
ret = result;
|
||||
}
|
||||
// UNSAFE — will crash under GC pressure:
|
||||
JSValue obj = JS_NewObject(js);
|
||||
JS_SetPropertyStr(js, obj, "x", JS_NewInt32(js, 1)); // can GC → obj is stale
|
||||
JS_SetPropertyStr(js, obj, "y", JS_NewInt32(js, 2)); // obj may be garbage
|
||||
return obj;
|
||||
|
||||
// SAFE:
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(obj, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, obj.val, "x", JS_NewInt32(js, 1));
|
||||
JS_SetPropertyStr(js, obj.val, "y", JS_NewInt32(js, 2));
|
||||
JS_RETURN(obj.val);
|
||||
```
|
||||
|
||||
### Patterns
|
||||
|
||||
**Object with properties** — the most common pattern in this codebase:
|
||||
|
||||
```c
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(result, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, result.val, "width", JS_NewInt32(js, w));
|
||||
JS_SetPropertyStr(js, result.val, "height", JS_NewInt32(js, h));
|
||||
JS_SetPropertyStr(js, result.val, "pixels", js_new_blob_stoned_copy(js, data, len));
|
||||
JS_RETURN(result.val);
|
||||
```
|
||||
|
||||
**Array with loop** — root both the array and each element created in the loop:
|
||||
|
||||
```c
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(arr, JS_NewArray(js));
|
||||
for (int i = 0; i < count; i++) {
|
||||
JS_ROOT(item, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, item.val, "index", JS_NewInt32(js, i));
|
||||
JS_SetPropertyStr(js, item.val, "data", js_new_blob_stoned_copy(js, ptr, sz));
|
||||
JS_SetPropertyNumber(js, arr.val, i, item.val);
|
||||
}
|
||||
JS_RETURN(arr.val);
|
||||
```
|
||||
|
||||
**Nested objects** — root every object that persists across an allocating call:
|
||||
|
||||
```c
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(outer, JS_NewObject(js));
|
||||
JS_ROOT(inner, JS_NewArray(js));
|
||||
// ... populate inner ...
|
||||
JS_SetPropertyStr(js, outer.val, "items", inner.val);
|
||||
JS_RETURN(outer.val);
|
||||
```
|
||||
|
||||
**Inside `JSC_CCALL`** — use `JS_RestoreFrame` and assign to `ret`:
|
||||
|
||||
```c
|
||||
JSC_CCALL(mymod_make,
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(obj, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, obj.val, "x", number2js(js, 42));
|
||||
JS_RestoreFrame(_js_ctx, _js_gc_frame, _js_local_frame);
|
||||
ret = obj.val;
|
||||
)
|
||||
```
|
||||
|
||||
@@ -297,19 +351,18 @@ JSC_CCALL(vector_normalize,
|
||||
|
||||
| Macro | Purpose |
|
||||
|-------|---------|
|
||||
| `JS_FRAME(js)` | Save the GC and local frames. Required before any `JS_LOCAL`. |
|
||||
| `JS_LOCAL(name, init)` | Declare and root a JSValue. GC updates it through its address. |
|
||||
| `JS_FRAME(js)` | Save the GC frame. Required before any `JS_ROOT`. |
|
||||
| `JS_ROOT(name, init)` | Declare a `JSGCRef` and root its value. Access via `name.val`. |
|
||||
| `JS_LOCAL(name, init)` | Declare a rooted `JSValue` (GC updates it through its address). |
|
||||
| `JS_RETURN(val)` | Restore the frame and return a value. |
|
||||
| `JS_RETURN_NULL()` | Restore the frame and return `JS_NULL`. |
|
||||
| `JS_RETURN_EX()` | Restore the frame and return `JS_EXCEPTION`. |
|
||||
| `JS_RestoreFrame(...)` | Manual frame restore (for `JSC_CCALL` bodies that use `ret =`). |
|
||||
|
||||
### Rules of thumb
|
||||
### Error return rules
|
||||
|
||||
1. **One allocation, immediate return** — no rooting needed.
|
||||
2. **Object + property sets** — root the object with `JS_LOCAL`.
|
||||
3. **Array + loop** — root the array; if loop body creates objects, root the loop variable too with a manual `JSLocalRef`.
|
||||
4. **`CELL_USE_INIT` bodies** — always use `JS_FRAME` / `JS_LOCAL` / `JS_RETURN`.
|
||||
- Error returns **before** `JS_FRAME` can use plain `return JS_ThrowTypeError(...)` etc.
|
||||
- Error returns **after** `JS_FRAME` must use `JS_RETURN_EX()` or `JS_RETURN_NULL()` — never plain `return`, which would leak the GC frame.
|
||||
|
||||
### Migrating from gc_mark
|
||||
|
||||
|
||||
Reference in New Issue
Block a user