Skip to content

Commit

Permalink
Add recursion limit to FFI safety lint
Browse files Browse the repository at this point in the history
Fixes stack overflow in the case of recursive types
  • Loading branch information
gurry committed Sep 20, 2024
1 parent e2dc1a1 commit 8ea1a53
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 19 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_lint/src/errors.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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,
}
23 changes: 19 additions & 4 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -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()
Expand Down Expand Up @@ -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) => {
Expand All @@ -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 });
}
}
}

Expand Down
15 changes: 0 additions & 15 deletions tests/crashes/130310.rs

This file was deleted.

19 changes: 19 additions & 0 deletions tests/ui/lint/improper-types-stack-overflow-130310.rs
Original file line number Diff line number Diff line change
@@ -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() {}
8 changes: 8 additions & 0 deletions tests/ui/lint/improper-types-stack-overflow-130310.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 8ea1a53

Please sign in to comment.