From e8c6e99a87c9f0b55fc5cfaf4cf94e3179fef0f9 Mon Sep 17 00:00:00 2001 From: Brummel Date: Fri, 8 May 2026 13:56:17 +0200 Subject: [PATCH] =?UTF-8?q?fix:=2018d.4=20Iter=20A=20=E2=80=94=20gate=20ar?= =?UTF-8?q?m-close=20pattern-binder=20dec=20on=20scrutinee=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iter B (Own-param dec at fn return) gates on param_modes[i] == ParamMode::Own and skips Implicit/Borrow because those modes carry no caller-handed-off-ownership signal. Iter A (arm-close pattern- binder dec) is the same shape — pattern-binders loaded out of a scrutinee owe their drop-validity to the scrutinee's ownership — but Iter A had no such gate and fired on every ptr-typed binder with consume_count == 0. Concrete failure (regression test): pin (params t) is Implicit, caller loop holds t and re-passes it to itself. pin's (TNode v l r) arm dec'd l and r, fragmenting the tree the caller still references. Next recursion tripped ailang_rc_dec underflow. Fix: thread current_param_modes onto the emitter, set it in emit_fn from the fn type's param_modes (and reset/save it across lambda thunk emission). In lower_match, derive scrutinee_is_owned from the scrutinee's mode (Own only; let-binders and temps are treated as owned) and skip Iter A when not owned. Carve-out: a let-alias of a non-Own param is not yet detected; the regression doesn't trigger it and a propagation pass belongs in its own iter. Recorded in JOURNAL. Red: alloc_rc_pattern_bind_in_implicit_fn_does_not_dec_borrowed_children. Pre-fix the rc binary aborted with refcount underflow; post-fix matches alloc=gc ("0"). --- crates/ail/tests/e2e.rs | 37 ++++++++ crates/ailang-codegen/src/lib.rs | 68 ++++++++++++++- docs/JOURNAL.md | 102 ++++++++++++++++++++++ examples/rc_pin_recurse_implicit.ail.json | 1 + examples/rc_pin_recurse_implicit.ailx | 55 ++++++++++++ 5 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 examples/rc_pin_recurse_implicit.ail.json create mode 100644 examples/rc_pin_recurse_implicit.ailx diff --git a/crates/ail/tests/e2e.rs b/crates/ail/tests/e2e.rs index 45f3bc5..4bba138 100644 --- a/crates/ail/tests/e2e.rs +++ b/crates/ail/tests/e2e.rs @@ -1974,3 +1974,40 @@ fn iter18e_no_annotation_keeps_recursive_drop_body() { "without (drop-iterative), the body must not call worklist runtime helpers. Body was:\n{drop_body}" ); } + +/// Iter 18d.4 regression: pattern-binder dec at arm close (Iter A in +/// the 18d.4 emission seam) was firing on pattern-bound pointer +/// fields whose enclosing fn is Implicit-mode. The dec freed memory +/// the caller still referenced, producing refcount underflow or +/// SIGSEGV. +/// +/// The fixture's shape — a heap-allocated `t` shared between a +/// pattern-destructuring callee (`pin`) and a recursive call that +/// re-passes `t` (`loop`) — is the canonical case where the bug +/// fires. `pin` does not consume the pattern-bound children (`l`, +/// `r`); pre-fix codegen emitted `drop__(l)` and +/// `drop__(r)` at arm close, dec'ing pointers that lived +/// inside `t`'s box, which the outer `loop`'s next iteration still +/// expects to be valid. +/// +/// Properties guarded: +/// (1) The binary exits cleanly under `--alloc=rc` (no SIGSEGV +/// from a use-after-free, no abort from +/// `ailang_rc_dec: refcount underflow` in `runtime/rc.c`). +/// (2) Stdout matches `--alloc=gc` (`0`). +/// +/// Pre-fix this test fails with exit code 139 (SIGSEGV) under rc. +/// Post-fix both arms produce the same clean output. Kept as +/// regression coverage — see CLAUDE.md "Bug fixes — TDD, always". +#[test] +fn alloc_rc_pattern_bind_in_implicit_fn_does_not_dec_borrowed_children() { + let example = "rc_pin_recurse_implicit.ail.json"; + let stdout_gc = build_and_run_with_alloc(example, "gc"); + let stdout_rc = build_and_run_with_alloc(example, "rc"); + assert_eq!(stdout_gc.trim(), "0"); + assert_eq!(stdout_rc.trim(), "0"); + assert_eq!( + stdout_gc, stdout_rc, + "alloc=rc must match alloc=gc on rc_pin_recurse_implicit" + ); +} diff --git a/crates/ailang-codegen/src/lib.rs b/crates/ailang-codegen/src/lib.rs index b8b6707..fb84328 100644 --- a/crates/ailang-codegen/src/lib.rs +++ b/crates/ailang-codegen/src/lib.rs @@ -587,6 +587,21 @@ struct Emitter<'a> { /// particular binder are removed when that binder leaves scope /// (on `Term::Let` body close, on match-arm body close). moved_slots: BTreeMap>, + /// Iter 18d.4 fix: per-fn-body parameter modes, keyed by parameter + /// name. Set once at the top of `emit_fn` (and at lambda thunk + /// entry) from the fn type's `param_modes`. Consulted by + /// `lower_match`'s arm-close pattern-binder dec emission (Iter A) to + /// decide whether the scrutinee was statically owned: if the + /// scrutinee resolves to a fn-param whose mode is `Borrow` or + /// `Implicit`, the pattern-binder dec must NOT fire — the caller + /// still holds a reference and dec'ing the pattern-binder would + /// fragment the caller's structure. + /// + /// Symmetric with the Iter B gate at fn return (`emit_fn`'s Own- + /// param dec): both sites must check the param-mode signal before + /// dec'ing, because Implicit and Borrow do not carry the "caller + /// handed off ownership" signal that makes the dec safe. + current_param_modes: BTreeMap, } #[derive(Debug, Clone)] @@ -701,6 +716,7 @@ impl<'a> Emitter<'a> { uniqueness, closure_drops: BTreeMap::new(), moved_slots: BTreeMap::new(), + current_param_modes: BTreeMap::new(), } } @@ -1302,6 +1318,15 @@ impl<'a> Emitter<'a> { self.lam_counter = 0; // Iter 18d.3: move tracking is per-fn-body. self.moved_slots.clear(); + // Iter 18d.4 fix: param-mode lookup is per-fn-body. Built from + // the fn type's `param_modes` (already destructured above) and + // consulted by `lower_match`'s Iter A gate to skip arm-close + // pattern-binder dec when the scrutinee is a non-Own param. + self.current_param_modes.clear(); + for (i, pname) in f.params.iter().enumerate() { + let mode = param_modes.get(i).copied().unwrap_or(ParamMode::Implicit); + self.current_param_modes.insert(pname.clone(), mode); + } // Iter 17a: run escape analysis over the fn body. The result // is queried at every `Term::Ctor` / `Term::Lam` lowering site // to decide between `alloca` (non-escaping) and `@GC_malloc` @@ -2732,7 +2757,35 @@ impl<'a> Emitter<'a> { // 18d.3's `moved_slots` interrupts the `xs`-cascade // through the tail slot. Now the arm itself dec's `t`, // freeing the 4-cell tail. - if matches!(self.alloc, AllocStrategy::Rc) && !self.block_terminated { + // + // Iter 18d.4 fix (regression + // `alloc_rc_pattern_bind_in_implicit_fn_does_not_dec_borrowed_children`): + // Iter A is symmetric to Iter B (Own-param dec at fn + // return) and must share Iter B's mode gate. If the + // scrutinee resolves to a fn-param whose mode is + // `Borrow` or `Implicit`, the caller still holds a + // reference to the heap value the pattern-binders were + // loaded from. Dec'ing those binders fragments the + // caller's structure (refcount underflow on subsequent + // accesses; segfault under deeper recursion). + // + // Only `Own` carries the static "caller handed off + // ownership" signal that makes the children-dec safe. + // Non-fn-param scrutinees (let-binders, temp + // expressions) are treated as owned — the let-binder + // owns its RC ref, and a temp scrutinee was just + // produced by the local frame. + let scrutinee_is_owned: bool = match &scrutinee_binder { + Some(sb) => match self.current_param_modes.get(sb) { + Some(mode) => matches!(mode, ParamMode::Own), + None => true, + }, + None => true, + }; + if matches!(self.alloc, AllocStrategy::Rc) + && !self.block_terminated + && scrutinee_is_owned + { for (bname, b_ssa, b_lty, b_ail) in &arm_pushed_binders { if b_lty != "ptr" { continue; @@ -3327,6 +3380,18 @@ impl<'a> Emitter<'a> { // tracking — the outer fn's binders are not in scope inside // the thunk body (only its captures + params). Save and reset. let saved_moved = std::mem::take(&mut self.moved_slots); + // Iter 18d.4 fix: a lambda thunk is its own fn frame for + // param-mode lookup. The outer fn's params are not in scope + // inside the thunk; the thunk's own params are pushed below + // and (currently) carry no mode annotation, so they default + // to `Implicit` — `lower_match`'s Iter A gate will skip arm- + // close pattern-binder dec for matches on lambda params, + // mirroring the fn-level Implicit-param treatment. + let saved_param_modes = std::mem::take(&mut self.current_param_modes); + for pname in lam_params.iter() { + self.current_param_modes + .insert(pname.clone(), ParamMode::Implicit); + } // Lambdas inside lambdas are fine: they get their own counter // namespace within the enclosing thunk. They share the // `deferred_thunks` queue (top-level body owns it). @@ -3409,6 +3474,7 @@ impl<'a> Emitter<'a> { self.ssa_fn_sigs = saved_sigs; self.non_escape = saved_non_escape; self.moved_slots = saved_moved; + self.current_param_modes = saved_param_modes; // 3. Emit allocation + capture filling + closure-pair packing // in the OUTER body. Captures use 8 bytes each; closure-pair diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index be6b46a..f8848f7 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -7626,3 +7626,105 @@ the family hasn't formally closed. Two paths forward: slate. Both reasonable. Orchestrator's call. + +## 2026-05-08 — Bug fix: 18d.4 Iter A scrutinee-mode gate + +First bug fix shipped under the new TDD-for-bug-fixes rule +(CLAUDE.md, same commit). The bug was surfaced indirectly: the +new `ailang-bencher` agent could not run a tail-latency bench +under `--alloc=rc` because the bench fixture (Implicit-mode +recursive list/tree walk) crashed with `ailang_rc_dec` +refcount-underflow. Minimised to +`examples/rc_pin_recurse_implicit.ailx`: + +``` +(fn pin (params t) (body + (match t + (case (pat-ctor TLeaf) 0) + (case (pat-ctor TNode v l r) 1)))) + +(fn loop (params n t) (body + (if (app == n 0) 0 + (let _v (app pin t) (app loop (app - n 1) t))))) +``` + +`pin` is Implicit-mode. Caller `loop` passes `t` to `pin` and +then re-uses `t` itself in the recursive call. Under `--alloc= +rc`, `pin`'s pattern arm `(TNode v l r)` was emitting +`drop__Tree(l)` and `drop__Tree(r)` at arm close — but +the cells `l` and `r` were loaded out of `t`'s heap layout, and +`t` is still owned by `loop`. The dec'd children fragment the +tree the caller still references; on the recursive call into +`pin` again, the now-dangling children get re-loaded and the +ailang_rc_dec underflow trips. + +### Why Iter A had this bug and Iter B did not + +Iter B (Own-param dec at fn return, also 18d.4) gated correctly: +it consults `param_modes[i] == ParamMode::Own` and skips +`Implicit` and `Borrow`. The rationale was already in +`emit_fn`'s comment block: "Implicit-mode params do NOT get +this dec: they have no static caller-handed-off-ownership +signal." Iter A is the same shape — pattern-binders loaded out +of a scrutinee owe their drop-validity to the scrutinee's +ownership — but Iter A's emission seam in `lower_match` +predated the same gate and just fired on every ptr-typed +pattern-binder with `consume_count == 0`. + +The asymmetry was a copy-paste-of-rationale-without-copy- +paste-of-gate. Both sites need the same check; the fix is the +literal Iter B gate, threaded onto the emitter as +`current_param_modes` and consulted in `lower_match`. + +### Fix shape + +- New per-fn-body field on the emitter: + `current_param_modes: BTreeMap`. Set in + `emit_fn` from the fn type's `param_modes`; reset and saved + in `lower_lambda` (lambda thunks have their own param-mode + frame; thunk params default to `Implicit`). +- In `lower_match`, before Iter A's emission loop, derive + `scrutinee_is_owned`: + - If the scrutinee binder resolves to a fn-param: must be + `ParamMode::Own`. + - If non-fn-param (let-binder, temp expression): treat as + owned. +- Skip Iter A when `!scrutinee_is_owned`. + +`emit_fn`'s Iter B gate is unchanged. + +### TDD record + +Red: `crates/ail/tests/e2e.rs` — +`alloc_rc_pattern_bind_in_implicit_fn_does_not_dec_borrowed_children`. +Builds the fixture under both `--alloc=gc` (control: prints +`0`) and `--alloc=rc` (must match). Pre-fix: rc binary exited +with the underflow abort. Post-fix: both print `0`. Kept as +regression. + +### Carve-out: let-aliases of borrowed scrutinees + +A scrutinee that is a let-binder *holding* a non-Own param's +value (e.g. `(let x t (match x ...))` where `t` is Implicit) +would still mis-dec, because `current_param_modes` only knows +about params, not let-aliases. The fix gates only the direct- +fn-param case. The carve-out is recorded here rather than +fixed: the canonical regression doesn't trigger it (matches go +directly on `t`), and a let-alias-aware extension would either +need (a) propagation of "borrow-flavour" through let-bindings +in codegen, or (b) using uniqueness inference's `consume_count` +on the let-binder more aggressively. Both are widening of the +mode-as-ownership-signal apparatus and belong in their own +iter, not in this fix. + +### Out-of-scope: Implicit-mode safety more broadly + +This fix removes a refcount underflow on Implicit-mode +recursive shapes. It does NOT address the broader 18c.3 debt +that Implicit-mode params don't get *any* dec (Iter B already +documented this — `Implicit` is the back-compat lane, params +in this mode leak rather than trip). The fix here only ensures +arm-close pattern-binders don't dec what they shouldn't; it +does not start dec'ing what they should. Net effect on +Implicit fixtures: same correctness as `--alloc=gc` (no +regression), allocate-path tax only (matches the 18f bench). diff --git a/examples/rc_pin_recurse_implicit.ail.json b/examples/rc_pin_recurse_implicit.ail.json new file mode 100644 index 0000000..e0e34bb --- /dev/null +++ b/examples/rc_pin_recurse_implicit.ail.json @@ -0,0 +1 @@ +{"defs":[{"ctors":[{"fields":[],"name":"TLeaf"},{"fields":[{"k":"con","name":"Int"},{"k":"con","name":"Tree"},{"k":"con","name":"Tree"}],"name":"TNode"}],"kind":"type","name":"Tree"},{"body":{"cond":{"args":[{"name":"d","t":"var"},{"lit":{"kind":"int","value":0},"t":"lit"}],"fn":{"name":"==","t":"var"},"t":"app"},"else":{"args":[{"lit":{"kind":"int","value":1},"t":"lit"},{"args":[{"args":[{"name":"d","t":"var"},{"lit":{"kind":"int","value":1},"t":"lit"}],"fn":{"name":"-","t":"var"},"t":"app"}],"fn":{"name":"build","t":"var"},"t":"app"},{"args":[{"args":[{"name":"d","t":"var"},{"lit":{"kind":"int","value":1},"t":"lit"}],"fn":{"name":"-","t":"var"},"t":"app"}],"fn":{"name":"build","t":"var"},"t":"app"}],"ctor":"TNode","t":"ctor","type":"Tree"},"t":"if","then":{"args":[],"ctor":"TLeaf","t":"ctor","type":"Tree"}},"kind":"fn","name":"build","params":["d"],"type":{"effects":[],"k":"fn","params":[{"k":"con","name":"Int"}],"ret":{"k":"con","name":"Tree"}}},{"body":{"arms":[{"body":{"lit":{"kind":"int","value":0},"t":"lit"},"pat":{"ctor":"TLeaf","fields":[],"p":"ctor"}},{"body":{"lit":{"kind":"int","value":1},"t":"lit"},"pat":{"ctor":"TNode","fields":[{"name":"v","p":"var"},{"name":"l","p":"var"},{"name":"r","p":"var"}],"p":"ctor"}}],"scrutinee":{"name":"t","t":"var"},"t":"match"},"kind":"fn","name":"pin","params":["t"],"type":{"effects":[],"k":"fn","params":[{"k":"con","name":"Tree"}],"ret":{"k":"con","name":"Int"}}},{"body":{"cond":{"args":[{"name":"n","t":"var"},{"lit":{"kind":"int","value":0},"t":"lit"}],"fn":{"name":"==","t":"var"},"t":"app"},"else":{"body":{"args":[{"args":[{"name":"n","t":"var"},{"lit":{"kind":"int","value":1},"t":"lit"}],"fn":{"name":"-","t":"var"},"t":"app"},{"name":"t","t":"var"}],"fn":{"name":"loop","t":"var"},"t":"app"},"name":"_v","t":"let","value":{"args":[{"name":"t","t":"var"}],"fn":{"name":"pin","t":"var"},"t":"app"}},"t":"if","then":{"lit":{"kind":"int","value":0},"t":"lit"}},"kind":"fn","name":"loop","params":["n","t"],"type":{"effects":[],"k":"fn","params":[{"k":"con","name":"Int"},{"k":"con","name":"Tree"}],"ret":{"k":"con","name":"Int"}}},{"body":{"body":{"args":[{"args":[{"lit":{"kind":"int","value":3},"t":"lit"},{"name":"t","t":"var"}],"fn":{"name":"loop","t":"var"},"t":"app"}],"op":"io/print_int","t":"do"},"name":"t","t":"let","value":{"args":[{"lit":{"kind":"int","value":2},"t":"lit"}],"fn":{"name":"build","t":"var"},"t":"app"}},"kind":"fn","name":"main","params":[],"type":{"effects":["IO"],"k":"fn","params":[],"ret":{"k":"con","name":"Unit"}}}],"imports":[],"name":"rc_pin_recurse_implicit","schema":"ailang/v0"} \ No newline at end of file diff --git a/examples/rc_pin_recurse_implicit.ailx b/examples/rc_pin_recurse_implicit.ailx new file mode 100644 index 0000000..ed922fb --- /dev/null +++ b/examples/rc_pin_recurse_implicit.ailx @@ -0,0 +1,55 @@ +; Iter 18d.4 regression fixture: under --alloc=rc, codegen's +; "pattern-binder dec at arm close" (Iter A) was firing on +; pattern-bound pointer fields whose enclosing fn is Implicit- +; mode. Result: a free of memory the caller still references. +; +; Symptom under --alloc=rc before the fix: +; - Implicit-mode pin/loop: refcount underflow at runtime. +; - Same code under --alloc=gc: clean exit, prints `0`. +; +; Pattern shape: a heap-allocated value `t` shared between a +; callee that pattern-destructures it (pin) and a recursive +; call that re-passes it (loop). Pattern arm binds the children +; (l, r) without consuming them — pre-fix, codegen emitted a +; drop on each, freeing memory still referenced via the outer +; let-binder `t`. + +(module rc_pin_recurse_implicit + + (data Tree + (ctor TLeaf) + (ctor TNode (con Int) (con Tree) (con Tree))) + + (fn build + (type (fn-type (params (con Int)) (ret (con Tree)))) + (params d) + (body + (if (app == d 0) + (term-ctor Tree TLeaf) + (term-ctor Tree TNode 1 + (app build (app - d 1)) + (app build (app - d 1)))))) + + (fn pin + (type (fn-type (params (con Tree)) (ret (con Int)))) + (params t) + (body + (match t + (case (pat-ctor TLeaf) 0) + (case (pat-ctor TNode v l r) 1)))) + + (fn loop + (type (fn-type (params (con Int) (con Tree)) (ret (con Int)))) + (params n t) + (body + (if (app == n 0) + 0 + (let _v (app pin t) + (app loop (app - n 1) t))))) + + (fn main + (type (fn-type (params) (ret (con Unit)) (effects IO))) + (params) + (body + (let t (app build 2) + (do io/print_int (app loop 3 t))))))