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 1a4286d
Show file tree
Hide file tree
Showing 6 changed files with 63 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 @@ -721,6 +721,9 @@ lint_reason_must_be_string_literal = reason must be a string literal
lint_reason_must_come_last = reason in lint attribute must come last
lint_recursion_limit_reached =
reached recursion limit while {$action}
lint_redundant_import = the item `{$ident}` is imported redundantly
.label_imported_here = the item `{$ident}` is already imported here
.label_defined_here = the item `{$ident}` is already defined here
Expand Down
10 changes: 10 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,12 @@ pub(crate) struct CheckNameUnknownTool<'a> {
#[subdiagnostic]
pub sub: RequestedLevel<'a>,
}

#[derive(Diagnostic)]
#[diag(lint_recursion_limit_reached)]
pub(crate) struct RecursionLimitReached<'tcx> {
pub ty: Ty<'tcx>,
#[primary_span]
pub span: Span,
pub action: String,
}
27 changes: 23 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,13 @@ 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,
action: format!("checking type `{ty}` for FFI safety"),
});
}
}
}

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 1a4286d

Please sign in to comment.