fix(compiler): storing an owned value in a container is a MOVE
The disclosed "move-on-push" gap detonated on iteration 16's route-table pattern: `push(self.routes, r)` moved the Route into the container while the `take r` parameter's scope-end DROP still fired — the container's own drop plan (multi_free) then freed the element a second time. ASan: SEGV in class_free during trap unwind; latent until now because pushed elements were Texts, which copy at the boundary (2026-08-14). owner.ml analyze_call: `push`'s value slot and `set`'s key/value slots now TRANSFER an Owned, non-copy-stored place (record_move, exactly the take-arg shape), so the pusher's drop disappears. Text/json.Value keep the copy-store path (stores_by_copy) and the caller still drops the fresh copy. Traced (gc) values remain exempt (tracing owns them). A user-declared push/set fn of the same name wins, per the builtin shadowing rule. Pinned by tests/corpus/run/container-owned-move (route table: interface- typed field values pushed via take params, dispatched by ICALL, mixed with Text pushes) — the exact iteration-16 shape, ASan-clean. Verified: woc-test 540/0; oop-e2e 88/0; log-watcher 7/0; employee 8/0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e48b175472
commit
6ff119cd57
3 changed files with 74 additions and 2 deletions
|
|
@ -1304,8 +1304,27 @@ and analyze_call (ctx : ctx) (call_e : Ast.expr) (callee : Ast.expr) (args : Ast
|
|||
(* transfers last *)
|
||||
(* iteration 7b deleted the `push`-of-a-gc-value special case (an RC_INC
|
||||
escape site): a traced value stored into a container needs no
|
||||
bookkeeping — tracing finds it through the container. The bug class the
|
||||
old special case guarded against cannot recur without RC. *)
|
||||
bookkeeping — tracing finds it through the container.
|
||||
|
||||
Storing an OWNED, non-copied value into a container is a MOVE, exactly
|
||||
like a `take` argument: the container owns the element and frees it in
|
||||
its own drop plan (runtime/src/gc.c multi_free/map_free), so the caller
|
||||
dropping it too was a double free. This was the disclosed
|
||||
"move-on-push" gap — it stayed latent while pushed elements were Texts
|
||||
(copied at the boundary, 2026-08-14) and detonated the moment iteration
|
||||
16's route-table pattern pushed a CLASS value (`push(self.routes, r)`
|
||||
plus the take-param's scope-end DROP = the container's element freed
|
||||
twice). `push`'s value slot and `set`'s key/value slots transfer;
|
||||
Text/json.Value stay copy-stored (`stores_by_copy`), so their fresh-
|
||||
value drop is still the caller's. A user-declared fn of the same name
|
||||
wins, exactly like the builtin table's shadowing rule. *)
|
||||
let container_store_slot (i : int) : bool =
|
||||
match callee.kind with
|
||||
| Ident "push" -> i = 1 && Types.StringMap.find_opt "push" ctx.syms.Types.free_fns = None
|
||||
| Ident "set" ->
|
||||
(i = 1 || i = 2) && Types.StringMap.find_opt "set" ctx.syms.Types.free_fns = None
|
||||
| _ -> false
|
||||
in
|
||||
List.iteri
|
||||
(fun i a ->
|
||||
match place_of a with
|
||||
|
|
@ -1315,6 +1334,10 @@ and analyze_call (ctx : ctx) (call_e : Ast.expr) (callee : Ast.expr) (args : Ast
|
|||
if conv = Take then
|
||||
(if transfer ctx p ~what:(Printf.sprintf "cannot be passed to `take %s`" pname) then
|
||||
record_move ctx p (MvArg pname))
|
||||
else if container_store_slot i && place_class ctx p = Owned
|
||||
&& not (stores_by_copy ctx p) then
|
||||
(if transfer ctx p ~what:"cannot be stored in a container" then
|
||||
record_move ctx p (MvArg "element"))
|
||||
)
|
||||
args;
|
||||
record_drop ctx ~node:call_e.id ~pos:call_e.pos ~kind:DLiveMask
|
||||
|
|
|
|||
4
tests/corpus/run/container-owned-move/fixture.out
Normal file
4
tests/corpus/run/container-owned-move/fixture.out
Normal file
|
|
@ -0,0 +1,4 @@
|
|||
101
|
||||
8
|
||||
-1
|
||||
2
|
||||
45
tests/corpus/run/container-owned-move/fixture.wo
Normal file
45
tests/corpus/run/container-owned-move/fixture.wo
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
-- Storing an OWNED class value into a container is a MOVE (iteration 16's
|
||||
-- route-table pattern found the disclosed "move-on-push" double-free: the
|
||||
-- container's drop plan frees elements, so the pusher must not also drop
|
||||
-- what it pushed). Texts stay copy-stored — the mixed push below pins both.
|
||||
interface Handler {
|
||||
fn handle(x: Int) -> Int
|
||||
}
|
||||
|
||||
class AddOne {
|
||||
n: Int
|
||||
fn handle(x: Int) -> Int { return x + self.n }
|
||||
}
|
||||
|
||||
class Route {
|
||||
path: Text
|
||||
h: Handler
|
||||
}
|
||||
|
||||
class App {
|
||||
routes: multi Route
|
||||
names: multi Text
|
||||
|
||||
fn add(take r: Route, name: Text) {
|
||||
push(self.routes, r);
|
||||
push(self.names, name);
|
||||
}
|
||||
|
||||
fn run(path: Text, x: Int) -> Int {
|
||||
for r in self.routes {
|
||||
if r.path == path { return r.h.handle(x); }
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
fn main(args: multi Text) -> Int {
|
||||
let app = App { routes: [], names: [] };
|
||||
app.add(Route { path: "/a", h: AddOne { n: 7 } }, "a");
|
||||
app.add(Route { path: "/b", h: AddOne { n: 100 } }, "b");
|
||||
print_int(app.run("/b", 1));
|
||||
print_int(app.run("/a", 1));
|
||||
print_int(app.run("/x", 1));
|
||||
print_int(count(app.names));
|
||||
return 0;
|
||||
}
|
||||
Loading…
Reference in a new issue