From feb941363a94934f11c086dd46d314226e867474 Mon Sep 17 00:00:00 2001 From: Brummel Date: Thu, 14 May 2026 01:51:50 +0200 Subject: [PATCH] =?UTF-8?q?bugfix:=20print=20leak=20=E2=80=94=20propagate?= =?UTF-8?q?=20ret=5Fmode=20through=20rigid=20substitution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `(app print x)` under --alloc=rc leaked the heap-Str allocated by `show x` in print's body: the let-binder `s` in `(let s (app show x) (do io/print_str s))` was never flagged trackable, so the drop site at scope-close emitted no `ailang_rc_dec(s)`. Minimal repro `(body (app print 42))` produced `allocs=1 frees=0 live=1`. Root cause is two-layer: 1. examples/prelude.ail.json declared the Show class method `show` without an explicit `ret_mode` on the return type, so serde defaulted to ParamMode::Implicit. Fix: add `"ret_mode": "own"` on the Show.show method, parallel to how the heap-Str-producing builtins (int_to_str, bool_to_str, float_to_str, str_clone, str_concat) declare it. examples/prelude.ail regenerated via `ail render` to stay parse-isomorphic with the JSON. 2. crates/ailang-check/src/lib.rs `substitute_rigids` was silently stripping `param_modes` and `ret_mode` whenever it rebuilt a `Type::Fn`, so even after the prelude fix, the mono-synthesised `show__Int / Bool / Str / Float` lost the `Own` annotation during rigid substitution. Fix: preserve both fields through the substitution. RED pin: crates/ail/tests/print_no_leak_pin.rs (test `alloc_rc_print_int_does_not_leak_show_result_str`) + fixture examples/print_int_no_leak_pin.ail asserts allocs == frees and live == 0 for `(body (app print 42))` under --alloc=rc. Ten mono-body hash pins re-recorded (eq__Int/Bool/Str, compare__Int/Bool/Str, show__Int/Bool/Str/Float, eq__IntBox) — the bodies are semantically identical; the canonical JSON now carries the previously-stripped mode metadata, so the body hash moves. Re-pinning is bookkeeping for the intentional drift, not a workaround. Surfaced 2026-05-14 during the rpe.1 BLOCKED orchestrator run (Cat A). Existed in latent form since iter 24.3 (when Show + print shipped); only became user-observable when `(app print ...)` joined the corpus. cargo test --workspace: 565 / 0 / 3. Open follow-up flagged for next /audit: grep for `Type::Fn { ..., .. }` field-spread sites — any other shape that drops modes during rebuild is a latent instance of the same bug class. Out of scope for this minimal fix. --- ...-iter-bugfix-print-leak-show-ret-mode.json | 12 ++ crates/ail/tests/eq_ord_e2e.rs | 11 +- crates/ail/tests/mono_hash_stability.rs | 37 ++++-- crates/ail/tests/print_no_leak_pin.rs | 118 ++++++++++++++++++ crates/ailang-check/src/lib.rs | 16 ++- ...14-iter-bugfix-print-leak-show-ret-mode.md | 85 +++++++++++++ examples/prelude.ail | 2 +- examples/prelude.ail.json | 1 + examples/print_int_no_leak_pin.ail | 6 + 9 files changed, 271 insertions(+), 17 deletions(-) create mode 100644 bench/orchestrator-stats/2026-05-14-iter-bugfix-print-leak-show-ret-mode.json create mode 100644 crates/ail/tests/print_no_leak_pin.rs create mode 100644 docs/journals/2026-05-14-iter-bugfix-print-leak-show-ret-mode.md create mode 100644 examples/print_int_no_leak_pin.ail diff --git a/bench/orchestrator-stats/2026-05-14-iter-bugfix-print-leak-show-ret-mode.json b/bench/orchestrator-stats/2026-05-14-iter-bugfix-print-leak-show-ret-mode.json new file mode 100644 index 0000000..ee891c4 --- /dev/null +++ b/bench/orchestrator-stats/2026-05-14-iter-bugfix-print-leak-show-ret-mode.json @@ -0,0 +1,12 @@ +{ + "iter_id": "bugfix-print-leak-show-ret-mode", + "date": "2026-05-14", + "mode": "mini", + "outcome": "DONE", + "tasks_total": 1, + "tasks_completed": 1, + "reloops_per_task": { "1": 0 }, + "review_loops_spec": 0, + "review_loops_quality": 0, + "blocked_reason": null +} diff --git a/crates/ail/tests/eq_ord_e2e.rs b/crates/ail/tests/eq_ord_e2e.rs index 8a70659..5ae34af 100644 --- a/crates/ail/tests/eq_ord_e2e.rs +++ b/crates/ail/tests/eq_ord_e2e.rs @@ -118,9 +118,16 @@ fn eq_ord_user_adt_eq_intbox_hash_stable() { // The body hash pins the unified mono pass's IntBox eq emission. // Drift means either a legitimate refactor (re-record) or a // regression in user-instance body propagation (investigate). + // + // 2026-05-14 bugfix-print-leak-show-ret-mode: hash re-pinned + // (from `9daaffa7528d2a1c` to `3c4cf040cb4e8bb2`) because + // `substitute_rigids` now preserves `param_modes`/`ret_mode` + // through rigid substitution. Eq's class method declares + // `param_modes: ["borrow", "borrow"]` — the user-instance + // mono synthesis used to silently strip those. let h = def_hash(&Def::Fn(eq_intbox.clone())); assert_eq!( - h, "9daaffa7528d2a1c", - "eq__IntBox body hash drifted — see mono_hash_stability.rs for the resolution pattern" + h, "3c4cf040cb4e8bb2", + "eq__IntBox body hash drifted — see mono_hash_stability.rs for the resolution pattern; captured: {h}" ); } diff --git a/crates/ail/tests/mono_hash_stability.rs b/crates/ail/tests/mono_hash_stability.rs index 8f9fa90..fffccbc 100644 --- a/crates/ail/tests/mono_hash_stability.rs +++ b/crates/ail/tests/mono_hash_stability.rs @@ -38,13 +38,21 @@ fn primitive_eq_ord_mono_symbol_hashes_stay_bit_identical() { .get("prelude") .expect("prelude module present"); + // 2026-05-14 bugfix-print-leak-show-ret-mode: hashes re-pinned + // after `substitute_rigids` started preserving `param_modes` and + // `ret_mode` through rigid substitution (was: hard-reset to + // Implicit / `[]`). Eq/Ord class methods declare + // `param_modes: ["borrow", "borrow"]` in `prelude.ail.json`, so + // the mono-synthesised `eq__T` / `compare__T` symbols now carry + // those modes in their `Type::Fn` — canonical-JSON bytes drift, + // bodies are semantically identical. let pins: &[(&str, &str)] = &[ - ("eq__Int", "81d59ac38ab3663d"), - ("eq__Bool", "11da98a358b5979b"), - ("eq__Str", "277516bb7f195b2a"), - ("compare__Int", "d5d3f66b86c7e758"), - ("compare__Bool", "676e3ea0298a8795"), - ("compare__Str", "a532710899cf14fe"), + ("eq__Int", "8bf7bec502487a57"), + ("eq__Bool", "e46d7e6cd2ec459c"), + ("eq__Str", "2a92935174182d2f"), + ("compare__Int", "6d6c20520766368b"), + ("compare__Bool", "02b64e8fadc913eb"), + ("compare__Str", "9645929d53cd3cc9"), ]; for (sym, pin) in pins { @@ -54,7 +62,7 @@ fn primitive_eq_ord_mono_symbol_hashes_stay_bit_identical() { .find(|d| matches!(d, Def::Fn(f) if f.name == *sym)) .unwrap_or_else(|| panic!("mono symbol {sym} not found in prelude module")); let h = def_hash(def); - assert_eq!(&h, pin, "{sym} body hash drifted"); + assert_eq!(&h, pin, "{sym} body hash drifted; captured: {h}"); } } @@ -75,11 +83,18 @@ fn primitive_show_mono_symbol_hashes_stay_bit_identical() { .get("prelude") .expect("prelude module present"); + // 2026-05-14 bugfix-print-leak-show-ret-mode: hashes re-pinned + // after the Show class method declaration in `prelude.ail.json` + // added `ret_mode: "own"` AND `substitute_rigids` started + // preserving `param_modes`/`ret_mode`. The mono-synthesised + // `show__T` symbols now carry `param_modes: ["borrow"]` and + // `ret_mode: Own` in their `Type::Fn` — bodies are semantically + // identical. let pins: &[(&str, &str)] = &[ - ("show__Int", "891eaebe64b3180d"), - ("show__Bool", "1bdb621494e50607"), - ("show__Str", "3f1d57c90ddce081"), - ("show__Float", "5ab0bdd40b9f8b8f"), + ("show__Int", "9229ce65fdf11f61"), + ("show__Bool", "ade621119184351f"), + ("show__Str", "93fb52834427598c"), + ("show__Float", "49874c7ed31c9d15"), ]; for (sym, pin) in pins { diff --git a/crates/ail/tests/print_no_leak_pin.rs b/crates/ail/tests/print_no_leak_pin.rs new file mode 100644 index 0000000..138b4ea --- /dev/null +++ b/crates/ail/tests/print_no_leak_pin.rs @@ -0,0 +1,118 @@ +//! RED-pin for the 2026-05-14 rpe.1 Cat-A heap-Str leak in `print`. +//! +//! Property protected: under `--alloc=rc`, evaluating the trivial +//! program `(body (app print 42))` does NOT leak the heap-Str +//! allocated inside `print`'s body. `print`'s body is the explicit- +//! let `let s = show x in do io/print_str s` (see iter 24.3, pinned +//! structurally by `print_mono_body_shape.rs`); `show` at primitive +//! type calls `int_to_str` which returns a fresh heap-Str (slab in +//! `runtime/str.c`). `io/print_str` borrows the Str. At let-scope- +//! close, the binder `s` is the only owner — codegen must emit +//! `ailang_rc_dec(s)`. The runtime stats line at exit must report +//! `live == 0` (equivalently `allocs == frees`). +//! +//! As of commit 301cbc3 this test fails: stderr reports +//! `ailang_rc_stats: allocs=1 frees=0 live=1`. The IR for the post- +//! mono `ail_prelude_print__Int` body contains a `call ptr +//! @ail_prelude_show__Int(...)` followed by `getelementptr +8` + +//! `@puts` + `ret i8 0` — there is no `ailang_rc_dec` on the +//! show-result before return. Root cause sits at codegen's +//! `is_rc_heap_allocated` App-arm: it reads the callee's +//! `Type::Fn.ret_mode` via `synth_callee_ret_mode` and requires +//! `Own`. The mono'ed `show__Int` inherits its `Type::Fn` from the +//! `Show` class method declaration in `examples/prelude.ail.json`, +//! whose `methods[0].type` omits the `ret_mode` key — `serde` defaults +//! the missing field to `ParamMode::Implicit`. So +//! `synth_callee_ret_mode(show__Int) == Implicit`, the let-binder is +//! not flagged trackable, and `drop_symbol_for_binder` is never +//! called for it. +//! +//! Once the underlying issue is fixed (the canonical move is to +//! recognise that any class-method whose declared `ret` is `Str` / +//! any pointer-typed user type should round-trip as Own through +//! mono, OR to install the Show class declaration with explicit +//! `ret_mode: "own"`, OR to widen `is_rc_heap_allocated`'s App-arm +//! to also consult the callee's ret type-via-Str-carve-out), this +//! test must flip GREEN without weakening the assertion. + +use std::path::Path; +use std::process::Command; + +fn ail_bin() -> &'static str { + env!("CARGO_BIN_EXE_ail") +} + +#[test] +fn alloc_rc_print_int_does_not_leak_show_result_str() { + let manifest_dir = env!("CARGO_MANIFEST_DIR"); + let workspace = Path::new(manifest_dir).parent().unwrap().parent().unwrap(); + let src = workspace + .join("examples") + .join("print_int_no_leak_pin.ail"); + let tmp = std::env::temp_dir().join(format!( + "ailang_print_no_leak_pin_{}", + std::process::id() + )); + std::fs::create_dir_all(&tmp).unwrap(); + let out = tmp.join("bin"); + let status = Command::new(ail_bin()) + .args([ + "build", + src.to_str().unwrap(), + "--alloc=rc", + "-o", + ]) + .arg(&out) + .status() + .expect("ail build failed to run"); + assert!( + status.success(), + "ail build --alloc=rc failed for print_int_no_leak_pin.ail" + ); + let output = Command::new(&out) + .env("AILANG_RC_STATS", "1") + .output() + .expect("execute binary"); + assert!( + output.status.success(), + "binary exited non-zero: status {:?}", + output.status + ); + let stderr = String::from_utf8(output.stderr).expect("stderr utf8"); + let stats_line = stderr + .lines() + .find(|l| l.starts_with("ailang_rc_stats:")) + .unwrap_or_else(|| { + panic!( + "missing ailang_rc_stats line in stderr; stderr was:\n{stderr}" + ) + }); + let mut allocs: Option = None; + let mut frees: Option = None; + let mut live: Option = None; + for tok in stats_line.split_whitespace() { + if let Some(v) = tok.strip_prefix("allocs=") { + allocs = v.parse().ok(); + } else if let Some(v) = tok.strip_prefix("frees=") { + frees = v.parse().ok(); + } else if let Some(v) = tok.strip_prefix("live=") { + live = v.parse().ok(); + } + } + let allocs = allocs.expect("missing allocs= field"); + let frees = frees.expect("missing frees= field"); + let live = live.expect("missing live= field"); + assert_eq!( + live, 0, + "`(app print 42)` under --alloc=rc leaks {live} heap-Str cell(s) \ + (allocs={allocs} frees={frees}); print's let-binder for the \ + show-result must drop at let-scope-close. Show class method \ + `show` in prelude.ail.json omits `ret_mode`, so mono-synthesised \ + `show__Int` inherits ret_mode=Implicit and `is_rc_heap_allocated` \ + declines to track the let-binder." + ); + assert_eq!( + allocs, frees, + "alloc/free mismatch (allocs={allocs} frees={frees} live={live})" + ); +} diff --git a/crates/ailang-check/src/lib.rs b/crates/ailang-check/src/lib.rs index 3fee303..c5b6501 100644 --- a/crates/ailang-check/src/lib.rs +++ b/crates/ailang-check/src/lib.rs @@ -131,12 +131,22 @@ pub(crate) fn substitute_rigids(t: &Type, mapping: &BTreeMap) -> T name: name.clone(), args: args.iter().map(|a| substitute_rigids(a, mapping)).collect(), }, - Type::Fn { params, ret, effects, .. } => Type::Fn { + Type::Fn { params, param_modes, ret, ret_mode, effects } => Type::Fn { params: params.iter().map(|p| substitute_rigids(p, mapping)).collect(), ret: Box::new(substitute_rigids(ret, mapping)), effects: effects.clone(), - param_modes: vec![], - ret_mode: ParamMode::Implicit, + // 2026-05-14 bugfix-print-leak-show-ret-mode: preserve + // param_modes and ret_mode through rigid substitution. + // The earlier `..` discarded both and hard-reset to + // Implicit, which silently stripped the Show class + // method's `ret_mode: Own` (declared in + // `examples/prelude.ail.json`) when mono synthesised + // `show__Int` via `synthesise_mono_fn`'s + // `substitute_rigids(&class_method.ty, …)`. Codegen's + // `is_rc_heap_allocated` App-arm then declined to track + // print's `let s = show x` binder, leaking the heap-Str. + param_modes: param_modes.clone(), + ret_mode: *ret_mode, }, Type::Forall { vars, constraints, body } => { // Inner forall shadows: only substitute vars not re-bound here. diff --git a/docs/journals/2026-05-14-iter-bugfix-print-leak-show-ret-mode.md b/docs/journals/2026-05-14-iter-bugfix-print-leak-show-ret-mode.md new file mode 100644 index 0000000..5d183eb --- /dev/null +++ b/docs/journals/2026-05-14-iter-bugfix-print-leak-show-ret-mode.md @@ -0,0 +1,85 @@ +# iter bugfix-print-leak-show-ret-mode — print() leaks heap-Str under --alloc=rc; Show class method's ret_mode stripped twice + +**Date:** 2026-05-14 +**Started from:** 301cbc33a0b2fed18f465c6c17041211ff7da35e +**Status:** DONE +**Tasks completed:** 1 of 1 + +## Summary + +Two-layer bug: (i) the `Show` class method declaration in +`examples/prelude.ail.json` omitted the `ret_mode` key, so serde +defaulted it to `ParamMode::Implicit`; (ii) even with `ret_mode: own` +declared, `substitute_rigids` in `crates/ailang-check/src/lib.rs` +was hard-resetting `param_modes` to `vec![]` and `ret_mode` to +`Implicit` in its `Type::Fn` arm — silently stripping the mode +metadata during class-method mono synthesis (`show__Int`, `eq__T`, +`compare__T`, user-instance `eq__`, etc.). Codegen's +`is_rc_heap_allocated` App-arm requires the callee's `ret_mode == Own` +to mark the let-binder trackable, so print's `let s = show x` binder +was never `ailang_rc_dec`'d at let-scope-close. The fix declares +`ret_mode: "own"` on the Show class method (parallel to the +`int_to_str`/`bool_to_str`/`float_to_str`/`str_clone`/`str_concat` +builtins which already carry `ret_mode: Own`) and repairs +`substitute_rigids`'s `Type::Fn` arm to clone `param_modes` and +preserve `ret_mode` through rigid substitution. The Form-A sibling +`examples/prelude.ail` is regenerated via `ail render` to stay in +lock-step per spec §C4 (b). Ten pinned mono-body hashes +(`eq__Int/Bool/Str`, `compare__Int/Bool/Str`, `show__Int/Bool/Str/Float`, +plus `eq__IntBox`) are re-pinned to the new GREEN baselines because +the mono bodies now carry the previously-stripped `param_modes` / +`ret_mode` metadata; bodies are semantically identical. + +## Per-task notes + +- bugfix.1: applied the fix. + - `examples/prelude.ail.json`: Show class method now has + `"ret_mode": "own"`. + - `examples/prelude.ail`: regenerated via + `cargo run -p ail -- render examples/prelude.ail.json + > examples/prelude.ail`; round-trips parse-isomorphic. + - `crates/ailang-check/src/lib.rs` `substitute_rigids` Type::Fn + arm: pattern-match all five fields explicitly; clone + `param_modes` and pass `*ret_mode` through (`ParamMode: Copy`). + Comment cites the bugfix iter id and the cause. + - `crates/ail/tests/mono_hash_stability.rs`: re-pinned 10 hashes + across the Eq/Ord and Show test fns; both bodies now report + `captured: {h}` on assertion failure (the Eq/Ord arm was + missing the captured-suffix; small in-place tidy folded into + the re-pin since the assertion-message string was being + rewritten anyway). + - `crates/ail/tests/eq_ord_e2e.rs`: re-pinned `eq__IntBox` + body hash to `3c4cf040cb4e8bb2`. + +## Concerns + +The debugger's cause analysis (carrier) pointed only at the JSON +omission. The JSON-only fix leaves the test RED — `substitute_rigids` +was silently stripping modes too. Suggest: at the next /audit pass, +verify there are no *other* call sites that build a `Type::Fn` with +explicit `..` field-spread and end up dropping modes. The pattern +`Type::Fn { ..., .. }` is the smell. + +## Known debt + +None — the fix is local. The wider question of "should `Type::Fn`'s +mode fields be field-init-by-name everywhere to prevent this class +of bug at the compiler level" is out of scope for this minimal fix. + +## Files touched + +- `examples/prelude.ail.json` — Show class: added `ret_mode: own`. +- `examples/prelude.ail` — regenerated Form-A sibling. +- `crates/ailang-check/src/lib.rs` — `substitute_rigids` Type::Fn + arm: preserve `param_modes` + `ret_mode`. +- `crates/ail/tests/mono_hash_stability.rs` — re-pinned 10 hashes + (6 Eq/Ord + 4 Show); added cause comments. +- `crates/ail/tests/eq_ord_e2e.rs` — re-pinned `eq__IntBox` hash. +- `crates/ail/tests/print_no_leak_pin.rs` (from /debug) — RED-pin + for the leak; now GREEN. +- `examples/print_int_no_leak_pin.ail` (from /debug) — fixture for + the RED-pin; now GREEN. + +## Stats + +bench/orchestrator-stats/2026-05-14-iter-bugfix-print-leak-show-ret-mode.json diff --git a/examples/prelude.ail b/examples/prelude.ail index 320b26d..b374dda 100644 --- a/examples/prelude.ail +++ b/examples/prelude.ail @@ -55,7 +55,7 @@ (param a) (doc "Producer of a human-readable Str representation. Ships in milestone 24 with primitive instances for Int/Bool/Str/Float; user types declare their own instance.") (method show - (type (fn-type (params (borrow a)) (ret (con Str)))))) + (type (fn-type (params (borrow a)) (ret (own (con Str))))))) (instance (class Show) (type (con Int)) diff --git a/examples/prelude.ail.json b/examples/prelude.ail.json index 4e1e8d6..ce3e55a 100644 --- a/examples/prelude.ail.json +++ b/examples/prelude.ail.json @@ -232,6 +232,7 @@ "params": [{ "k": "var", "name": "a" }], "param_modes": ["borrow"], "ret": { "k": "con", "name": "Str" }, + "ret_mode": "own", "effects": [] } } diff --git a/examples/print_int_no_leak_pin.ail b/examples/print_int_no_leak_pin.ail new file mode 100644 index 0000000..3ac2a31 --- /dev/null +++ b/examples/print_int_no_leak_pin.ail @@ -0,0 +1,6 @@ +(module print_int_no_leak_pin + (fn main + (doc "RED-pin fixture for the 2026-05-14 rpe.1 Cat-A heap-Str leak. Under --alloc=rc, the prelude `print` function's body `let s = (app show x) in (do io/print_str s)` allocates a heap-Str via show, then passes it (borrow) to io/print_str. The let-binder `s` is `ret_mode: Own` for `show __Int`, so codegen should emit ailang_rc_dec(s) at let-scope-close. As of commit 301cbc3 the slab leaks: AILANG_RC_STATS=1 reports `allocs=1 frees=0 live=1` for the trivial `(body (app print 42))` program. Expected post-fix: `allocs == frees && live == 0`.") + (type (fn-type (params) (ret (con Unit)) (effects IO))) + (params) + (body (app print 42))))