lint ImproperCTypes: refactor linting architecture (part 3)

compiler/rustc_lint/src/types/improper_ctypes.rs

@@ -349,6 +349,8 @@ struct VisitorState {
     /// Flags describing both the immediate context in which the current Ty is,
     /// linked to how it relates to its parent Ty (or lack thereof).
     outer_ty_kind: OuterTyKind,
+    /// Type recursion depth, to prevent infinite recursion
+    depth: usize,
 }
 
 impl RootUseFlags {
@@ -376,6 +378,7 @@ impl VisitorState {
         VisitorState {
             root_use_flags: self.root_use_flags,
             outer_ty_kind: OuterTyKind::from_ty(current_ty),
+            depth: self.depth + 1,
         }
     }
 
@@ -390,6 +393,7 @@ impl VisitorState {
                 FnPos::Arg => RootUseFlags::ARGUMENT_TY_IN_FNPTR,
             },
             outer_ty_kind: OuterTyKind::from_ty(current_ty),
+            depth: self.depth + 1,
         }
     }
 
@@ -401,12 +405,16 @@ impl VisitorState {
             (CItemKind::Definition, FnPos::Arg) => RootUseFlags::ARGUMENT_TY_IN_DEFINITION,
             (CItemKind::Declaration, FnPos::Arg) => RootUseFlags::ARGUMENT_TY_IN_DECLARATION,
         };
-        VisitorState { root_use_flags: p_flags, outer_ty_kind: OuterTyKind::None }
+        VisitorState { root_use_flags: p_flags, outer_ty_kind: OuterTyKind::None, depth: 0 }
     }
 
     /// Get the proper visitor state for a static variable's type
     fn static_entry_point() -> Self {
-        VisitorState { root_use_flags: RootUseFlags::STATIC_TY, outer_ty_kind: OuterTyKind::None }
+        VisitorState {
+            root_use_flags: RootUseFlags::STATIC_TY,
+            outer_ty_kind: OuterTyKind::None,
+            depth: 0,
+        }
     }
 
     /// Whether the type is used in a function.
@@ -730,9 +738,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
 
         // Protect against infinite recursion, for example
         // `struct S(*mut S);`.
-        // FIXME: A recursion limit is necessary as well, for irregular
-        // recursive types.
-        if !self.cache.insert(ty) {
+        if !(self.cache.insert(ty) && self.cx.tcx.recursion_limit().value_within_limit(state.depth))
+        {
             return FfiSafe;
         }
 
@@ -967,20 +974,25 @@ impl<'tcx> ImproperCTypesLint {
         fn_mode: CItemKind,
     ) {
         struct FnPtrFinder<'tcx> {
+            current_depth: usize,
+            depths: Vec<usize>,
             spans: Vec<Span>,
             tys: Vec<Ty<'tcx>>,
         }
 
         impl<'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'tcx> {
             fn visit_ty(&mut self, ty: &'_ hir::Ty<'_, AmbigArg>) {
                 debug!(?ty);
+                self.current_depth += 1;
                 if let hir::TyKind::FnPtr(hir::FnPtrTy { abi, .. }) = ty.kind
                     && !abi.is_rustic_abi()
                 {
+                    self.depths.push(self.current_depth);
                     self.spans.push(ty.span);
                 }
 
                 hir::intravisit::walk_ty(self, ty);
+                self.current_depth -= 1;
             }
         }
 
@@ -998,16 +1010,25 @@ impl<'tcx> ImproperCTypesLint {
             }
         }
 
-        let mut visitor = FnPtrFinder { spans: Vec::new(), tys: Vec::new() };
+        let mut visitor = FnPtrFinder {
+            spans: Vec::new(),
+            tys: Vec::new(),
+            depths: Vec::new(),
+            current_depth: 0,
+        };
         ty.visit_with(&mut visitor);
         visitor.visit_ty_unambig(hir_ty);
 
-        let all_types = iter::zip(visitor.tys.drain(..), visitor.spans.drain(..));
-        for (fn_ptr_ty, span) in all_types {
+        let all_types = iter::zip(
+            visitor.depths.drain(..),
+            iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)),
+        );
+        for (depth, (fn_ptr_ty, span)) in all_types {
             let fn_ptr_ty = Unnormalized::new_wip(fn_ptr_ty);
             let mut visitor = ImproperCTypesVisitor::new(cx, fn_ptr_ty, fn_mode);
+            let bridge_state = VisitorState { depth, ..state };
             // FIXME(ctypes): make a check_for_fnptr
-            let ffi_res = visitor.check_type(state, fn_ptr_ty);
+            let ffi_res = visitor.check_type(bridge_state, fn_ptr_ty);
 
             self.process_ffi_result(cx, span, ffi_res, fn_mode);
         }

tests/ui/lint/improper-ctypes/ice-irregular-recursive-types.rs

@@ -0,0 +1,21 @@
+//@ check-pass
+
+//! this test checks that irregular recursive types do not cause stack overflow in ImproperCTypes
+//! Issue: https://github.com/rust-lang/rust/issues/94223
+
+#![deny(improper_ctypes, improper_ctypes_definitions)]
+
+use std::marker::PhantomData;
+
+#[repr(C)]
+struct A<T> {
+    a: *const A<A<T>>,  // without a recursion limit, checking this ends up creating checks for
+                        // infinitely deep types the likes of `A<A<A<A<A<A<...>>>>>>`
+    p: PhantomData<T>,
+}
+
+extern "C" {
+    fn f(a: *const A<()>);
+}
+
+fn main() {}

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

@@ -1,8 +1,10 @@
-//@ check-pass
+//! This test checks that the depth limit of the ImproperCTypes lints counts the depth
+//! of a type properly.
+//! Issue: https://github.com/rust-lang/rust/issues/130757
 
 #![recursion_limit = "5"]
 #![allow(unused)]
-#![deny(improper_ctypes)]
+#![deny(improper_ctypes_definitions)]
 
 #[repr(C)]
 struct F1(*const ());
@@ -15,7 +17,7 @@ struct F4(*const ());
 #[repr(C)]
 struct F5(*const ());
 #[repr(C)]
-struct F6(*const ());
+struct F6([char;8]); //oops!
 
 #[repr(C)]
 struct B {
@@ -24,9 +26,10 @@ struct B {
     f3: F3,
     f4: F4,
     f5: F5,
-    f6: F6,
+    f6: F6,  // when the recursion limit hits, things are assumed safe, so this should error
 }
 
 extern "C" fn foo(_: B) {}
+//~^ ERROR: uses type `char`
 
 fn main() {}

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

@@ -0,0 +1,16 @@
+error: `extern` fn uses type `char`, which is not FFI-safe
+  --> $DIR/lint-non-recursion-limit.rs:32:22
+   |
+LL | extern "C" fn foo(_: B) {}
+   |                      ^ not FFI-safe
+   |
+   = help: consider using `u32` or `libc::wchar_t` instead
+   = note: the `char` type has no C equivalent
+note: the lint level is defined here
+  --> $DIR/lint-non-recursion-limit.rs:7:9
+   |
+LL | #![deny(improper_ctypes_definitions)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+