Skip to content

Commit e1831b2

Browse files
hyperpolymathclaude
andcommitted
fix(borrow): round-3 hardening — ref-count Slice-B reassign + union reassigned-local summary
A third adversarial-verification pass on the #554 return-borrow work surfaced two more reachable holes (and confirmed the interprocedural fixpoint is robust: 60-level wrapper chains, mutual recursion, and 3-cycles all terminate in ms and summarise correctly). - Slice-B `&` reassign (a PRE-EXISTING soundness bug, exposed by the #554 (c) block being more correct): `r = &b` ended the binder's old loan UNCONDITIONALLY, so `let r2 = r; r = &b; consume(a); *r2` dropped the borrow `r2` still aliased and accepted a use-after-move. Now ref-counted by b_id (mirroring the (c) block and expire_dead_ref_bindings), so an aliased loan survives the reassign. The `&mut` variant (exclusive aliasing) is caught too. - Reassigned-local summary: the summary walker did not update a reassigned ref-local's origins, so `let mut t = pick(y); t = pick(x); return t` summarised the stale {y}. It now UNIONs origins on reassignment (conservative, flow-insensitive), summarising {x,y}. Two low-severity residuals are documented in-code (Polonius #553 closes them): a summary that cannot bootstrap an origin for a base-case-less divergent recursion (unreachable — the function never returns), and a loop-body reassign to an outer borrow (&-symmetric, predates #554). Full suite 397/397 green; +2 regression fixtures (alias-survives-reassign, reassigned-local-summary). Refs #554. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 20b2131 commit e1831b2

4 files changed

Lines changed: 145 additions & 5 deletions

File tree

lib/borrow.ml

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,22 @@ let compute_ret_borrow_params (lookup : string -> int list option)
361361
match s with
362362
| StmtLet sl -> walk_expr sl.sl_value; record_let sl.sl_pat sl.sl_value
363363
| StmtExpr e -> walk_expr e
364-
| StmtAssign (l, _, r) -> walk_expr l; walk_expr r
364+
| StmtAssign (l, _, r) ->
365+
walk_expr l; walk_expr r;
366+
(* A whole-var reassignment may change which parameters a ref-local
367+
borrows. UNION the new origins into the local's set (rather than
368+
replace) so the flow-insensitive summary never *drops* a possible
369+
origin — conservative, so [let mut t = pick(y); t = pick(x); return t]
370+
summarises {x,y} not the stale {y}, closing the reassigned-local
371+
false negative. #554 round-3. *)
372+
(match peel l with
373+
| ExprVar id ->
374+
let prior =
375+
match Hashtbl.find_opt local_origins id.name with Some x -> x | None -> []
376+
in
377+
Hashtbl.replace local_origins id.name
378+
(List.sort_uniq compare (prior @ origins_of_ref_source r))
379+
| _ -> ())
365380
| StmtWhile (c, b) -> walk_expr c; walk_block b
366381
| StmtFor (_, it, b) -> walk_expr it; walk_block b
367382
and walk_block (blk : block) : unit =
@@ -1793,10 +1808,22 @@ and check_stmt (ctx : context) (state : state) (symbols : Symbol.t) (stmt : stmt
17931808
when not rhs_is_self_binder
17941809
&& is_reborrow_source state symbols rhs
17951810
&& List.mem_assoc binder_sym state.ref_bindings ->
1796-
let old_borrow = List.assoc binder_sym state.ref_bindings in
1797-
end_borrow state old_borrow;
1798-
state.ref_bindings <-
1799-
List.filter (fun (s, _) -> s <> binder_sym) state.ref_bindings;
1811+
(* Ref-count the released loan(s) by [b_id]: only [end_borrow] an
1812+
old borrow if no surviving ref-binding (e.g. a [let r2 = r]
1813+
alias) still holds it — mirrors [expire_dead_ref_bindings] and
1814+
the #554 (c) call-result reassign block. Pre-fix this ended
1815+
the borrow unconditionally, so [let r2 = r; r = &b] dropped the
1816+
loan [r2] still names and accepted a later use-after-move of
1817+
the old target. (Found by #554 round-3 adversarial review;
1818+
the bug predates #554 but the (c) block made the asymmetry
1819+
visible.) *)
1820+
let old_entries, remaining =
1821+
List.partition (fun (s, _) -> s = binder_sym) state.ref_bindings
1822+
in
1823+
List.iter (fun (_, ob) ->
1824+
if not (List.exists (fun (_, b') -> b'.b_id = ob.b_id) remaining)
1825+
then end_borrow state ob) old_entries;
1826+
state.ref_bindings <- remaining;
18001827
Some binder_sym
18011828
| _ -> None
18021829
in
@@ -2111,6 +2138,12 @@ let check_program (symbols : Symbol.t) (program : program) : unit result =
21112138
loan (ref-counted by [b_id]) and rebinds to the escaping call result,
21122139
so a later use of the old target is no longer spuriously rejected —
21132140
symmetric with the plain-`&` Slice-B reborrow.
2141+
- *round-3 hardening* — the Slice-B `&` reassign path now ref-counts the
2142+
released loan by [b_id] too, so [let r2 = r; r = &b] keeps the borrow
2143+
[r2] still aliases (a *pre-existing* soundness bug the (c) block exposed
2144+
by being more correct); and the summary walker UNIONs a reassigned
2145+
ref-local's origins, so [let mut t = pick(y); t = pick(x); return t]
2146+
summarises {x,y} rather than the stale {y}.
21142147
21152148
Still deferred:
21162149
- Origin/region variables (true Polonius surface) — a region
@@ -2133,6 +2166,20 @@ let check_program (symbols : Symbol.t) (program : program) : unit result =
21332166
same pre-existing through-branch / copy-out lexical limitation,
21342167
inherited by the call-result path, not introduced by it. A true
21352168
flow-sensitive borrow graph (Polonius) discharges it uniformly.
2169+
- #554 minor residuals (round-3, both low-severity; Polonius #553 closes
2170+
them):
2171+
* *Divergent self/mutual recursion with no base case* — a function
2172+
whose only ref-return is its own recursive call (e.g.
2173+
[fn f(ref x) -> ref Int { let t = f(x); return t; }]) gets an empty
2174+
summary (the fixpoint cannot bootstrap an origin from a purely
2175+
self-transitive return), so a use-after-move through its result is
2176+
accepted. Unreachable in practice: such a function never returns,
2177+
so the use never executes. Any *terminating* recursion has a
2178+
non-recursive return path, which IS summarised.
2179+
* *Reassign inside a loop body to an outer borrow* —
2180+
[let mut r = pick(a); while … { r = pick(b) } consume(b); *r] is
2181+
accepted because the loop-body block drops its borrows at block exit
2182+
(predates #554, `&`-symmetric).
21362183
- Tighter integration with the quantity checker for captured
21372184
linears (Slice D).
21382185
*)
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// SPDX-FileCopyrightText: 2026 hyperpolymath
3+
//
4+
// #554 reassigned-local summary (found by round-3 adversarial review). When a
5+
// returned ref-local is reassigned (`let mut t = pick(y); t = pick(x)`), the
6+
// return-borrow summary must UNION the origins so it does not go stale on the
7+
// initial binding. `wrap` here ultimately returns a borrow of `x`, so the
8+
// summary must include parameter 0; `let r = wrap(a, b); consume(a); *r` is a
9+
// use-after-move on `a` and must be rejected (the summary walker unions the
10+
// reassigned local's origins, conservatively).
11+
12+
module BorrowCalleeReturnedBorrowReassignSummary;
13+
14+
fn pick(ref x: Int) -> ref Int {
15+
return &x;
16+
}
17+
18+
fn wrap(ref x: Int, ref y: Int) -> ref Int {
19+
let mut t = pick(y);
20+
t = pick(x);
21+
return t;
22+
}
23+
24+
fn consume(own v: Int) -> Int {
25+
return v;
26+
}
27+
28+
fn main() -> Int {
29+
let a: Int = 1;
30+
let b: Int = 2;
31+
let r = wrap(a, b);
32+
let _gone = consume(a);
33+
let v = *r;
34+
return v;
35+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// SPDX-FileCopyrightText: 2026 hyperpolymath
3+
//
4+
// Slice-B reassign ref-count (pre-existing soundness bug, found by #554
5+
// round-3 adversarial review). When a ref-binder is reassigned (`r = &b`),
6+
// its OLD loan must only be released if no surviving alias still holds it.
7+
// Here `let r2 = r` aliases `a`'s borrow into `r2`; after `r = &b`, `r2` still
8+
// borrows `a`, so moving `a` and then reading `*r2` is a use-after-move and
9+
// must be rejected. Pre-fix the Slice-B path ended the old borrow
10+
// unconditionally and the move slipped past.
11+
12+
module BorrowReassignAliasSurvives;
13+
14+
fn consume(own v: Int) -> Int {
15+
return v;
16+
}
17+
18+
fn main() -> Int {
19+
let mut a: Int = 1;
20+
let mut b: Int = 2;
21+
let mut r = &a;
22+
let r2 = r;
23+
r = &b;
24+
let _gone = consume(a);
25+
let v = *r2;
26+
return v;
27+
}

test/test_e2e.ml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5277,6 +5277,33 @@ let test_borrow_callee_returned_borrow_reassign_old_ok () =
52775277
target `a` must be movable again, got: "
52785278
^ Borrow.format_borrow_error e)
52795279

5280+
(* Slice-B reassign ref-count (pre-existing soundness bug found by #554
5281+
round-3): `let r2 = r; r = &b` must NOT drop the loan `r2` still aliases,
5282+
so a later move of the old target is rejected. *)
5283+
let test_borrow_reassign_alias_survives () =
5284+
match borrow_result (fixture "borrow_reassign_alias_survives.affine") with
5285+
| Error (Borrow.MoveWhileBorrowed _) -> ()
5286+
| Error e ->
5287+
Alcotest.fail ("Slice-B alias ref-count: expected MoveWhileBorrowed (r2 \
5288+
still borrows `a` after `r = &b`), got: "
5289+
^ Borrow.format_borrow_error e)
5290+
| Ok () ->
5291+
Alcotest.fail "Slice-B alias regressed: reassigning `r` dropped the borrow \
5292+
the `let r2 = r` alias still held — use-after-move accepted"
5293+
5294+
(* #554 reassigned-local summary (found by round-3): a returned ref-local that
5295+
is reassigned must union its origins so the summary does not go stale. *)
5296+
let test_borrow_callee_returned_borrow_reassign_summary () =
5297+
match borrow_result (fixture "borrow_callee_returned_borrow_reassign_summary.affine") with
5298+
| Error (Borrow.MoveWhileBorrowed _) -> ()
5299+
| Error e ->
5300+
Alcotest.fail ("#554 reassigned-local summary: expected MoveWhileBorrowed \
5301+
(the reassigned local's origin must be unioned into the \
5302+
summary), got: " ^ Borrow.format_borrow_error e)
5303+
| Ok () ->
5304+
Alcotest.fail "#554 reassigned-local summary regressed: the summary went \
5305+
stale on the initial binding — use-after-move accepted"
5306+
52805307
let borrow_tests = [
52815308
Alcotest.test_case "BorrowOutlivesOwner: &local escapes its block"
52825309
`Quick test_borrow_outlives_owner;
@@ -5352,6 +5379,10 @@ let borrow_tests = [
53525379
`Quick test_borrow_callee_returned_borrow_interproc;
53535380
Alcotest.test_case "#554 (c): reassign to call result releases old loan (precision)"
53545381
`Quick test_borrow_callee_returned_borrow_reassign_old_ok;
5382+
Alcotest.test_case "Slice-B: reassign ref-counts old loan vs surviving alias"
5383+
`Quick test_borrow_reassign_alias_survives;
5384+
Alcotest.test_case "#554: reassigned returned ref-local unions summary origins"
5385+
`Quick test_borrow_callee_returned_borrow_reassign_summary;
53555386
]
53565387

53575388
(* ============================================================================

0 commit comments

Comments
 (0)