Skip to content

Commit a219c4e

Browse files
committed
ImproperCTypes: add recursion limit
Simple change to stop irregular recursive types from causing infinitely-deep recursion in type checking.
1 parent c52aa53 commit a219c4e

5 files changed

Lines changed: 66 additions & 28 deletions

File tree

compiler/rustc_lint/src/types/improper_ctypes.rs

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,8 @@ struct VisitorState {
348348
/// Flags describing both the immediate context in which the current mir::Ty is,
349349
/// linked to how it relates to its parent mir::Ty (or lack thereof).
350350
outer_ty_kind: OuterTyKind,
351+
/// Type recursion depth, to prevent infinite recursion
352+
depth: usize,
351353
}
352354

353355
impl RootUseFlags {
@@ -375,6 +377,7 @@ impl VisitorState {
375377
Self {
376378
root_use_flags: self.root_use_flags,
377379
outer_ty_kind: OuterTyKind::from_outer_ty(current_ty),
380+
depth: self.depth + 1,
378381
}
379382
}
380383
/// From an existing state, compute the state of any subtype of the current type.
@@ -388,12 +391,13 @@ impl VisitorState {
388391
FnPos::Arg => RootUseFlags::ARGUMENT_TY_IN_FNPTR,
389392
},
390393
outer_ty_kind: OuterTyKind::from_outer_ty(current_ty),
394+
depth: self.depth + 1,
391395
}
392396
}
393397

394398
/// Generate the state for an "outermost" type that needs to be checked
395399
fn entry_point(root_use_flags: RootUseFlags) -> Self {
396-
Self { root_use_flags, outer_ty_kind: OuterTyKind::None }
400+
Self { root_use_flags, outer_ty_kind: OuterTyKind::None, depth: 0 }
397401
}
398402

399403
/// Get the proper visitor state for a given function's arguments or return type.
@@ -729,9 +733,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
729733

730734
// Protect against infinite recursion, for example
731735
// `struct S(*mut S);`.
732-
// FIXME: A recursion limit is necessary as well, for irregular
733-
// recursive types.
734-
if !self.cache.insert(ty) {
736+
if !(self.cache.insert(ty) && self.cx.tcx.recursion_limit().value_within_limit(state.depth))
737+
{
735738
return FfiSafe;
736739
}
737740

@@ -954,20 +957,25 @@ impl<'tcx> ImproperCTypesLint {
954957
fn_mode: CItemKind,
955958
) {
956959
struct FnPtrFinder<'tcx> {
960+
current_depth: usize,
961+
depths: Vec<usize>,
957962
spans: Vec<Span>,
958963
tys: Vec<Ty<'tcx>>,
959964
}
960965

961966
impl<'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'tcx> {
962967
fn visit_ty(&mut self, ty: &'_ hir::Ty<'_, AmbigArg>) {
963968
debug!(?ty);
969+
self.current_depth += 1;
964970
if let hir::TyKind::FnPtr(hir::FnPtrTy { abi, .. }) = ty.kind
965971
&& !abi.is_rustic_abi()
966972
{
973+
self.depths.push(self.current_depth);
967974
self.spans.push(ty.span);
968975
}
969976

970977
hir::intravisit::walk_ty(self, ty);
978+
self.current_depth -= 1;
971979
}
972980
}
973981

@@ -985,15 +993,24 @@ impl<'tcx> ImproperCTypesLint {
985993
}
986994
}
987995

988-
let mut visitor = FnPtrFinder { spans: Vec::new(), tys: Vec::new() };
996+
let mut visitor = FnPtrFinder {
997+
spans: Vec::new(),
998+
tys: Vec::new(),
999+
depths: Vec::new(),
1000+
current_depth: 0,
1001+
};
9891002
ty.visit_with(&mut visitor);
9901003
visitor.visit_ty_unambig(hir_ty);
9911004

992-
let all_types = iter::zip(visitor.tys.drain(..), visitor.spans.drain(..));
993-
for (fn_ptr_ty, span) in all_types {
1005+
let all_types = iter::zip(
1006+
visitor.depths.drain(..),
1007+
iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)),
1008+
);
1009+
for (depth, (fn_ptr_ty, span)) in all_types {
9941010
let mut visitor = ImproperCTypesVisitor::new(cx, fn_ptr_ty, fn_mode);
1011+
let bridge_state = VisitorState { depth, ..state };
9951012
// FIXME(ctypes): make a check_for_fnptr
996-
let ffi_res = visitor.check_type(state, fn_ptr_ty);
1013+
let ffi_res = visitor.check_type(bridge_state, fn_ptr_ty);
9971014

9981015
self.process_ffi_result(cx, span, ffi_res, fn_mode);
9991016
}

tests/crashes/130310.rs

Lines changed: 0 additions & 15 deletions
This file was deleted.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//@ check-pass
2+
3+
//! this test checks that irregular recursive types do not cause stack overflow in ImproperCTypes
4+
//! Issue: https://github.com/rust-lang/rust/issues/94223
5+
6+
#![deny(improper_ctypes, improper_ctypes_definitions)]
7+
8+
use std::marker::PhantomData;
9+
10+
#[repr(C)]
11+
struct A<T> {
12+
a: *const A<A<T>>, // without a recursion limit, checking this ends up creating checks for
13+
// infinitely deep types the likes of `A<A<A<A<A<A<...>>>>>>`
14+
p: PhantomData<T>,
15+
}
16+
17+
extern "C" {
18+
fn f(a: *const A<()>);
19+
}
20+
21+
fn main() {}

tests/ui/lint/improper-ctypes/lint-non-recursion-limit.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
//@ check-pass
2-
31
//! This test checks that the depth limit of the ImproperCTypes lints counts the depth
42
//! of a type properly.
53
//! Issue: https://github.com/rust-lang/rust/issues/130757
64
75
#![recursion_limit = "5"]
86
#![allow(unused)]
9-
#![deny(improper_ctypes)]
7+
#![deny(improper_ctypes_definitions)]
108

119
#[repr(C)]
1210
struct F1(*const ());
@@ -19,7 +17,7 @@ struct F4(*const ());
1917
#[repr(C)]
2018
struct F5(*const ());
2119
#[repr(C)]
22-
struct F6(*const ());
20+
struct F6([char;8]); //oops!
2321

2422
#[repr(C)]
2523
struct B {
@@ -28,9 +26,10 @@ struct B {
2826
f3: F3,
2927
f4: F4,
3028
f5: F5,
31-
f6: F6,
29+
f6: F6, // when the recursion limit hits, things are assumed safe, so this should error
3230
}
3331

3432
extern "C" fn foo(_: B) {}
33+
//~^ ERROR: uses type `char`
3534

3635
fn main() {}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: `extern` fn uses type `char`, which is not FFI-safe
2+
--> $DIR/lint-non-recursion-limit.rs:32:22
3+
|
4+
LL | extern "C" fn foo(_: B) {}
5+
| ^ not FFI-safe
6+
|
7+
= help: consider using `u32` or `libc::wchar_t` instead
8+
= note: the `char` type has no C equivalent
9+
note: the lint level is defined here
10+
--> $DIR/lint-non-recursion-limit.rs:7:9
11+
|
12+
LL | #![deny(improper_ctypes_definitions)]
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
15+
error: aborting due to 1 previous error
16+

0 commit comments

Comments
 (0)