Skip to content

Commit

Permalink
Rollup merge of rust-lang#130758 - compiler-errors:ctype-recursion-li…
Browse files Browse the repository at this point in the history
…mit, r=jieyouxu

Revert "Add recursion limit to FFI safety lint"

It's not necessarily clear if warning when we hit the recursion limit is the right thing to do, first of all.

**More importantly**, this PR was implemented incorrectly in the first place; it was not decrementing the recursion limit when stepping out of a type, so it would trigger when a ctype has more than RECURSION_LIMIT fields *anywhere* in the type's set of recursively reachable fields.

Reverts rust-lang#130598
Reopens rust-lang#130310
Fixes rust-lang#130757
  • Loading branch information
compiler-errors authored Sep 24, 2024
2 parents 54fd38b + 9050b33 commit d7517ec
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 50 deletions.
2 changes: 0 additions & 2 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,6 @@ lint_improper_ctypes_opaque = opaque types have no C equivalent
lint_improper_ctypes_pat_help = consider using the base type instead
lint_improper_ctypes_pat_reason = pattern types have no C equivalent
lint_improper_ctypes_recursion_limit_reached = type is infinitely recursive
lint_improper_ctypes_slice_help = consider using a raw pointer instead
lint_improper_ctypes_slice_reason = slices have no C equivalent
Expand Down
20 changes: 3 additions & 17 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,6 @@ 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> {
Expand Down Expand Up @@ -899,23 +897,12 @@ 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 FfiUnsafe {
ty: acc.base_ty,
reason: fluent::lint_improper_ctypes_recursion_limit_reached,
help: None,
};
}

acc.recursion_depth += 1;

match *ty.kind() {
ty::Adt(def, args) => {
if let Some(boxed) = ty.boxed_ty()
Expand Down Expand Up @@ -1261,8 +1248,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
return;
}

let mut acc =
CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty, recursion_depth: 0 };
let mut acc = CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty };
match self.check_type_for_ffi(&mut acc, ty) {
FfiResult::FfiSafe => {}
FfiResult::FfiPhantom(ty) => {
Expand Down
15 changes: 15 additions & 0 deletions tests/crashes/130310.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@ known-bug: rust-lang/rust#130310

use std::marker::PhantomData;

#[repr(C)]
struct A<T> {
a: *const A<A<T>>,
p: PhantomData<T>,
}

extern "C" {
fn f(a: *const A<()>);
}

fn main() {}
20 changes: 0 additions & 20 deletions tests/ui/lint/improper-types-stack-overflow-130310.rs

This file was deleted.

11 changes: 0 additions & 11 deletions tests/ui/lint/improper-types-stack-overflow-130310.stderr

This file was deleted.

32 changes: 32 additions & 0 deletions tests/ui/lint/lint-ctypes-non-recursion-limit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//@ check-pass

#![recursion_limit = "5"]
#![allow(unused)]
#![deny(improper_ctypes)]

#[repr(C)]
struct F1(*const ());
#[repr(C)]
struct F2(*const ());
#[repr(C)]
struct F3(*const ());
#[repr(C)]
struct F4(*const ());
#[repr(C)]
struct F5(*const ());
#[repr(C)]
struct F6(*const ());

#[repr(C)]
struct B {
f1: F1,
f2: F2,
f3: F3,
f4: F4,
f5: F5,
f6: F6,
}

extern "C" fn foo(_: B) {}

fn main() {}

0 comments on commit d7517ec

Please sign in to comment.