Skip to content

Commit 309710d

Browse files
FabianWolffFabianWolff
committed
Fix stack overflow when checking for structural recursion
1 parent 377d1a9 commit 309710d

File tree

1 file changed

+199
-17
lines changed

1 file changed

+199
-17
lines changed

‎compiler/rustc_ty_utils/src/representability.rs‎

Lines changed: 199 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,18 @@ pub fn ty_is_representable<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, sp: Span) -> R
2828
// contains a different, structurally recursive type, maintain a stack
2929
// of seen types and check recursion for each of them (issues #3008, #3779).
3030
let mut seen: Vec<Ty<'_>> = Vec::new();
31+
let mut shadow_seen: Vec<Ty<'_>> = Vec::new();
3132
let mut representable_cache = FxHashMap::default();
32-
let r = is_type_structurally_recursive(tcx, sp, &mut seen, &mut representable_cache, ty);
33+
let mut f_res = false;
34+
let r = is_type_structurally_recursive(
35+
tcx,
36+
sp,
37+
&mut seen,
38+
&mut shadow_seen,
39+
&mut representable_cache,
40+
ty,
41+
&mut f_res,
42+
);
3343
debug!("is_type_representable: {:?} is {:?}", ty, r);
3444
r
3545
}
@@ -48,21 +58,38 @@ fn are_inner_types_recursive<'tcx>(
4858
tcx: TyCtxt<'tcx>,
4959
sp: Span,
5060
seen: &mut Vec<Ty<'tcx>>,
61+
shadow_seen: &mut Vec<Ty<'tcx>>,
5162
representable_cache: &mut FxHashMap<Ty<'tcx>, Representability>,
5263
ty: Ty<'tcx>,
64+
f_res: &mut bool,
5365
) -> Representability {
66+
debug!("are_inner_types_recursive({:?}, {:?}, {:?})", ty, seen, shadow_seen);
5467
match ty.kind() {
5568
ty::Tuple(..) => {
5669
// Find non representable
57-
fold_repr(
58-
ty.tuple_fields().map(|ty| {
59-
is_type_structurally_recursive(tcx, sp, seen, representable_cache, ty)
60-
}),
61-
)
70+
fold_repr(ty.tuple_fields().map(|ty| {
71+
is_type_structurally_recursive(
72+
tcx,
73+
sp,
74+
seen,
75+
shadow_seen,
76+
representable_cache,
77+
ty,
78+
f_res,
79+
)
80+
}))
6281
}
6382
// Fixed-length vectors.
6483
// FIXME(#11924) Behavior undecided for zero-length vectors.
65-
ty::Array(ty, _) => is_type_structurally_recursive(tcx, sp, seen, representable_cache, ty),
84+
ty::Array(ty, _) => is_type_structurally_recursive(
85+
tcx,
86+
sp,
87+
seen,
88+
shadow_seen,
89+
representable_cache,
90+
ty,
91+
f_res,
92+
),
6693
ty::Adt(def, substs) => {
6794
// Find non representable fields with their spans
6895
fold_repr(def.all_fields().map(|field| {
@@ -76,12 +103,142 @@ fn are_inner_types_recursive<'tcx>(
76103
Some(hir::Node::Field(field)) => field.ty.span,
77104
_ => sp,
78105
};
79-
match is_type_structurally_recursive(tcx, span, seen, representable_cache, ty) {
80-
Representability::SelfRecursive(_) => {
81-
Representability::SelfRecursive(vec![span])
106+
107+
let mut result = None;
108+
109+
// First, we check whether the field type per se is representable.
110+
// This catches cases as in #74224 and #84611. There is a special
111+
// case related to mutual recursion, though; consider this example:
112+
//
113+
// struct A<T> {
114+
// z: T,
115+
// x: B<T>,
116+
// }
117+
//
118+
// struct B<T> {
119+
// y: A<T>
120+
// }
121+
//
122+
// Here, without the following special case, both A and B are
123+
// ContainsRecursive, which is a problem because we only report
124+
// errors for SelfRecursive. We fix this by detecting this special
125+
// case (shadow_seen.first() is the type we are originally
126+
// interested in, and if we ever encounter the same AdtDef again,
127+
// we know that it must be SelfRecursive) and "forcibly" returning
128+
// SelfRecursive (by setting f_res, which tells the calling
129+
// invocations of are_inner_types_representable to forward the
130+
// result without adjusting).
131+
if shadow_seen.len() > 1 && shadow_seen.len() > seen.len() {
132+
match shadow_seen.first().map(|ty| ty.kind()) {
133+
Some(ty::Adt(f_def, _)) => {
134+
if f_def == def {
135+
*f_res = true;
136+
result = Some(Representability::SelfRecursive(vec![span]));
137+
}
138+
}
139+
Some(_) => {
140+
bug!("shadow_seen stack contains non-ADT type: {:?}", ty);
141+
}
142+
None => unreachable!(),
143+
}
144+
}
145+
146+
if result == None {
147+
result = Some(Representability::Representable);
148+
149+
// Now, we check whether the field types per se are representable, e.g.
150+
// for struct Foo { x: Option<Foo> }, we first check whether Option<_>
151+
// by itself is representable (which it is), and the nesting of Foo
152+
// will be detected later. This is necessary for #74224 and #84611.
153+
154+
// If we have encountered an ADT definition that we have not seen
155+
// before (no need to check them twice), recurse to see whether that
156+
// definition is SelfRecursive. If so, we must be ContainsRecursive.
157+
if shadow_seen.iter().len() > 1
158+
&& !shadow_seen.iter().take(shadow_seen.iter().len() - 1).any(|seen_ty| {
159+
match seen_ty.kind() {
160+
ty::Adt(seen_def, _) => seen_def == def,
161+
_ => {
162+
bug!("seen stack contains non-ADT type: {:?}", seen_ty);
163+
}
164+
}
165+
})
166+
{
167+
let adt_def_id = def.did;
168+
let raw_adt_ty = tcx.type_of(adt_def_id);
169+
debug!("are_inner_types_recursive: checking nested type: {:?}", raw_adt_ty);
170+
171+
// Check independently whether the ADT is SelfRecursive. If so,
172+
// we must be ContainsRecursive (except for the special case
173+
// mentioned above).
174+
let mut nested_seen: Vec<Ty<'_>> = vec![];
175+
result = Some(
176+
match is_type_structurally_recursive(
177+
tcx,
178+
span,
179+
&mut nested_seen,
180+
shadow_seen,
181+
representable_cache,
182+
raw_adt_ty,
183+
f_res,
184+
) {
185+
Representability::SelfRecursive(_) => {
186+
if *f_res {
187+
Representability::SelfRecursive(vec![span])
188+
} else {
189+
Representability::ContainsRecursive
190+
}
191+
}
192+
x => x,
193+
},
194+
);
195+
}
196+
197+
// We only enter the following block if the type looks representable
198+
// so far. This is necessary for cases such as this one (#74224):
199+
//
200+
// struct A<T> {
201+
// x: T,
202+
// y: A<A<T>>,
203+
// }
204+
//
205+
// struct B {
206+
// z: A<usize>
207+
// }
208+
//
209+
// When checking B, we recurse into A and check field y of type
210+
// A<A<usize>>. We haven't seen this exact type before, so we recurse
211+
// into A<A<usize>>, which contains, A<A<A<usize>>>, and so forth,
212+
// ad infinitum. We can prevent this from happening by first checking
213+
// A separately (the code above) and only checking for nested Bs if
214+
// A actually looks representable (which it wouldn't in this example).
215+
if result == Some(Representability::Representable) {
216+
// Now, even if the type is representable (e.g. Option<_>),
217+
// it might still contribute to a recursive type, e.g.:
218+
// struct Foo { x: Option<Option<Foo>> }
219+
// These cases are handled by passing the full `seen`
220+
// stack to is_type_structurally_recursive (instead of the
221+
// empty `nested_seen` above):
222+
result = Some(
223+
match is_type_structurally_recursive(
224+
tcx,
225+
span,
226+
seen,
227+
shadow_seen,
228+
representable_cache,
229+
ty,
230+
f_res,
231+
) {
232+
Representability::SelfRecursive(_) => {
233+
Representability::SelfRecursive(vec![span])
234+
}
235+
x => x,
236+
},
237+
);
82238
}
83-
x => x,
84239
}
240+
241+
result.unwrap()
85242
}))
86243
}
87244
ty::Closure(..) => {
@@ -106,8 +263,10 @@ fn is_type_structurally_recursive<'tcx>(
106263
tcx: TyCtxt<'tcx>,
107264
sp: Span,
108265
seen: &mut Vec<Ty<'tcx>>,
266+
shadow_seen: &mut Vec<Ty<'tcx>>,
109267
representable_cache: &mut FxHashMap<Ty<'tcx>, Representability>,
110268
ty: Ty<'tcx>,
269+
f_res: &mut bool,
111270
) -> Representability {
112271
debug!("is_type_structurally_recursive: {:?} {:?}", ty, sp);
113272
if let Some(representability) = representable_cache.get(ty) {
@@ -118,8 +277,15 @@ fn is_type_structurally_recursive<'tcx>(
118277
return representability.clone();
119278
}
120279

121-
let representability =
122-
is_type_structurally_recursive_inner(tcx, sp, seen, representable_cache, ty);
280+
let representability = is_type_structurally_recursive_inner(
281+
tcx,
282+
sp,
283+
seen,
284+
shadow_seen,
285+
representable_cache,
286+
ty,
287+
f_res,
288+
);
123289

124290
representable_cache.insert(ty, representability.clone());
125291
representability
@@ -129,12 +295,16 @@ fn is_type_structurally_recursive_inner<'tcx>(
129295
tcx: TyCtxt<'tcx>,
130296
sp: Span,
131297
seen: &mut Vec<Ty<'tcx>>,
298+
shadow_seen: &mut Vec<Ty<'tcx>>,
132299
representable_cache: &mut FxHashMap<Ty<'tcx>, Representability>,
133300
ty: Ty<'tcx>,
301+
f_res: &mut bool,
134302
) -> Representability {
135303
match ty.kind() {
136304
ty::Adt(def, _) => {
137305
{
306+
debug!("is_type_structurally_recursive_inner: adt: {:?}, seen: {:?}", ty, seen);
307+
138308
// Iterate through stack of previously seen types.
139309
let mut iter = seen.iter();
140310

@@ -158,8 +328,10 @@ fn is_type_structurally_recursive_inner<'tcx>(
158328
// will recurse infinitely for some inputs.
159329
//
160330
// It is important that we DO take generic parameters into account
161-
// here, so that code like this is considered SelfRecursive, not
162-
// ContainsRecursive:
331+
// here, because nesting e.g. Options is allowed (as long as the
332+
// definition of Option doesn't itself include an Option field, which
333+
// would be a case of SelfRecursive above). The following, too, counts
334+
// as SelfRecursive:
163335
//
164336
// struct Foo { Option<Option<Foo>> }
165337

@@ -174,13 +346,23 @@ fn is_type_structurally_recursive_inner<'tcx>(
174346
// For structs and enums, track all previously seen types by pushing them
175347
// onto the 'seen' stack.
176348
seen.push(ty);
177-
let out = are_inner_types_recursive(tcx, sp, seen, representable_cache, ty);
349+
shadow_seen.push(ty);
350+
let out = are_inner_types_recursive(
351+
tcx,
352+
sp,
353+
seen,
354+
shadow_seen,
355+
representable_cache,
356+
ty,
357+
f_res,
358+
);
359+
shadow_seen.pop();
178360
seen.pop();
179361
out
180362
}
181363
_ => {
182364
// No need to push in other cases.
183-
are_inner_types_recursive(tcx, sp, seen, representable_cache, ty)
365+
are_inner_types_recursive(tcx, sp, seen, shadow_seen, representable_cache, ty, f_res)
184366
}
185367
}
186368
}

0 commit comments

Comments
 (0)