From d72fe0c2e61e44026f506e6bc9a8f1ad9a68ce8d Mon Sep 17 00:00:00 2001 From: Brummel Date: Tue, 2 Jun 2026 15:47:01 +0200 Subject: [PATCH] fix(codegen): drop branch-consume-split owned params (#63 leg 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last leg of the #63 drop-soundness cluster: an (own ...) heap param consumed on one match-arm / if-branch but live on a sibling was never dropped on the live path, leaking one slab per call. This was the residual live=2 leak in series_sma that kept the series headline from being leak-clean. Root: the per-fn aggregate consume_count (the worst-case max over all branches from uniqueness::merge_states) was codegen's only consume signal, and every owned-param drop site gated on it. A param consumed on SOME branch makes the aggregate >= 1, so every site skipped it — including the branch where it stays live. match and if share this root (if is first-class MTerm::If, not desugared to match). Mechanism (spec 0068): - CAPTURE: uniqueness retains the per-branch consume snapshots it already computes per branch and discarded at the max merge — new BranchConsume + infer_module_with_cross_branches (the aggregate-only infer_module_with_cross now delegates and drops the channel). - CARRY: additive MArm.consume / MTerm::If.{then,else}_consume, attached by lower_to_mir via a traversal-order cursor (post-order pop, kind- and exhaustion-asserts make a desync a loud panic; neutral for const bodies which uniqueness does not walk). The AST carries no node id, so the correspondence is the structural pre-order both walks share — pinned by branch_consume_maps_attach_to_matching_arms. - GATE: emit_leakclass_branch_param_drops fires a fall-through drop only for the leak class (branch_consume==0 AND aggregate>=1), in match arms + both if branches; the pre-tail-call dec switches its gate source from the aggregate to per-arm. The fn-return dec and arm-close pattern-binder dec are UNCHANGED. Safety: - Double-free: the drop sites partition by aggregate (==0 -> existing fn-return/pre-tail-call; >=1 -> new per-branch). Disjoint, so no param is dropped twice — no fn-return disable, no tail-position analysis. - Use-after-free: the checker rejects use-after-consume, so an aggregate>=1 param is provably dead past the construct (path-terminal drop, tail position irrelevant). - Type eligibility (found by the full-suite gate during implement; spec 0068 refined): the new agg>=1 site is the FIRST drop site that can reach a STATIC closure-pair param (a top-level fn ref like inc in Either.either, consumed on one arm, live on the other) — a .rodata constant with no rc-header. field_drop_call routes Type::Fn / static-Str / Type::Var to the bare ailang_rc_dec, which underflowed on it. The helper now drops only params with a real per-type heap-ADT drop fn (field_drop_call != "ailang_rc_dec"). Sound (no underflow) and leak-correct (a static closure/Str allocates nothing). Witnessed green by std_either_demo / std_list_demo / poly_rec_capture_demo. Verification: full workspace suite green (116 binaries); both new leak pins (if + match) and series_sma_no_leak_pin green at live=0; legs 1/2 pins still green; INTERCEPTS<->(intrinsic) bijection intact; no Pattern::Lit reject path added. The leg-3 helper's type precondition was applied inline by the orchestrator (a narrowing guard clause in an already-reviewed Task-4 helper, full context loaded) after the implement-orchestrator correctly surfaced the spec gap rather than papering over the regression. This clears the series_sma leak tail; the series milestone (#61) close stays a separate deliberate step (its end-to-end milestone fieldtest). closes #63 --- .../if_branch_consume_split_no_leak_pin.rs | 91 +++++++++++++ .../match_arm_consume_split_no_leak_pin.rs | 10 -- crates/ail/tests/series_sma_no_leak_pin.rs | 78 ++++++++++++ crates/ailang-check/src/lower_to_mir.rs | 120 ++++++++++++++++-- crates/ailang-check/src/uniqueness.rs | 84 +++++++++++- crates/ailang-check/tests/lower_to_mir_ty.rs | 27 ++++ crates/ailang-codegen/src/escape.rs | 3 + crates/ailang-codegen/src/lib.rs | 97 +++++++++++++- crates/ailang-codegen/src/match_lower.rs | 26 +++- crates/ailang-mir/src/lib.rs | 16 ++- docs/plans/0126-branch-consume-split-drop.md | 22 +++- docs/specs/0068-branch-consume-split-drop.md | 25 ++++ .../if_branch_consume_split_no_leak_pin.ail | 21 +++ 13 files changed, 585 insertions(+), 35 deletions(-) create mode 100644 crates/ail/tests/if_branch_consume_split_no_leak_pin.rs create mode 100644 crates/ail/tests/series_sma_no_leak_pin.rs create mode 100644 examples/if_branch_consume_split_no_leak_pin.ail diff --git a/crates/ail/tests/if_branch_consume_split_no_leak_pin.rs b/crates/ail/tests/if_branch_consume_split_no_leak_pin.rs new file mode 100644 index 0000000..9907907 --- /dev/null +++ b/crates/ail/tests/if_branch_consume_split_no_leak_pin.rs @@ -0,0 +1,91 @@ +//! RED-pin for leg 3 of bug #63 — the per-branch-vs-per-fn consume- +//! accounting leak at an `if`-branch fall-through (the `if` half of the +//! branch-consume-split class; the `match` half is +//! match_arm_consume_split_no_leak_pin.rs). +//! +//! Property protected: under `--alloc=rc`, when an `(own ...)` heap +//! param is consumed on ONE `if` branch but is LIVE on the sibling +//! branch, it MUST be dropped on the live branch before the fn returns. +//! `run_if b 0` takes the `then` branch (`ge 0 0` is true) where `b` is +//! live; the `else` branch (`sink b`) consumes it. As of HEAD this fails: +//! the binary reports `live=1` because the per-fn aggregate consume of +//! `b` is `>= 1` (the `else` branch consumes it) so the fn-return dec +//! skips `b` and nothing drops it on the `then` path. +//! +//! `if` is a first-class `MTerm::If` (it does NOT desugar to `match`), +//! so it needs the per-branch drop in its own codegen node. Goes green +//! with the #63-leg-3 fix. + +use std::path::Path; +use std::process::Command; + +fn ail_bin() -> &'static str { + env!("CARGO_BIN_EXE_ail") +} + +#[test] +fn alloc_rc_owned_param_consumed_on_one_if_branch_live_on_sibling_does_not_leak() { + let manifest_dir = env!("CARGO_MANIFEST_DIR"); + let workspace = Path::new(manifest_dir).parent().unwrap().parent().unwrap(); + let src = workspace + .join("examples") + .join("if_branch_consume_split_no_leak_pin.ail"); + let tmp = std::env::temp_dir().join(format!( + "ailang_if_branch_consume_split_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 if_branch_consume_split_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, + "an (own) heap param consumed on one if branch but live on the \ + sibling branch leaks {live} slab(s) (allocs={allocs} frees={frees}): \ + the per-branch live param is never dropped because the fn-return \ + dec gates on the per-fn aggregate consume (>= 1 from the consuming \ + branch) instead of the per-branch consume." + ); + assert_eq!( + allocs, frees, + "alloc/free mismatch (allocs={allocs} frees={frees} live={live})" + ); +} diff --git a/crates/ail/tests/match_arm_consume_split_no_leak_pin.rs b/crates/ail/tests/match_arm_consume_split_no_leak_pin.rs index d6ed81d..02c43fa 100644 --- a/crates/ail/tests/match_arm_consume_split_no_leak_pin.rs +++ b/crates/ail/tests/match_arm_consume_split_no_leak_pin.rs @@ -62,16 +62,6 @@ fn ail_bin() -> &'static str { env!("CARGO_BIN_EXE_ail") } -// IGNORED: leg 3 of #63 is a design fork, not a contained fix. Per-arm -// consume liveness is destroyed by the worst-case `max` merge in -// uniqueness.rs (`merge_states`) before MIR, so codegen has no per-arm -// information to gate a fall-through drop on. Closing this leg needs a -// memory-model change (retain per-arm consume, thread it to codegen, -// emit the drop only on the live arm — never at the join or the -// consuming arm) and a fresh brainstorm cycle. Un-ignore when that -// ships. The assertion below is a correct symptom pin (live == 0); only -// the fix mechanism is undecided. -#[ignore = "leg 3 of #63 — design fork (per-arm consume dataflow); un-ignore when fixed"] #[test] fn alloc_rc_owned_param_consumed_on_one_arm_live_on_sibling_does_not_leak() { let manifest_dir = env!("CARGO_MANIFEST_DIR"); diff --git a/crates/ail/tests/series_sma_no_leak_pin.rs b/crates/ail/tests/series_sma_no_leak_pin.rs new file mode 100644 index 0000000..5a52ba1 --- /dev/null +++ b/crates/ail/tests/series_sma_no_leak_pin.rs @@ -0,0 +1,78 @@ +//! Pins that the #61 headline `series_sma` example is leak-clean under +//! `--alloc=rc`. The residual `live=2` was #63 leg 3 (branch-consume- +//! split); this pin closes that leak tail. +//! +//! Property protected: `series_sma` runs to completion under +//! `AILANG_RC_STATS=1` with `live == 0` (equivalently `allocs == frees`). +//! `series_sma_pin.rs` already pins stdout; this pin is the RC ledger, +//! distinct. Goes green once the branch-consume-split leak is fixed. + +use std::path::Path; +use std::process::Command; + +fn ail_bin() -> &'static str { + env!("CARGO_BIN_EXE_ail") +} + +#[test] +fn series_sma_is_leak_clean() { + let manifest_dir = env!("CARGO_MANIFEST_DIR"); + let workspace = Path::new(manifest_dir).parent().unwrap().parent().unwrap(); + let src = workspace.join("examples").join("series_sma.ail"); + let tmp = std::env::temp_dir().join(format!( + "ailang_series_sma_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 series_sma.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, + "series_sma leaks {live} slab(s) (allocs={allocs} frees={frees}): the \ + #61 residual live=2 was the #63 leg-3 branch-consume-split leak; if \ + this is non-zero the leak-class drop is not firing on the series path." + ); + assert_eq!( + allocs, frees, + "alloc/free mismatch (allocs={allocs} frees={frees} live={live})" + ); +} diff --git a/crates/ailang-check/src/lower_to_mir.rs b/crates/ailang-check/src/lower_to_mir.rs index fd11951..07d0414 100644 --- a/crates/ailang-check/src/lower_to_mir.rs +++ b/crates/ailang-check/src/lower_to_mir.rs @@ -23,6 +23,76 @@ struct Ctx<'a> { in_def: &'a str, locals: IndexMap, loop_stack: Vec>, + /// #63 leg 3: per-branch consume snapshots for the current def, in the + /// uniqueness pass's pre-order. Consumed by a traversal-order cursor + /// as `lower_term` lowers each `if`/`match` (post-order pop). Empty + + /// `track_branches=false` for const bodies (uniqueness walks only fns). + branch_consumes: Vec, + branch_idx: usize, + track_branches: bool, +} + +impl Ctx<'_> { + /// Pop the next `If` branch-consume entry (post-order: called AFTER + /// lowering cond/then/else). Returns empty maps for const bodies. A + /// kind/cursor mismatch is a compiler-internal desync — panic loudly + /// rather than silently mis-attach a drop map. + fn next_branch_if( + &mut self, + ) -> (std::collections::BTreeMap, std::collections::BTreeMap) { + use crate::uniqueness::BranchConsume; + if !self.track_branches { + return (std::collections::BTreeMap::new(), std::collections::BTreeMap::new()); + } + match self.branch_consumes.get(self.branch_idx) { + Some(BranchConsume::If { then, else_ }) => { + let r = (then.clone(), else_.clone()); + self.branch_idx += 1; + r + } + other => panic!( + "lower_to_mir branch-consume desync in `{}` at idx {}: expected If, got {}", + self.in_def, + self.branch_idx, + match other { + Some(BranchConsume::Match { .. }) => "Match", + Some(BranchConsume::If { .. }) => "If", + None => "end-of-list", + } + ), + } + } + + /// Pop the next `Match` branch-consume entry (post-order: called AFTER + /// lowering scrutinee + all arm bodies). Returns `n_arms` empty maps + /// for const bodies. Arm-count / kind mismatch is a desync — panic. + fn next_branch_match( + &mut self, + n_arms: usize, + ) -> Vec> { + use crate::uniqueness::BranchConsume; + if !self.track_branches { + return vec![std::collections::BTreeMap::new(); n_arms]; + } + match self.branch_consumes.get(self.branch_idx) { + Some(BranchConsume::Match { arms }) if arms.len() == n_arms => { + let r = arms.clone(); + self.branch_idx += 1; + r + } + other => panic!( + "lower_to_mir branch-consume desync in `{}` at idx {}: expected Match[{}], got {}", + self.in_def, + self.branch_idx, + n_arms, + match other { + Some(BranchConsume::Match { arms }) => format!("Match[{}]", arms.len()), + Some(BranchConsume::If { .. }) => "If".into(), + None => "end-of-list".into(), + } + ), + } + } } impl<'a> Ctx<'a> { @@ -323,12 +393,20 @@ fn lower_term(ctx: &mut Ctx, t: &Term) -> Result { } } - Term::If { cond, then, else_ } => MTerm::If { - cond: Box::new(lower_term(ctx, cond)?), - then: Box::new(lower_term(ctx, then)?), - else_: Box::new(lower_term(ctx, else_)?), - ty, - }, + Term::If { cond, then, else_ } => { + let m_cond = lower_term(ctx, cond)?; + let m_then = lower_term(ctx, then)?; + let m_else = lower_term(ctx, else_)?; + let (then_consume, else_consume) = ctx.next_branch_if(); + MTerm::If { + cond: Box::new(m_cond), + then: Box::new(m_then), + else_: Box::new(m_else), + then_consume, + else_consume, + ty, + } + } Term::Do { op, args, tail } => { let m_args = args @@ -378,7 +456,15 @@ fn lower_term(ctx: &mut Ctx, t: &Term) -> Result { } } } - m_arms.push(MArm { pat: a.pat.clone(), body: m_body }); + m_arms.push(MArm { + pat: a.pat.clone(), + body: m_body, + consume: std::collections::BTreeMap::new(), + }); + } + let arm_maps = ctx.next_branch_match(arms.len()); + for (marm, cmap) in m_arms.iter_mut().zip(arm_maps) { + marm.consume = cmap; } MTerm::Match { scrutinee: m_scrut, arms: m_arms, ty } } @@ -599,8 +685,8 @@ pub fn lower_module( // re-running `infer_module_with_cross`. Same pass, same post-mono // input as codegen used — a pure relocation, so drop placement is // unchanged. - let uniqueness = - crate::uniqueness::infer_module_with_cross(module, cross_module_types); + let result = + crate::uniqueness::infer_module_with_cross_branches(module, cross_module_types); let mut defs = Vec::new(); let mut consts = Vec::new(); for def in &module.defs { @@ -614,6 +700,9 @@ pub fn lower_module( in_def: &c.name, locals: IndexMap::new(), loop_stack: Vec::new(), + branch_consumes: Vec::new(), + branch_idx: 0, + track_branches: false, }; let body = lower_term(&mut ctx, &c.value)?; consts.push(ailang_mir::MirConst { @@ -657,14 +746,25 @@ pub fn lower_module( in_def: &f.name, locals, loop_stack: Vec::new(), + branch_consumes: result.branches.get(&f.name).cloned().unwrap_or_default(), + branch_idx: 0, + track_branches: true, }; let body = lower_term(&mut ctx, &f.body)?; + assert!( + ctx.branch_idx == ctx.branch_consumes.len(), + "lower_to_mir branch-consume cursor desync for `{}`: consumed {} of {} entries", + f.name, + ctx.branch_idx, + ctx.branch_consumes.len() + ); defs.push(MirDef { name: f.name.clone(), sig: f.ty.clone(), params: f.params.clone(), body, - consume: uniqueness + consume: result + .table .iter() .filter(|((d, _), _)| d == &f.name) .map(|((_, b), info)| (b.clone(), info.consume_count)) diff --git a/crates/ailang-check/src/uniqueness.rs b/crates/ailang-check/src/uniqueness.rs index 6e934e2..b48f081 100644 --- a/crates/ailang-check/src/uniqueness.rs +++ b/crates/ailang-check/src/uniqueness.rs @@ -99,6 +99,14 @@ pub struct UniquenessInfo { /// they mean — there is no shadow collapse. pub type UniquenessTable = BTreeMap<(String, String), UniquenessInfo>; +/// The full uniqueness output: the per-`(def, binder)` aggregate table +/// (codegen's classification + scope-close consume) AND the per-fn +/// ordered list of per-branch consume snapshots (#63 leg 3). +pub struct UniquenessResult { + pub table: UniquenessTable, + pub branches: BTreeMap>, +} + /// Top-level entry: walk every fn def in `m` and produce per-binder /// classifications keyed by `(fn_name, binder_name)`. /// @@ -130,10 +138,10 @@ pub fn infer_module(m: &Module) -> UniquenessTable { /// RawBuf — it removes a latent over-conservative consume default. /// The same-module `globals` lookup stays the first resort; the /// cross-module table is consulted only on a miss. -pub fn infer_module_with_cross( +pub fn infer_module_with_cross_branches( m: &Module, cross_module_types: &BTreeMap>, -) -> UniquenessTable { +) -> UniquenessResult { let mut globals: BTreeMap = BTreeMap::new(); // register builtin signatures so the App-arg walker // sees their `param_modes` and walks `Borrow`-mode args as @@ -168,12 +176,23 @@ pub fn infer_module_with_cross( } let mut table: UniquenessTable = BTreeMap::new(); + let mut branches: BTreeMap> = BTreeMap::new(); for def in &m.defs { if let Def::Fn(f) = def { - infer_fn(f, &globals, cross_module_types, &mut table); + infer_fn(f, &globals, cross_module_types, &mut table, &mut branches); } } - table + UniquenessResult { table, branches } +} + +/// Aggregate-only convenience used by existing callers (codegen tests, +/// etc.). Delegates to [`infer_module_with_cross_branches`] and drops the +/// per-branch channel. +pub fn infer_module_with_cross( + m: &Module, + cross_module_types: &BTreeMap>, +) -> UniquenessTable { + infer_module_with_cross_branches(m, cross_module_types).table } /// Per-fn driver: install the parameters as binders, walk the body, @@ -183,19 +202,21 @@ fn infer_fn( globals: &BTreeMap, cross_module_types: &BTreeMap>, out: &mut UniquenessTable, + branches_out: &mut BTreeMap>, ) { let inner = strip_forall(&f.ty); if !matches!(inner, Type::Fn { .. }) { return; } - let param_states = { + let (param_states, branch_consumes) = { let mut walker = Walker { globals, cross_module_types, binders: BTreeMap::new(), def_name: f.name.clone(), out, + branch_consumes: Vec::new(), }; // Install fn parameters. Their consume counter starts at 0; @@ -207,10 +228,12 @@ fn infer_fn( walker.walk(&f.body, Position::Consume); // Snapshot params before dropping the walker. - f.params + let ps = f + .params .iter() .filter_map(|name| walker.binders.get(name).map(|s| (name.clone(), s.clone()))) - .collect::>() + .collect::>(); + (ps, walker.branch_consumes) }; // Walker dropped — `out` is borrowable again. @@ -221,6 +244,7 @@ fn infer_fn( }; out.insert((f.name.clone(), name), info); } + branches_out.insert(f.name.clone(), branch_consumes); } #[inline] @@ -247,6 +271,32 @@ struct BinderState { consume_count: u32, } +/// Per-branch consume snapshots retained for codegen's leak-class drop +/// siting (#63 leg 3). Each map is the absolute per-binder +/// `consume_count` at a branch's end-state — the value the `max` merge +/// in `merge_states` would otherwise collapse. The pass emits one entry +/// per `if`/`match` construct in PRE-ORDER of branch constructs (the +/// same order `lower_to_mir` lowers them), so `lower_to_mir` correlates +/// entries to MIR branch nodes by a traversal-order cursor (the AST +/// carries no node id). +#[derive(Debug, Clone)] +pub enum BranchConsume { + If { + then: BTreeMap, + else_: BTreeMap, + }, + Match { + arms: Vec>, + }, +} + +fn snapshot_consume(binders: &BTreeMap) -> BTreeMap { + binders + .iter() + .map(|(k, v)| (k.clone(), v.consume_count)) + .collect() +} + struct Walker<'a> { globals: &'a BTreeMap, /// Workspace-flat `module -> def -> Type` table. Consulted only @@ -256,6 +306,9 @@ struct Walker<'a> { binders: BTreeMap, def_name: String, out: &'a mut UniquenessTable, + /// Per-branch consume snapshots, accumulated in pre-order during the + /// walk (#63 leg 3). Drained per fn by `infer_fn`. + branch_consumes: Vec, } impl<'a> Walker<'a> { @@ -300,20 +353,37 @@ impl<'a> Walker<'a> { self.walk(then, pos); let after_then = std::mem::replace(&mut self.binders, saved); self.walk(else_, pos); + // #63 leg 3: retain both branch end-states (pre-order push + // AFTER walking both children, so nested branches inside + // then/else are already pushed — matches lower_to_mir's + // post-order pop). + self.branch_consumes.push(BranchConsume::If { + then: snapshot_consume(&after_then), + else_: snapshot_consume(&self.binders), + }); merge_states(&mut self.binders, &after_then); } Term::Match { scrutinee, arms } => { self.walk(scrutinee, Position::Borrow); let saved = self.binders.clone(); let mut acc: Option> = None; + let mut arm_maps: Vec> = + Vec::with_capacity(arms.len()); for arm in arms { self.binders = saved.clone(); self.walk_arm(arm, pos); + // #63 leg 3: snapshot this arm's end-state (pattern + // binders already popped by walk_arm) before the merge. + arm_maps.push(snapshot_consume(&self.binders)); match acc.as_mut() { None => acc = Some(self.binders.clone()), Some(m) => merge_states(m, &self.binders), } } + // pre-order push AFTER all arms (nested arm-body branches + // already pushed) — matches lower_to_mir's post-order pop. + self.branch_consumes + .push(BranchConsume::Match { arms: arm_maps }); if let Some(merged) = acc { self.binders = merged; } else { diff --git a/crates/ailang-check/tests/lower_to_mir_ty.rs b/crates/ailang-check/tests/lower_to_mir_ty.rs index 0f4a0b5..95ed363 100644 --- a/crates/ailang-check/tests/lower_to_mir_ty.rs +++ b/crates/ailang-check/tests/lower_to_mir_ty.rs @@ -318,3 +318,30 @@ fn find_str_node_with_rep(t: &MTerm, want: StrRep) -> bool { _ => false, } } + +/// #63 leg 3 lock-step: the per-arm consume map attaches to the matching +/// MArm in traversal order. In the `match_arm_consume_split_no_leak_pin` +/// fixture's `run`, `b` is consumed on the `On` arm (forwarded into +/// `sink`) and live on the `Off` arm — so traversal-order attachment must +/// land MArm.consume[b] == 0 on `Off` (source-order arms[0]) and >= 1 on +/// `On` (arms[1]). A cursor desync would mis-attach and flip these. +#[test] +fn branch_consume_maps_attach_to_matching_arms() { + let m = "match_arm_consume_split_no_leak_pin"; + let mir = elaborate_fixture(m); + let arms = match body(&mir, m, "run") { + MTerm::Match { arms, .. } => arms, + other => panic!("run body is not a Match: {other:?}"), + }; + assert_eq!( + arms[0].consume.get("b").copied().unwrap_or(99), + 0, + "Off arm (arms[0]) must leave `b` live (consume 0); got {:?}", + arms[0].consume.get("b") + ); + assert!( + arms[1].consume.get("b").copied().unwrap_or(0) >= 1, + "On arm (arms[1]) must consume `b` (>= 1); got {:?}", + arms[1].consume.get("b") + ); +} diff --git a/crates/ailang-codegen/src/escape.rs b/crates/ailang-codegen/src/escape.rs index 24a8337..cb302d9 100644 --- a/crates/ailang-codegen/src/escape.rs +++ b/crates/ailang-codegen/src/escape.rs @@ -612,6 +612,7 @@ mod tests { arms: vec![MArm { pat: Pattern::Wild, body: lit_int(0), + consume: std::collections::BTreeMap::new(), }], ty: Type::int(), }), @@ -743,6 +744,7 @@ mod tests { ], }, body: var("h"), + consume: std::collections::BTreeMap::new(), }, MArm { pat: Pattern::Ctor { @@ -750,6 +752,7 @@ mod tests { fields: vec![], }, body: lit_int(0), + consume: std::collections::BTreeMap::new(), }, ], ty: Type::int(), diff --git a/crates/ailang-codegen/src/lib.rs b/crates/ailang-codegen/src/lib.rs index 40e6a8c..e961c17 100644 --- a/crates/ailang-codegen/src/lib.rs +++ b/crates/ailang-codegen/src/lib.rs @@ -1591,6 +1591,99 @@ impl<'a> Emitter<'a> { Ok(()) } + /// #63 leg 3: drop owned heap params that are LIVE on this branch + /// (`branch_consume[p] == 0`) but CONSUMED on a sibling branch (per-fn + /// aggregate `self.consume[p] >= 1`) — the leak class. The + /// `aggregate >= 1` guard keeps this disjoint from the fn-return dec + /// (which owns the `aggregate == 0` class), so no param is double- + /// dropped — no fn-return disable / tail-position analysis is needed. + /// And because the checker rejects use-after-consume, an + /// `aggregate >= 1` param is provably dead past the branch construct, + /// so dropping it at the branch close is safe regardless of tail + /// position. `tail_val` is the branch's own tail SSA (a param returned + /// by the branch transfers ownership to the join consumer — skip it); + /// `scrutinee` is the match scrutinee binder to skip (None for `if`). + fn emit_leakclass_branch_param_drops( + &mut self, + branch_consume: &std::collections::BTreeMap, + tail_val: &str, + scrutinee: Option<&str>, + ) { + if !matches!(self.alloc, AllocStrategy::Rc) { + return; + } + let owned_params: Vec<(String, ParamMode)> = self + .current_param_modes + .iter() + .map(|(n, m)| (n.clone(), *m)) + .collect(); + for (pname, mode) in &owned_params { + if !matches!(mode, ParamMode::Own) { + continue; + } + if scrutinee == Some(pname.as_str()) { + continue; + } + let branch_cc = branch_consume.get(pname).copied().unwrap_or(u32::MAX); + if branch_cc != 0 { + continue; // consumed on this branch — ownership transferred + } + let agg = self + .consume + .get(&(self.current_def.clone(), pname.clone())) + .copied() + .unwrap_or(0); + if agg == 0 { + continue; // live on every path — the fn-return dec owns this + } + let p_ssa = format!("%arg_{}", pname); + if p_ssa == tail_val { + continue; // param is this branch's return value + } + let p_ail = self + .locals + .iter() + .find(|(_, ssa, _, _)| ssa == &p_ssa) + .map(|(_, _, lty, ail)| (lty.clone(), ail.clone())); + let (p_lty, p_ail) = match p_ail { + Some(t) => t, + None => continue, + }; + if p_lty != "ptr" { + continue; + } + // #63 leg 3 TYPE precondition: only drop params with a real + // per-type heap-ADT drop fn. `field_drop_call` routes + // `Type::Fn` (static OR heap closures), static-`Str`, and + // `Type::Var` to the bare `ailang_rc_dec`, which reads a + // non-existent rc-header on a `.rodata` static closure/Str and + // underflows. This `agg >= 1` site is the FIRST drop site that + // can reach a static closure param (a called closure has + // `agg >= 1`; every existing site needs `agg == 0`), so it must + // gate on a real drop fn. Sound + leak-correct: a static + // closure / static `Str` allocates nothing to drop. + let drop_call = self.field_drop_call(&p_ail); + if drop_call == "ailang_rc_dec" { + continue; + } + let moves = self.moved_slots.get(pname).cloned().unwrap_or_default(); + if moves.is_empty() { + self.body + .push_str(&format!(" call void @{drop_call}(ptr {p_ssa})\n")); + } else { + let sym = self.partial_drop_symbol_for_type(&p_ail); + let mask = Self::build_moved_mask(&moves); + if let (Some(sym), Some(mask)) = (sym, mask) { + self.body + .push_str(&format!(" call void @{sym}(ptr {p_ssa}, i64 {mask})\n")); + } else { + self.body + .push_str(&format!(" call void @ailang_rc_dec(ptr {p_ssa})\n")); + } + } + } + } + /// closure-pair scaffold for a top-level fn. Always emitted /// (one wrapper per fn), so cross-module references just use the /// `__clos` symbol without coordination. @@ -1903,7 +1996,7 @@ impl<'a> Emitter<'a> { r } - MTerm::If { cond, then, else_, .. } => { + MTerm::If { cond, then, else_, then_consume, else_consume, .. } => { let (cond_v, cond_ty) = self.lower_term(cond)?; if cond_ty != "i1" { return Err(CodegenError::Internal(format!( @@ -1926,6 +2019,7 @@ impl<'a> Emitter<'a> { let then_terminated = self.block_terminated; let then_block_end = self.current_block.clone(); if !then_terminated { + self.emit_leakclass_branch_param_drops(then_consume, &then_v, None); self.body.push_str(&format!(" br label %{join_lbl}\n")); } @@ -1939,6 +2033,7 @@ impl<'a> Emitter<'a> { ))); } if !else_terminated { + self.emit_leakclass_branch_param_drops(else_consume, &else_v, None); self.body.push_str(&format!(" br label %{join_lbl}\n")); } diff --git a/crates/ailang-codegen/src/match_lower.rs b/crates/ailang-codegen/src/match_lower.rs index aae6c6e..e93d8f6 100644 --- a/crates/ailang-codegen/src/match_lower.rs +++ b/crates/ailang-codegen/src/match_lower.rs @@ -820,11 +820,7 @@ impl<'a> Emitter<'a> { if p_lty != "ptr" { continue; } - let consume_count = self - .consume - .get(&(self.current_def.clone(), pname.clone())) - .copied() - .unwrap_or(u32::MAX); + let consume_count = arm.consume.get(pname).copied().unwrap_or(u32::MAX); if consume_count != 0 { continue; } @@ -963,6 +959,18 @@ impl<'a> Emitter<'a> { } } } + // #63 leg 3: a fall-through (non-tail-call) arm that left an + // owned param live (arm.consume == 0) but consumed it on a + // sibling arm (aggregate >= 1) must drop it here, before the + // br to the join. The aggregate >= 1 guard inside the helper + // keeps this disjoint from the fn-return dec. + if !self.block_terminated { + self.emit_leakclass_branch_param_drops( + &arm.consume, + &val, + scrutinee_binder.as_deref(), + ); + } // pop bindings for _ in 0..pushed { self.locals.pop(); @@ -1007,6 +1015,14 @@ impl<'a> Emitter<'a> { for _ in 0..pushed { self.locals.pop(); } + // #63 leg 3: same leak-class drop for a Wild/Var default arm. + if !self.block_terminated { + self.emit_leakclass_branch_param_drops( + &arm.consume, + &val, + scrutinee_binder.as_deref(), + ); + } if !self.block_terminated { phi_inputs.push((val, self.current_block.clone())); self.body diff --git a/crates/ailang-mir/src/lib.rs b/crates/ailang-mir/src/lib.rs index 1ddc291..5522c1a 100644 --- a/crates/ailang-mir/src/lib.rs +++ b/crates/ailang-mir/src/lib.rs @@ -88,7 +88,18 @@ pub enum MTerm { in_term: Box, ty: Type, }, - If { cond: Box, then: Box, else_: Box, ty: Type }, + If { + cond: Box, + then: Box, + else_: Box, + /// #63 leg 3: per-branch consume of each owned binder on the + /// `then` / `else` branch (absolute count at the branch end). + /// Codegen drops a param live here (`0`) but consumed on the + /// sibling (`MirDef.consume >= 1`). Empty for const bodies. + then_consume: BTreeMap, + else_consume: BTreeMap, + ty: Type, + }, Do { op: String, args: Vec, tail: bool, ty: Type }, Ctor { type_name: String, ctor: String, args: Vec, ty: Type }, Match { scrutinee: Box, arms: Vec, ty: Type }, @@ -116,6 +127,9 @@ pub enum MTerm { pub struct MArm { pub pat: Pattern, pub body: MTerm, + /// #63 leg 3: per-arm consume of each owned binder (absolute count at + /// this arm's end). See `MTerm::If.then_consume`. Empty for const bodies. + pub consume: BTreeMap, } #[derive(Debug, Clone)] diff --git a/docs/plans/0126-branch-consume-split-drop.md b/docs/plans/0126-branch-consume-split-drop.md index aaeb9ab..7f63272 100644 --- a/docs/plans/0126-branch-consume-split-drop.md +++ b/docs/plans/0126-branch-consume-split-drop.md @@ -810,9 +810,22 @@ Add this method to the codegen impl in `crates/ailang-codegen/src/lib.rs` if p_lty != "ptr" { continue; } + // #63 leg 3 TYPE precondition: only drop params with a real + // per-type heap-ADT drop fn. `field_drop_call` routes + // `Type::Fn` (static OR heap closures), static-`Str`, and + // `Type::Var` to the bare `ailang_rc_dec`, which reads a + // non-existent rc-header on a `.rodata` static closure/Str + // and underflows. This `agg >= 1` site is the FIRST drop site + // that can reach a static closure param (a called closure has + // `agg >= 1`; every existing site needs `agg == 0`), so it + // must gate on a real drop fn. Sound + leak-correct: a static + // closure / static `Str` allocates nothing to drop. + let drop_call = self.field_drop_call(&p_ail); + if drop_call == "ailang_rc_dec" { + continue; + } let moves = self.moved_slots.get(pname).cloned().unwrap_or_default(); if moves.is_empty() { - let drop_call = self.field_drop_call(&p_ail); self.body .push_str(&format!(" call void @{drop_call}(ptr {p_ssa})\n")); } else { @@ -970,6 +983,13 @@ Expected: ALL PASS. (The typed-MIR re-synth strictness family means a MIR-node change can ripple — this is the gate that catches a regression the targeted pins miss, including any double-free/leak from the new drop interacting with an existing branch-tailed fn in prelude/kernel/examples.) +In particular the `std_either_demo`, `std_list_demo`, and +`poly_rec_capture_demo` e2e tests are the **closure-param regression +witnesses**: they pass top-level fn refs (static closure-pairs) through a +`match` that consumes them on one arm and leaves them live on another — +exactly the leak class. Without the helper's `field_drop_call == +"ailang_rc_dec"` type precondition (Step 1) they abort with +`ailang_rc_dec: refcount underflow`; with it they stay green. - [ ] **Step 4: Confirm no lockstep-pair drift** diff --git a/docs/specs/0068-branch-consume-split-drop.md b/docs/specs/0068-branch-consume-split-drop.md index 73bb717..00d11c8 100644 --- a/docs/specs/0068-branch-consume-split-drop.md +++ b/docs/specs/0068-branch-consume-split-drop.md @@ -119,6 +119,31 @@ Two independent guarantees make this safe: does double duty: it secures both disjointness and the no-after-use property. +### Type eligibility — the per-branch drop fires only for heap-RC ADTs + +The new fall-through drop is the *first* drop site that can reach an +`aggregate >= 1` owned param — and an `aggregate >= 1` closure param is +routinely a *static* closure-pair (a top-level fn ref like `inc` lowered +to a `.rodata` constant with no rc-header), not a heap RC slab. The +existing drop sites never hit this because they all require +`aggregate == 0`, and a *called* closure has `aggregate >= 1`; their +soundness for `Type::Fn` / static-`Str` params rests on a codegen-level +invariant (move-tracking + the `aggregate == 0` gate) that keeps static +constants out of the bare `ailang_rc_dec` route. The new site would +violate that invariant, so it carries an explicit **type precondition**: +it drops a param only when `field_drop_call` resolves to a real per-type +heap-ADT drop fn (`drop__`), i.e. **not** the bare +`ailang_rc_dec` fallback that `Type::Fn`, static-`Str`, and `Type::Var` +route to. This is sound (no underflow on a static constant) *and* +leak-correct for the real cases: a static closure / static `Str` +allocates nothing, so there is nothing to drop. (A heap-capturing closure +param in the exact leak-class position would leak rather than be freed — +but the existing machinery cannot free it either, so this is not a +regression; soundness is preferred over completeness here.) Witnessed by +the `std_either_demo` / `std_list_demo` / `poly_rec_capture_demo` e2e +tests, which pass closure refs through a `match` that consumes them on +one arm and leaves them live on another. + ### Why this is behaviour-preserving for legs 1/2 Legs 1 and 2 (already shipped) drop an owned param whose **aggregate** diff --git a/examples/if_branch_consume_split_no_leak_pin.ail b/examples/if_branch_consume_split_no_leak_pin.ail new file mode 100644 index 0000000..7257b5f --- /dev/null +++ b/examples/if_branch_consume_split_no_leak_pin.ail @@ -0,0 +1,21 @@ +(module if_branch_consume_split_no_leak_pin + (data Box (ctor B (con Int))) + (fn sink + (type (fn-type (params (own (con Box))) (ret (own (con Unit))) (effects IO))) + (params b) + (body (match b (case (pat-ctor B n) (do io/print_str "x"))))) + (fn run_if + (type (fn-type (params (own (con Box)) (own (con Int))) (ret (own (con Unit))) (effects IO))) + (params b k) + (body + (if (app ge k 0) + (do io/print_str "") + (app sink b)))) + (fn main + (type (fn-type (params) (ret (own (con Unit))) (effects IO))) + (params) + (body + (let b (term-ctor Box B 7) + (seq + (app run_if b 0) + (do io/print_str "done\n"))))))