Add recursion limit to FFI safety lint

Fixes stack overflow in the case of recursive types

Author: gurry

compiler/rustc_lint/messages.ftl

@@ -276,6 +276,9 @@ lint_extern_without_abi = extern declarations without an explicit ABI are deprec
     .label = ABI should be specified here
     .help = the default ABI is {$default_abi}
 
+lint_ffi_safety_recursion_limit_reached =
+    reached recursion limit while checking type `{$ty}` for FFI safety
+
 lint_for_loops_over_fallibles =
     for loop over {$article} `{$ref_prefix}{$ty}`. This is more readably written as an `if let` statement
     .suggestion = consider using `if let` to clear intent

compiler/rustc_lint/src/errors.rs

@@ -1,6 +1,7 @@
 use rustc_errors::codes::*;
 use rustc_errors::{Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic};
 use rustc_macros::{Diagnostic, Subdiagnostic};
+use rustc_middle::ty::Ty;
 use rustc_session::lint::Level;
 use rustc_span::{Span, Symbol};
 
@@ -110,3 +111,11 @@ pub(crate) struct CheckNameUnknownTool<'a> {
     #[subdiagnostic]
     pub sub: RequestedLevel<'a>,
 }
+
+#[derive(Diagnostic)]
+#[diag(lint_ffi_safety_recursion_limit_reached)]
+pub(crate) struct RecursionLimitReached<'tcx> {
+    pub ty: Ty<'tcx>,
+    #[primary_span]
+    pub span: Span,
+}

compiler/rustc_lint/src/types.rs

@@ -18,6 +18,7 @@ use rustc_target::spec::abi::Abi as SpecAbi;
 use tracing::debug;
 use {rustc_ast as ast, rustc_attr as attr, rustc_hir as hir};
 
+use crate::errors::RecursionLimitReached;
 use crate::lints::{
     AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion,
     AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad,
@@ -991,12 +992,15 @@ struct CTypesVisitorState<'tcx> {
     /// The original type being checked, before we recursed
     /// to any other types it contains.
     base_ty: Ty<'tcx>,
+    /// Number of times we recursed while checking the type
+    recursion_depth: usize,
 }
 
 enum FfiResult<'tcx> {
     FfiSafe,
     FfiPhantom(Ty<'tcx>),
     FfiUnsafe { ty: Ty<'tcx>, reason: DiagMessage, help: Option<DiagMessage> },
+    RecursionLimitReached,
 }
 
 pub(crate) fn nonnull_optimization_guaranteed<'tcx>(
@@ -1270,7 +1274,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                 // `()` fields are FFI-safe!
                 FfiUnsafe { ty, .. } if ty.is_unit() => false,
                 FfiPhantom(..) => true,
-                r @ FfiUnsafe { .. } => return r,
+                r => return r,
             }
         }
 
@@ -1296,12 +1300,19 @@ 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 !acc.cache.insert(ty) {
             return FfiSafe;
         }
 
+        // Additional recursion check for more complex types like
+        // `struct A<T> { v: *const A<A<T>>, ... }` for which the
+        // cache check above won't be enough (fixes #130310)
+        if !tcx.recursion_limit().value_within_limit(acc.recursion_depth) {
+            return RecursionLimitReached;
+        }
+
+        acc.recursion_depth += 1;
+
         match *ty.kind() {
             ty::Adt(def, args) => {
                 if let Some(boxed) = ty.boxed_ty()
@@ -1644,7 +1655,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
             return;
         }
 
-        let mut acc = CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty };
+        let mut acc =
+            CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty, recursion_depth: 0 };
         match self.check_type_for_ffi(&mut acc, ty) {
             FfiResult::FfiSafe => {}
             FfiResult::FfiPhantom(ty) => {
@@ -1658,6 +1670,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
             FfiResult::FfiUnsafe { ty, reason, help } => {
                 self.emit_ffi_unsafe_type_lint(ty, sp, reason, help);
             }
+            FfiResult::RecursionLimitReached => {
+                self.cx.tcx.dcx().emit_err(RecursionLimitReached { ty, span: sp });
+            }
         }
     }
 

tests/crashes/130310.rs

@@ -0,0 +1,19 @@
+// Regression test for #130310
+// Tests that we do not fall into infinite
+// recursion while checking FFI safety of
+// recursive types like `A<T>` below
+
+use std::marker::PhantomData;
+
+#[repr(C)]
+struct A<T> {
+    a: *const A<A<T>>, // Recursive because of this field
+    p: PhantomData<T>,
+}
+
+extern "C" {
+    fn f(a: *const A<()>);
+    //~^ ERROR reached recursion limit while checking type `*const A<()>` for FFI safety
+}
+
+fn main() {}

tests/ui/lint/improper-types-stack-overflow-130310.rs

@@ -0,0 +1,8 @@
+error: reached recursion limit while checking type `*const A<()>` for FFI safety
+  --> $DIR/improper-types-stack-overflow-130310.rs:15:13
+   |
+LL |     fn f(a: *const A<()>);
+   |             ^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+