Skip to content

Commit 1750f25

Browse files
committed
Fix Bug 14: break and continue release the iteration's references
Both statements jumped past the loop body's scope-end drops, leaking every reference created earlier in the iteration on the jump path (a finalizer-bearing resource never finalized on a continued or broken iteration). Track the innermost loop's body scope alongside the loop labels, and have break and continue walk the scope chain up to and including it, releasing reference-typed variables declared before the statement, then jump. This mirrors the early-return drop machinery and is disjoint from the fall-through path, so nothing releases twice. The added jump-path drop also gives affected variables a second drop, which the move-elision single-drop guard already treats as not provably movable, so the fix and the elision compose conservatively. Pinned with finalizer-oracle tests for both statements. Found pre-existing by the elision adversarial review.
1 parent 1560ff2 commit 1750f25

3 files changed

Lines changed: 97 additions & 12 deletions

File tree

plans/KNOWN_BUGS.md

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,23 @@ declared without a return-type annotation (its inferred return is indeterminate)
7171
loose typing the dedicated paths had, narrowed here rather than fully closed. Found by the arc 1a
7272
adversarial review, reconfirmed as the residual channel by the arc 1b and Bug 12 reviews.
7373

74-
## Bug 14 (OPEN, moderate): `continue` skips drops of references created earlier in the loop body
74+
## Bug 14 (FIXED): `continue` and `break` skipped drops of references created in the loop body
7575

76-
A `continue` inside a `while` jumps to the next iteration without releasing reference-typed values
77-
created earlier in that iteration's body, so each continued iteration leaks them (a finalizer-bearing
78-
resource created before the `continue` never runs its finalizer on the continued path). The normal
79-
fall-through end of the body drops them correctly, so this is drop placement on the `continue` jump
80-
path, the same family as the early-return drops that `emit_return_param_drops` handles for `return`.
76+
A `continue` inside a `while` jumped to the next iteration without releasing reference-typed values
77+
created earlier in that iteration's body, so each continued iteration leaked them (a finalizer-bearing
78+
resource created before the `continue` never ran its finalizer on the continued path). `break` had
79+
the same defect on the loop-exit path. The normal fall-through end of the body dropped them
80+
correctly, so this was drop placement on the jump paths, the same family as the early-return drops.
8181
Pre-existing, found by the reference-count elision adversarial review by differential comparison
82-
against the pre-elision build (both leak identically, so the elision is unrelated). Fix direction:
83-
emit the current iteration's scope-end drops before the `continue` jump, mirroring the return path.
82+
against the pre-elision build (both leaked identically, so the elision was unrelated).
83+
84+
Fixed by `loop_exit_drops`: the loop lowering tracks the innermost loop's body scope alongside its
85+
labels, and `break` and `continue` walk the scope chain from the statement's scope up to and
86+
including that body scope, releasing every reference-typed variable declared before the statement,
87+
then jump. Disjoint from the fall-through path, whose scope-end drops the jump never reaches, so no
88+
double release. Pinned by finalizer-oracle tests for both statements, and the extra drop on the jump
89+
path also disqualifies affected variables from move elision through the existing single-drop guard,
90+
so the two features compose conservatively.
8491

8592
## Bug 13 (FIXED): a block match-arm in value position yielded the wrong value
8693

solid-snake-compiler/src/bytecode_gen.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2066,6 +2066,25 @@ mod codegen_tests {
20662066
);
20672067
}
20682068

2069+
#[test]
2070+
fn continue_releases_the_iterations_references() {
2071+
// `continue` jumps past the body's scope-end drops, so it must release the iteration's live
2072+
// references itself. The continued iteration's resource finalizes at the continue, in order.
2073+
let (_, out) = run_src(&format!(
2074+
"{RES_TYPE}let i = 0\nwhile i < 4:\n let r = Res(i)\n i = i + 1\n if i == 2:\n continue\n print(\"use\", r.id)\n"
2075+
));
2076+
assert_eq!(out, "use 0\nD 0\nD 1\nuse 2\nD 2\nuse 3\nD 3\n");
2077+
}
2078+
2079+
#[test]
2080+
fn break_releases_the_iterations_references() {
2081+
// Same for `break`: the final iteration's references release before leaving the loop.
2082+
let (_, out) = run_src(&format!(
2083+
"{RES_TYPE}let i = 0\nwhile i < 5:\n let r = Res(i)\n i = i + 1\n if i == 3:\n break\nprint(\"done\")\n"
2084+
));
2085+
assert_eq!(out, "D 0\nD 1\nD 2\ndone\n");
2086+
}
2087+
20692088
#[test]
20702089
fn rc_aliasing_last_use_is_movable() {
20712090
// `let b = a` with `a` dead afterward is a move of the reference into `b`. `a` is a parameter

solid-snake-compiler/src/intermediate_pass1.rs

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,10 @@ pub struct AnalysisContext {
14571457
label_allocator: LabelAllocator,
14581458
loop_break_labels: Vec<Label>,
14591459
loop_continue_labels: Vec<Label>,
1460+
/// The innermost enclosing loop's body scope, pushed with the loop labels. `break` and
1461+
/// `continue` walk the scope chain up to it (inclusive) to release the iteration's live
1462+
/// references before jumping, the loop-exit counterpart of the early-return drops.
1463+
loop_body_scopes: Vec<usize>,
14601464
var_reads: HashSet<usize>,
14611465
var_assigns: HashMap<usize, usize>,
14621466
/// Signatures of all functions, keyed by name, populated before lowering so calls can
@@ -1587,6 +1591,7 @@ impl AnalysisContext {
15871591
label_allocator: LabelAllocator::new(),
15881592
loop_break_labels: Vec::new(),
15891593
loop_continue_labels: Vec::new(),
1594+
loop_body_scopes: Vec::new(),
15901595
var_reads: HashSet::new(),
15911596
var_assigns: HashMap::new(),
15921597
function_table: HashMap::new(),
@@ -5725,6 +5730,50 @@ fn return_param_drops(
57255730
drops
57265731
}
57275732

5733+
/// The drops a `break` or `continue` must emit before its jump: every reference-typed variable
5734+
/// declared before the statement in the scopes from the current one up to and including the
5735+
/// innermost loop's body scope. Without these the jump skips the body's scope-end drops, leaking
5736+
/// each continued or broken iteration's references. The loop-exit counterpart of
5737+
/// `return_param_drops`, and disjoint from the fall-through path, whose own scope-end drops the
5738+
/// jump never reaches.
5739+
fn loop_exit_drops(ctx: &AnalysisContext, span: Span, stmt_id: usize) -> Vec<IRInstruction> {
5740+
let Some(&body_scope) = ctx.loop_body_scopes.last() else {
5741+
return Vec::new();
5742+
};
5743+
let mut drops = Vec::new();
5744+
let mut scope = ctx.current_scope;
5745+
loop {
5746+
for v in ctx.scopes.scopes[scope]
5747+
.vars
5748+
.values()
5749+
.filter(|v| !v.moved && v.declared_at < stmt_id)
5750+
{
5751+
let typ = ctx.get_type(v.id);
5752+
if matches!(typ, IntermediateType::Custom { .. }
5753+
| IntermediateType::String
5754+
| IntermediateType::Array { .. }
5755+
| IntermediateType::List { .. }
5756+
| IntermediateType::Map { .. }
5757+
| IntermediateType::StrBuf) {
5758+
let var = IRVar::Var {
5759+
metadata: v.metadata(typ, span),
5760+
};
5761+
drops.push(IRInstruction::drop_scope_end(var, span, scope));
5762+
}
5763+
}
5764+
if scope == body_scope {
5765+
break;
5766+
}
5767+
match ctx.scopes.scopes[scope].parent_index {
5768+
Some(parent) => scope = parent,
5769+
// The statement sits outside the loop's scope chain, which the label stack should
5770+
// preclude, so there is nothing further to release.
5771+
None => break,
5772+
}
5773+
}
5774+
drops
5775+
}
5776+
57285777
/// Lower one function definition into an IR function unit: bind its parameters as locals of a fresh
57295778
/// scope, lower its body, and record the unit. Shared by ordinary function lowering and by
57305779
/// monomorphized generic instances.
@@ -6681,13 +6730,15 @@ pub fn create_scopes_map_ast(
66816730
// Push labels
66826731
ctx.loop_break_labels.push(end_label.clone());
66836732
ctx.loop_continue_labels.push(start_label.clone());
6733+
ctx.loop_body_scopes.push(loop_scope);
66846734

66856735
// Lower loop body
66866736
create_scopes_map_ast(body, ctx, Some(repeated.unwrap_or(node_c.statement_id)));
66876737

66886738
// Pop labels
66896739
ctx.loop_break_labels.pop();
66906740
ctx.loop_continue_labels.pop();
6741+
ctx.loop_body_scopes.pop();
66916742
ctx.current_scope = saved_scope;
66926743

66936744
// Unconditional jump back to start
@@ -6708,9 +6759,13 @@ pub fn create_scopes_map_ast(
67086759
}
67096760
ASTNode::Break => {
67106761
let span = node_c.span;
6711-
if let Some(label) = ctx.loop_break_labels.last() {
6762+
if let Some(label) = ctx.loop_break_labels.last().cloned() {
6763+
// Release this iteration's live references before leaving the
6764+
// loop, since the jump skips the body's scope-end drops.
6765+
let drops = loop_exit_drops(ctx, span, node_c.statement_id);
6766+
ctx.ir_output.extend(drops);
67126767
ctx.ir_output.push(IRInstruction::jump(
6713-
label.clone(),
6768+
label,
67146769
span,
67156770
node_c.statement_id,
67166771
ctx.current_scope,
@@ -6722,9 +6777,13 @@ pub fn create_scopes_map_ast(
67226777
}
67236778
ASTNode::Continue => {
67246779
let span = node_c.span;
6725-
if let Some(label) = ctx.loop_continue_labels.last() {
6780+
if let Some(label) = ctx.loop_continue_labels.last().cloned() {
6781+
// Release this iteration's live references before jumping to the
6782+
// next, since the jump skips the body's scope-end drops.
6783+
let drops = loop_exit_drops(ctx, span, node_c.statement_id);
6784+
ctx.ir_output.extend(drops);
67266785
ctx.ir_output.push(IRInstruction::jump(
6727-
label.clone(),
6786+
label,
67286787
span,
67296788
node_c.statement_id,
67306789
ctx.current_scope,

0 commit comments

Comments
 (0)