Skip to content

Commit e47ecc5

Browse files
authored
Fix a soundness bug in Simplif (#3832)
Ensure that Alias bindings are not substituted under lambda This used to be the case, but the recent refactoring of Lletrec introduced a bug. (Due to locals, this is a soundness issue rather than just poor optimisation)
1 parent 062197a commit e47ecc5

File tree

4 files changed

+49
-4
lines changed

4 files changed

+49
-4
lines changed

lambda/lambda.mli

+2-1
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,8 @@ type let_kind = Strict | Alias | StrictOpt
691691
(If e is a simple expression, e.g. a variable or constant,
692692
we may still substitute e'[x/e].)
693693
Alias: e is pure, we can substitute e'[x/e] if x has 0 or 1 occurrences
694-
in e'
694+
in e', and these occurrences are definitely in the same region as
695+
the binding (not inside a lambda nor an exclave)
695696
StrictOpt: e does not have side-effects, but depend on the store;
696697
we can discard e if x does not appear in e'
697698
*)

lambda/simplif.ml

+5-3
Original file line numberDiff line numberDiff line change
@@ -465,8 +465,8 @@ let simplify_lets lam =
465465
end
466466
| _ -> no_opt ()
467467
end
468-
| Lfunction {body} ->
469-
count Ident.Map.empty body
468+
| Lfunction fn ->
469+
count_lfunction fn
470470
| Llet(_str, _k, v, Lvar w, l2) when optimize ->
471471
(* v will be replaced by w in l2, so each occurrence of v in l2
472472
increases w's refcount *)
@@ -480,7 +480,7 @@ let simplify_lets lam =
480480
count bv l1;
481481
count bv l2
482482
| Lletrec(bindings, body) ->
483-
List.iter (fun { def } -> count bv def.body) bindings;
483+
List.iter (fun { def } -> count_lfunction def) bindings;
484484
count bv body
485485
| Lprim(_p, ll, _) -> List.iter (count bv) ll
486486
| Lswitch(l, sw, _loc, _kind) ->
@@ -528,6 +528,8 @@ let simplify_lets lam =
528528
(* Don't move code into an exclave *)
529529
count Ident.Map.empty l2
530530

531+
and count_lfunction fn =
532+
count Ident.Map.empty fn.body
531533

532534
and count_default bv sw = match sw.sw_failaction with
533535
| None -> ()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
(* TEST *)
2+
3+
(* Regression test for incorrect Simplif substitution of Alias bindings
4+
into Lletrec closures *)
5+
6+
let ok () =
7+
Gc.full_major ();
8+
print_endline "ok"
9+
10+
let[@inline never] test1 f =
11+
match
12+
if Sys.opaque_identity false
13+
then (Sys.opaque_identity 1), 3
14+
else (Sys.opaque_identity 2), 4
15+
with
16+
| a, b ->
17+
let rec loop () =
18+
let retry () =
19+
ignore (Sys.opaque_identity b);
20+
loop ()
21+
in
22+
f ();
23+
if Sys.opaque_identity false then retry ()
24+
in
25+
loop ()
26+
27+
let () = test1 ok
28+
29+
type 'a g = { g: 'a @@ global }
30+
31+
let[@inline never] test2 f =
32+
let { g = r } = { g = ref 42 } in
33+
let rec loop () =
34+
f ();
35+
ignore (Sys.opaque_identity r);
36+
if Sys.opaque_identity false then loop ()
37+
in
38+
loop
39+
40+
let () = test2 ok ()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ok
2+
ok

0 commit comments

Comments
 (0)