Null-fill every unsent parameter at every call entry (heap use-after-free, #1661)

changes/fixed/1661-underarity-param-null-fill.md

@@ -0,0 +1,11 @@
+- **An unsent parameter is null-filled at every call entry, with or without
+  defaults (#1661).** A 2+-parameter callee without defaults, called with
+  fewer arguments than it takes, left the unsent parameters unbound. Once the
+  JIT compiled the callee, reading one read a stale slot of a recycled call
+  env: a heap-use-after-free that aborted release builds in `malloc` (seen in
+  recursion around depth 1000 or more), or a stale non-null value. Even
+  without the JIT, a closure over an unsent parameter resolved the name in an
+  outer scope instead of reading `null`. Direct calls, JIT-to-JIT calls,
+  `dispatch` (both forms) and `task_spawn` now bind every unsent parameter to
+  `null`, as SPEC.md and `docs/llms.txt` already promised. The bug was also
+  present in v0.44.0.

src/builtins.c

@@ -6578,6 +6578,11 @@ Value* builtin_dispatch(Value *arg) {
         if (dpc > 0) {
             env_set_local(call_env, fn->data.fn.params[0], fn_arg);
         }
+        /* #1661: under-arity null-fill of [1, param_count), as CASE(DISPATCH)
+         * does. The entry reserve below makes the slots exist, but leaves them
+         * nameless, so a closure resolved an unsent param in an outer scope. */
+        for (int i = 1; i < dpc; i++)
+            env_set_local_owned(call_env, fn->data.fn.params[i], make_null());
         if (fn->data.fn.body_count == -1) {
             /* Bytecode function */
             EigsChunk *fn_chunk = (EigsChunk *)fn->data.fn.body;
@@ -6590,16 +6595,13 @@ Value* builtin_dispatch(Value *arg) {
              * MENTIONS eval (#459), so an unrelated eval reference silently
              * changed parameter binding.
              *
-             * CASE(DISPATCH) additionally null-fills slots [1, param_count)
-             * before running the prologue. That is NOT needed here and is
-             * deliberately omitted: this path enters through vm_run_ex, whose
-             * first act is `if (chunk->local_count > env->count)
-             * env_reserve_slots(...)`, so the slots exist by the time
-             * OP_DEFAULT_PARAM writes them. CASE(DISPATCH) is a mid-execution
-             * frame push and gets no such entry reserve, which is why it
-             * carries its own fill. Verified by removing the fill here: all
-             * repros still pass (mechanical-gates §42 — a mutation that
-             * survives may mean redundant, so prove which and delete it). */
+             * This path enters through vm_run_ex, whose first act is
+             * `if (chunk->local_count > env->count) env_reserve_slots(...)`,
+             * so OP_DEFAULT_PARAM always has a slot to write. That reserve
+             * leaves the slots NAMELESS, though, which is why the null-fill
+             * above binds [1, param_count) by name (#1661: the fill was once
+             * judged redundant here because no repro read the unsent param
+             * through a closure). */
             if (fn_chunk->local_count > dpc)
                 env_reserve_slots(call_env, fn_chunk->local_count);
             Value *result = vm_execute_argc(fn_chunk, call_env, dpc > 0 ? 1 : 0);

src/task.c

@@ -620,6 +620,11 @@ static Value *task_start(Task *t) {
     } else {
         for (int i = 0; i < fn->data.fn.param_count && i < t->argc; i++)
             env_set_local(call_env, fn->data.fn.params[i], t->args[i]);
+        /* #1661: under-arity null-fill, as at every other entry point. An
+         * unbound param resolved through the CLOSURE, so a closure in the
+         * task read an outer variable of the same name instead of null. */
+        for (int i = t->argc; i < fn->data.fn.param_count; i++)
+            env_set_local_owned(call_env, fn->data.fn.params[i], make_null());
     }
     t->run_env = call_env;                  /* Task owns it; base frame borrows */
     t->started = 1;

src/vm.c

@@ -474,8 +474,9 @@ static inline void vm_bind_fresh_param(Env *env, int slot_idx,
  *     frame's own ref — that ref transfers to the chunk);
  *   - count == max(param_count, local_count): a binding created
  *     mid-call (e.g. SET_NAME_LOCAL of a new name, __loop_exit__)
- *     must NOT resolve in the next invocation, and an underfed call
- *     (argc < param_count) leaves params unhashed — both park-reject.
+ *     must NOT resolve in the next invocation, so it park-rejects.
+ *     (An underfed call, argc < param_count, null-binds its unsent
+ *     params by name since #1661, so it parks like a full call.)
  * Parked slots are dropped to slot_null (no value pinning) with
  * assign_counts zeroed; param names/hash/binding_version survive.
  * The parked env keeps its owned parent ref, so the callee's closure
@@ -517,8 +518,9 @@ static inline int vm_park_call_env(EigsChunk *chunk, Env *env) {
     if (env->count != expected) return 0;
     /* Every param must be name-bound, or the recycled env would
      * resolve params by slot but not by name. A single-arg OP_DISPATCH
-     * to a multi-param fn binds only param 0 (slots 1+ come from
-     * env_reserve_slots, nameless) — reject those. */
+     * to a multi-param fn used to bind only param 0 (slots 1+ came from
+     * env_reserve_slots, nameless); every entry null-binds by name since
+     * #1661, and this stays as the guard. */
     for (int i = 0; i < chunk->param_count; i++)
         if (!env->names[i]) return 0;
     for (int i = 0; i < env->count; i++) {
@@ -2665,13 +2667,12 @@ int jit_helper_call(EigsChunk *caller_chunk, int argc, int resume_off) {
                 val_decref(arg_list);
             }
         }
-        if (can_default) {
-            for (int i = argc; i < param_count; i++) {
-                uint32_t ph = phashes ? phashes[i]
-                                      : env_hash_name(fn_val->data.fn.params[i]);
-                env_bind_fresh_param_slot(call_env,
-                    fn_val->data.fn.params[i], fn_val->data.fn.param_intern_tbl, ph, slot_null());
-            }
+        /* #1661: null-fill every unsent param, defaults or not (as CASE(CALL)). */
+        for (int i = call_env->count; i < param_count; i++) {
+            uint32_t ph = phashes ? phashes[i]
+                                  : env_hash_name(fn_val->data.fn.params[i]);
+            env_bind_fresh_param_slot(call_env,
+                fn_val->data.fn.params[i], fn_val->data.fn.param_intern_tbl, ph, slot_null());
         }
         if (fn_chunk->local_count > param_count)
             env_reserve_slots(call_env, fn_chunk->local_count);
@@ -3693,12 +3694,15 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume,
             slot_incref(s);
             vm_push_slot(s);
         } else {
-            /* Out-of-range read -> null. This is LOAD-BEARING, not a bug
+            /* Out-of-range read -> null. Until #1661 this was LOAD-BEARING
              * (#348 tried to make it an error and 18 call-semantics checks
-             * failed): an underfed call to a defaults-free function binds
-             * only argc slots, and reading a missing parameter resolves
-             * HERE — this null IS the "missing parameters bind to null"
-             * semantics (SPEC.md). Only SET is a hard error (below). */
+             * failed): an underfed call to a defaults-free function bound
+             * only argc slots, and a missing parameter read resolved HERE.
+             * The JIT's GET_LOCAL has no such bounds check and read a stale
+             * freelist slot instead — a use-after-free. Every call entry now
+             * null-fills [argc, param_count), so compiler-emitted reads stay
+             * in range; this null remains the interpreter's guard for
+             * hand-built bytecode. Only SET is a hard error (below). */
             vm_push_slot(slot_null());
         }
         DISPATCH();
@@ -4304,11 +4308,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume,
             call_env = env_new(fn_val->data.fn.closure);
 
             uint32_t *phashes = fn_val->data.fn.param_hashes;
-            /* When this chunk has trailing defaults and the caller is
-             * underfed, bind null placeholders for every unsent slot
-             * in [argc, param_count); OP_DEFAULT_PARAM later fills the
-             * defaulted ones. Non-defaulted underfed slots stay null,
-             * matching the no-default underfed semantics. */
+            /* An underfed caller leaves [argc, param_count) unsent; the
+             * fill after the binds gives every one a null binding, and
+             * OP_DEFAULT_PARAM later fills the defaulted ones. */
             int can_default = (fn_chunk->first_default < param_count) &&
                               ((int)argc < param_count);
             if (param_count > 1 && argc > 0) {
@@ -4335,13 +4337,18 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume,
                     val_decref(arg_list);
                 }
             }
-            if (can_default) {
-                for (int i = (int)argc; i < param_count; i++) {
-                    uint32_t ph = phashes ? phashes[i]
-                                          : env_hash_name(fn_val->data.fn.params[i]);
-                    env_bind_fresh_param_slot(call_env,
-                        fn_val->data.fn.params[i], fn_val->data.fn.param_intern_tbl, ph, slot_null());
-                }
+            /* #1661: null-fill EVERY unsent param, defaults or not. This
+             * fill used to run only `if (can_default)`, so a defaults-free
+             * callee left them unbound: env->count stayed below
+             * param_count (and below local_count when it equals
+             * param_count, so no reserve ran), the JIT's unchecked
+             * GET_LOCAL read a stale freelist slot — a heap-use-after-free —
+             * and a closure resolved the unsent name in an outer scope. */
+            for (int i = call_env->count; i < param_count; i++) {
+                uint32_t ph = phashes ? phashes[i]
+                                      : env_hash_name(fn_val->data.fn.params[i]);
+                env_bind_fresh_param_slot(call_env,
+                    fn_val->data.fn.params[i], fn_val->data.fn.param_intern_tbl, ph, slot_null());
             }
 
             /* Pre-allocate slots for non-captured locals (compiler-assigned slots
@@ -6540,8 +6547,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume,
             }
             /* DISPATCH always feeds argc=1; underfed tail slots
              * [1..param_count) get null placeholders, then
-             * OP_DEFAULT_PARAM fills the defaulted ones. */
-            if (dpc > 1 && fn_chunk->first_default < dpc) {
+             * OP_DEFAULT_PARAM fills the defaulted ones. #1661: with or
+             * without defaults — see CASE(CALL). */
+            if (dpc > 1) {
                 uint32_t *phashes = fn->data.fn.param_hashes;
                 for (int i = 1; i < dpc; i++) {
                     uint32_t ph = phashes ? phashes[i]

tests/sections/0fd-underarity-bind-1661.sh

@@ -0,0 +1,21 @@
+echo "[0fd] Under-arity parameter binding (#1661)"
+# Every call entry null-fills an unsent parameter by name, with or without
+# defaults. Before #1661 a defaults-free callee left it unbound: the JIT's
+# unchecked GET_LOCAL read a stale freelist slot (heap-use-after-free, a
+# malloc abort in release), and a closure resolved the name in an outer
+# scope. Each program runs at the default tier, the interpreter tier and the
+# forced-JIT tier (every chunk compiled on first entry).
+for __u1661 in repro:"^ok$" \
+               depth:"underarity depth: all passed" \
+               trycatch:"underarity try/catch: all passed" \
+               bind:"underarity bind: all passed" \
+               jitcall:"underarity jit call: all passed" \
+               dispatch_eval:"underarity dispatch fallback: all passed"; do
+    __u1661_f="test_underarity_1661_${__u1661%%:*}.eigs"
+    __u1661_m="${__u1661#*:}"
+    check_eigs_suite "under-arity ${__u1661%%:*}: default tier" "$__u1661_f" "$__u1661_m"
+    EIGS_JIT_OFF=1 check_eigs_suite "under-arity ${__u1661%%:*}: interpreter tier" "$__u1661_f" "$__u1661_m"
+    EIGS_JIT_ENTRY_THRESHOLD=1 check_eigs_suite "under-arity ${__u1661%%:*}: forced-JIT tier" "$__u1661_f" "$__u1661_m"
+done
+unset __u1661 __u1661_f __u1661_m
+echo ""

tests/test_underarity_1661_bind.eigs

@@ -0,0 +1,57 @@
+# #1661: every call entry binds every parameter. An unsent parameter is
+# null-filled BY NAME, with or without defaults, so a closure over it reads
+# null, never an outer variable of the same name. Before the fix the direct
+# call, task_spawn and dispatch left it unbound (the closure read "outer"),
+# and a JIT-compiled callee read a stale slot (stale_slot below).
+b is "outer"
+
+define two(a, b) as:
+    peek is () => b
+    return peek of []
+
+define three(a, b, c) as:
+    peek is () => [b, c]
+    return peek of []
+
+define with_def(a, b, c is 3) as:
+    peek is () => [b, c]
+    return peek of []
+
+assert of [(two of [1]) == null, "direct call, argc 1: unsent b is null"]
+assert of [(two of []) == null, "direct call, argc 0: unsent b is null"]
+assert of [(three of [1]) == [null, null], "direct call: unsent b and c are null"]
+assert of [(with_def of [1]) == [null, 3], "defaults: b is null, c takes 3"]
+
+h is spawn of [two, 1]
+assert of [(thread_join of h) == null, "spawn: unsent b is null"]
+t is task_spawn of [two, 1]
+assert of [(task_join of t) == null, "task_spawn: unsent b is null"]
+t3 is task_spawn of [with_def, 1]
+assert of [(task_join of t3) == [null, 3], "task_spawn: b is null, default c fires"]
+tbl is [two, three]
+assert of [(dispatch of [tbl, 0, 1]) == null, "dispatch: unsent b is null"]
+assert of [(dispatch of [tbl, 1, 1]) == [null, null], "dispatch: unsent b, c are null"]
+
+seen is []
+define key2(x, b) as:
+    peek is () => b
+    append of [seen, peek of []]
+    return x
+sort_by of [[3, 1, 2], key2]
+for v in seen:
+    assert of [v == null, "sort_by key: unsent b is null"]
+
+# A hot callee whose only body is the unsent-param read: the JIT compiles it
+# and its GET_LOCAL reads the slot with no bounds check.
+define stale_slot(A, Q) as:
+    return Q
+define churn(a, b) as:
+    h is (x) => x
+    return 0
+i is 0
+loop while i < 50:
+    churn of [1, [i, "stale"]]
+    assert of [(stale_slot of ([1])) == null, "a JIT-read unsent param is null"]
+    i is i + 1
+
+print of "underarity bind: all passed"

tests/test_underarity_1661_depth.eigs

@@ -0,0 +1,35 @@
+# #1661: an unsent parameter reads null at depth 1000 and 3000, on a callee
+# without defaults (no_def) and one with (with_def), whose unsent param takes
+# its default. Each depth runs twice: the first descent fills the env
+# freelist that the second one recycles.
+n is {"d": 0, "limit": 0, "bad": 0}
+
+define no_def(A, Q) as:
+    if Q != null:
+        n.bad is n.bad + 1
+    n.d is n.d + 1
+    if n.d > n.limit:
+        return 0
+    local args is [A]
+    return no_def of (args)
+
+define with_def(A, Q is 7) as:
+    if Q != 7:
+        n.bad is n.bad + 1
+    n.d is n.d + 1
+    if n.d > n.limit:
+        return 0
+    local args is [A]
+    return with_def of (args)
+
+for depth in [1000, 3000]:
+    n.limit is depth
+    for round in [1, 2]:
+        n.d is 0
+        no_def of ([1])
+        assert of [n.d == depth + 1, "no_def descended to depth " + (str of depth)]
+        n.d is 0
+        with_def of ([1])
+        assert of [n.d == depth + 1, "with_def descended to depth " + (str of depth)]
+assert of [n.bad == 0, "an unsent param read a non-default value"]
+print of "underarity depth: all passed"

tests/test_underarity_1661_dispatch_eval.eigs

@@ -0,0 +1,11 @@
+# #1661: the C fallback of `dispatch` — the compiler takes it instead of
+# OP_DISPATCH whenever the unit mentions eval (#459). It bound only the first
+# parameter and left the rest nameless, so a closure read an outer variable.
+b is "outer"
+define two(a, b) as:
+    peek is () => b
+    return peek of []
+tbl is [two]
+assert of [(eval of "1 + 1") == 2, "eval is mentioned, so dispatch falls back"]
+assert of [(dispatch of [tbl, 0, 1]) == null, "dispatch fallback: unsent b is null"]
+print of "underarity dispatch fallback: all passed"

tests/test_underarity_1661_jitcall.eigs

@@ -0,0 +1,19 @@
+# #1661: the unsent-param read reached JIT-to-JIT. With every chunk compiled
+# (EIGS_JIT_ENTRY_THRESHOLD=1), drive's thunk calls stale_q through
+# jit_helper_call, which carries its own copy of the frame push and of the
+# under-arity null-fill. churn frees a captured env, holding a heap value in
+# slot 1, to the env freelist; the next call env recycles it. Kept in a file
+# of its own: other calls before it change which env is recycled.
+define stale_q(A, Q) as:
+    return Q
+define drive(x) as:
+    return stale_q of (x)
+define churn(a, b) as:
+    h is (x) => x
+    return 0
+i is 0
+loop while i < 50:
+    churn of [1, [i, "stale"]]
+    assert of [(drive of (i)) == null, "a JIT-to-JIT unsent param is null"]
+    i is i + 1
+print of "underarity jit call: all passed"

tests/test_underarity_1661_repro.eigs

@@ -0,0 +1,17 @@
+# #1661: the issue's reproducer, verbatim. A 2-param callee called
+# under-arity reads its unsent param at recursion depth 3000; the JIT read a
+# stale freelist slot (heap-use-after-free; release aborted in malloc).
+n is {"d": 0}
+define g(A, Q) as:
+    if Q == 1:
+        return 0
+    n.d is n.d + 1
+    if n.d > 3000:
+        return 0
+    local args is [A]
+    return g of (args)
+
+g of ([1])
+n.d is 0
+g of ([1])
+print of "ok"

tests/test_underarity_1661_trycatch.eigs

@@ -0,0 +1,22 @@
+# #1661: the try/catch variant — the same under-arity recursion inside a
+# try, then a raise and catch. The stale slot read was freed again on the
+# way out (release: "free(): double free detected in tcache 2").
+n is {"d": 0}
+define g(A, Q) as:
+    if Q == 1:
+        return 0
+    n.d is n.d + 1
+    if n.d > 3000:
+        throw of "deep"
+    local args is [A]
+    return g of (args)
+
+caught is 0
+for round in [1, 2, 3]:
+    n.d is 0
+    try:
+        g of ([1])
+    catch err:
+        caught is caught + 1
+assert of [caught == 3, "each descent raised and was caught"]
+print of "underarity try/catch: all passed"