Skip to content

Commit 47f6599

Browse files
SamChou19815facebook-github-bot
authored andcommitted
[flow] Make the defense against cyclic tvars more robust
Summary: It turns out that it's still possible to crash with cyclic tvars under the builtin defense. In `ForcingState.map`, we are directly mapping a potentially bad cyclic lazy payload. This diff adds in extra defense. I added a test (`invalid_self_recursive_mapped`) which will fail without the defense. Note that these are edge cases. In practice, the lazy cyclic tvar we created are not that crazy and usually has extra indirections like `AnnotT` and `EvalT`, so that's why it hasn't blown up LTI 2.0 yet. Changelog: [internal] Reviewed By: panagosg7 Differential Revision: D55728897 fbshipit-source-id: ea3809edfbf47385ef125eb43d46f2561808a642
1 parent a3f8425 commit 47f6599

File tree

6 files changed

+76
-10
lines changed

6 files changed

+76
-10
lines changed

src/typing/__tests__/type_test.ml

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
(*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*)
7+
8+
open OUnit2
9+
open Type
10+
11+
let assert_forced_to_any s =
12+
match Constraint.ForcingState.force ~on_error:AnyT.error s with
13+
| AnyT _ -> ()
14+
| _ -> failwith "Invalid type"
15+
16+
let forcing_state_tests =
17+
[
18+
( "invalid_self_recursive" >:: fun _ ->
19+
let rec s_lazy =
20+
lazy
21+
(Constraint.ForcingState.of_lazy_t
22+
~error_reason:Reason.(locationless_reason RNull)
23+
(lazy (Constraint.ForcingState.force ~on_error:AnyT.error (Lazy.force s_lazy)))
24+
)
25+
in
26+
let s = Lazy.force s_lazy in
27+
assert_forced_to_any s
28+
);
29+
( "invalid_mutually_recursive" >:: fun _ ->
30+
let rec s1_lazy =
31+
lazy
32+
(Constraint.ForcingState.of_lazy_t
33+
~error_reason:Reason.(locationless_reason RNull)
34+
(lazy (Constraint.ForcingState.force ~on_error:AnyT.error (Lazy.force s2_lazy)))
35+
)
36+
and s2_lazy =
37+
lazy
38+
(Constraint.ForcingState.of_lazy_t
39+
~error_reason:Reason.(locationless_reason RNull)
40+
(lazy (Constraint.ForcingState.force ~on_error:AnyT.error (Lazy.force s1_lazy)))
41+
)
42+
in
43+
let s1 = Lazy.force s1_lazy in
44+
let s2 = Lazy.force s2_lazy in
45+
assert_forced_to_any s1;
46+
assert_forced_to_any s2
47+
);
48+
( "invalid_self_recursive_mapped" >:: fun _ ->
49+
let rec s_lazy =
50+
lazy
51+
(Constraint.ForcingState.of_lazy_t
52+
~error_reason:Reason.(locationless_reason RNull)
53+
(lazy (Constraint.ForcingState.force ~on_error:AnyT.error (Lazy.force s_lazy)))
54+
)
55+
in
56+
let s = Lazy.force s_lazy in
57+
let s' = Constraint.ForcingState.map ~on_error:AnyT.error ~f:Base.Fn.id s in
58+
assert_forced_to_any s'
59+
);
60+
]
61+
62+
let tests = "type" >::: ["forcing_state" >::: forcing_state_tests]

src/typing/__tests__/typing_tests.ml

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
open OUnit2
99

10-
let tests = "typing" >::: [Type_hint_test.tests]
10+
let tests = "typing" >::: [Type_test.tests; Type_hint_test.tests]
1111

1212
let _handle =
1313
let one_gig = 1024 * 1024 * 1024 in

src/typing/context.ml

+7-7
Original file line numberDiff line numberDiff line change
@@ -984,14 +984,14 @@ let find_root cx id = Type.Constraint.find_root cx.ccx.sig_cx.graph id
984984

985985
let find_root_id cx id = Type.Constraint.find_root_id cx.ccx.sig_cx.graph id
986986

987+
let on_cyclic_tvar_error cx reason =
988+
let msg = Error_message.(ETrivialRecursiveDefinition (Reason.loc_of_reason reason, reason)) in
989+
let error = Flow_error.error_of_msg ~trace_reasons:[] ~source_file:cx.file msg in
990+
add_error cx error;
991+
Type.AnyT.error reason
992+
987993
let force_fully_resolved_tvar cx =
988-
let on_error reason =
989-
let msg = Error_message.(ETrivialRecursiveDefinition (Reason.loc_of_reason reason, reason)) in
990-
let error = Flow_error.error_of_msg ~trace_reasons:[] ~source_file:cx.file msg in
991-
add_error cx error;
992-
Type.AnyT.error reason
993-
in
994-
Type.Constraint.ForcingState.force ~on_error
994+
Type.Constraint.ForcingState.force ~on_error:(on_cyclic_tvar_error cx)
995995

996996
let find_resolved =
997997
let rec loop cx seen t_in =

src/typing/context.mli

+2
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,8 @@ val find_root : t -> Type.ident -> Type.ident * Type.Constraint.node * Type.Cons
471471

472472
val find_root_id : t -> Type.ident -> Type.ident
473473

474+
val on_cyclic_tvar_error : t -> Reason.reason -> Type.t
475+
474476
val force_fully_resolved_tvar : t -> Type.Constraint.ForcingState.t -> Type.t
475477

476478
val find_resolved : t -> Type.t -> Type.t option

src/typing/merge_js.ml

+1
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,7 @@ let copier =
648648
failwith "unexpected unresolved constraint"
649649
| FullyResolved s ->
650650
ForcingState.map
651+
~on_error:(Context.on_cyclic_tvar_error src_cx)
651652
~f:(fun t ->
652653
let (_ : Context.t) = self#type_ src_cx pole dst_cx t in
653654
t)

src/typing/type.ml

+3-2
Original file line numberDiff line numberDiff line change
@@ -3256,7 +3256,7 @@ module Constraint = struct
32563256

32573257
val get_forced : t -> TypeTerm.t option
32583258

3259-
val map : f:(TypeTerm.t -> TypeTerm.t) -> t -> t
3259+
val map : on_error:(Reason.t -> TypeTerm.t) -> f:(TypeTerm.t -> TypeTerm.t) -> t -> t
32603260
end = struct
32613261
type state =
32623262
| Unforced
@@ -3294,7 +3294,8 @@ module Constraint = struct
32943294
None
32953295
| Forced -> Some (Lazy.force_val s.valid)
32963296

3297-
let map ~f s = { valid = Lazy.map f s.valid; error_reason = s.error_reason; state = Unforced }
3297+
let map ~on_error ~f s =
3298+
{ valid = lazy (f (force ~on_error s)); error_reason = s.error_reason; state = Unforced }
32983299
end
32993300

33003301
(** Constraints carry type information that narrows down the possible solutions

0 commit comments

Comments
 (0)