Rollup merge of rust-lang#75554 - jumbatm:fix-clashing-extern-decl-overflow, r=lcnr

Fix clashing_extern_declarations stack overflow for recursive types.

Fixes rust-lang#75512.

Adds a seen set to `structurally_same_type` to avoid recursing indefinitely for types which contain values of the same type through a pointer or reference.

Author: tmandry

src/librustc_lint/builtin.rs

@@ -29,6 +29,7 @@ use rustc_ast::visit::{FnCtxt, FnKind};
 use rustc_ast::{self as ast, *};
 use rustc_ast_pretty::pprust::{self, expr_to_string};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString};
 use rustc_feature::{deprecated_attributes, AttributeGate, AttributeTemplate, AttributeType};
 use rustc_feature::{GateIssue, Stability};
@@ -2153,123 +2154,152 @@ impl ClashingExternDeclarations {
         b: Ty<'tcx>,
         ckind: CItemKind,
     ) -> bool {
-        debug!("structurally_same_type(cx, a = {:?}, b = {:?})", a, b);
-        let tcx = cx.tcx;
-        if a == b || rustc_middle::ty::TyS::same_type(a, b) {
-            // All nominally-same types are structurally same, too.
-            true
-        } else {
-            // Do a full, depth-first comparison between the two.
-            use rustc_middle::ty::TyKind::*;
-            let a_kind = &a.kind;
-            let b_kind = &b.kind;
-
-            let compare_layouts = |a, b| -> bool {
-                let a_layout = &cx.layout_of(a).unwrap().layout.abi;
-                let b_layout = &cx.layout_of(b).unwrap().layout.abi;
-                debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout);
-                a_layout == b_layout
-            };
+        fn structurally_same_type_impl<'tcx>(
+            seen_types: &mut FxHashSet<(Ty<'tcx>, Ty<'tcx>)>,
+            cx: &LateContext<'tcx>,
+            a: Ty<'tcx>,
+            b: Ty<'tcx>,
+            ckind: CItemKind,
+        ) -> bool {
+            debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b);
+            if !seen_types.insert((a, b)) {
+                // We've encountered a cycle. There's no point going any further -- the types are
+                // structurally the same.
+                return true;
+            }
+            let tcx = cx.tcx;
+            if a == b || rustc_middle::ty::TyS::same_type(a, b) {
+                // All nominally-same types are structurally same, too.
+                true
+            } else {
+                // Do a full, depth-first comparison between the two.
+                use rustc_middle::ty::TyKind::*;
+                let a_kind = &a.kind;
+                let b_kind = &b.kind;
+
+                let compare_layouts = |a, b| -> bool {
+                    let a_layout = &cx.layout_of(a).unwrap().layout.abi;
+                    let b_layout = &cx.layout_of(b).unwrap().layout.abi;
+                    debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout);
+                    a_layout == b_layout
+                };
+
+                #[allow(rustc::usage_of_ty_tykind)]
+                let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| {
+                    kind.is_primitive() || matches!(kind, RawPtr(..) | Ref(..))
+                };
 
-            #[allow(rustc::usage_of_ty_tykind)]
-            let is_primitive_or_pointer =
-                |kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..));
-
-            match (a_kind, b_kind) {
-                (Adt(_, a_substs), Adt(_, b_substs)) => {
-                    let a = a.subst(cx.tcx, a_substs);
-                    let b = b.subst(cx.tcx, b_substs);
-                    debug!("Comparing {:?} and {:?}", a, b);
-
-                    if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) {
-                        // Grab a flattened representation of all fields.
-                        let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter());
-                        let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter());
-                        compare_layouts(a, b)
+                ensure_sufficient_stack(|| {
+                    match (a_kind, b_kind) {
+                        (Adt(a_def, a_substs), Adt(b_def, b_substs)) => {
+                            let a = a.subst(cx.tcx, a_substs);
+                            let b = b.subst(cx.tcx, b_substs);
+                            debug!("Comparing {:?} and {:?}", a, b);
+
+                            // Grab a flattened representation of all fields.
+                            let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter());
+                            let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter());
+                            compare_layouts(a, b)
                             && a_fields.eq_by(
                                 b_fields,
                                 |&ty::FieldDef { did: a_did, .. },
                                  &ty::FieldDef { did: b_did, .. }| {
-                                    Self::structurally_same_type(
+                                    structurally_same_type_impl(
+                                        seen_types,
                                         cx,
                                         tcx.type_of(a_did),
                                         tcx.type_of(b_did),
                                         ckind,
                                     )
                                 },
                             )
-                    } else {
-                        unreachable!()
-                    }
-                }
-                (Array(a_ty, a_const), Array(b_ty, b_const)) => {
-                    // For arrays, we also check the constness of the type.
-                    a_const.val == b_const.val
-                        && Self::structurally_same_type(cx, a_const.ty, b_const.ty, ckind)
-                        && Self::structurally_same_type(cx, a_ty, b_ty, ckind)
-                }
-                (Slice(a_ty), Slice(b_ty)) => Self::structurally_same_type(cx, a_ty, b_ty, ckind),
-                (RawPtr(a_tymut), RawPtr(b_tymut)) => {
-                    a_tymut.mutbl == b_tymut.mutbl
-                        && Self::structurally_same_type(cx, &a_tymut.ty, &b_tymut.ty, ckind)
-                }
-                (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => {
-                    // For structural sameness, we don't need the region to be same.
-                    a_mut == b_mut && Self::structurally_same_type(cx, a_ty, b_ty, ckind)
-                }
-                (FnDef(..), FnDef(..)) => {
-                    let a_poly_sig = a.fn_sig(tcx);
-                    let b_poly_sig = b.fn_sig(tcx);
-
-                    // As we don't compare regions, skip_binder is fine.
-                    let a_sig = a_poly_sig.skip_binder();
-                    let b_sig = b_poly_sig.skip_binder();
-
-                    (a_sig.abi, a_sig.unsafety, a_sig.c_variadic)
-                        == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic)
-                        && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| {
-                            Self::structurally_same_type(cx, a, b, ckind)
-                        })
-                        && Self::structurally_same_type(cx, a_sig.output(), b_sig.output(), ckind)
-                }
-                (Tuple(a_substs), Tuple(b_substs)) => {
-                    a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| {
-                        Self::structurally_same_type(cx, a_ty, b_ty, ckind)
-                    })
-                }
-                // For these, it's not quite as easy to define structural-sameness quite so easily.
-                // For the purposes of this lint, take the conservative approach and mark them as
-                // not structurally same.
-                (Dynamic(..), Dynamic(..))
-                | (Error(..), Error(..))
-                | (Closure(..), Closure(..))
-                | (Generator(..), Generator(..))
-                | (GeneratorWitness(..), GeneratorWitness(..))
-                | (Projection(..), Projection(..))
-                | (Opaque(..), Opaque(..)) => false,
-
-                // These definitely should have been caught above.
-                (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(),
-
-                // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a
-                // non-null field.
-                (Adt(..), other_kind) | (other_kind, Adt(..))
-                    if is_primitive_or_pointer(other_kind) =>
-                {
-                    let (primitive, adt) =
-                        if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) };
-                    if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) {
-                        ty == primitive
-                    } else {
-                        compare_layouts(a, b)
+                        }
+                        (Array(a_ty, a_const), Array(b_ty, b_const)) => {
+                            // For arrays, we also check the constness of the type.
+                            a_const.val == b_const.val
+                                && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind)
+                        }
+                        (Slice(a_ty), Slice(b_ty)) => {
+                            structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind)
+                        }
+                        (RawPtr(a_tymut), RawPtr(b_tymut)) => {
+                            a_tymut.mutbl == b_tymut.mutbl
+                                && structurally_same_type_impl(
+                                    seen_types,
+                                    cx,
+                                    &a_tymut.ty,
+                                    &b_tymut.ty,
+                                    ckind,
+                                )
+                        }
+                        (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => {
+                            // For structural sameness, we don't need the region to be same.
+                            a_mut == b_mut
+                                && structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind)
+                        }
+                        (FnDef(..), FnDef(..)) => {
+                            let a_poly_sig = a.fn_sig(tcx);
+                            let b_poly_sig = b.fn_sig(tcx);
+
+                            // As we don't compare regions, skip_binder is fine.
+                            let a_sig = a_poly_sig.skip_binder();
+                            let b_sig = b_poly_sig.skip_binder();
+
+                            (a_sig.abi, a_sig.unsafety, a_sig.c_variadic)
+                                == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic)
+                                && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| {
+                                    structurally_same_type_impl(seen_types, cx, a, b, ckind)
+                                })
+                                && structurally_same_type_impl(
+                                    seen_types,
+                                    cx,
+                                    a_sig.output(),
+                                    b_sig.output(),
+                                    ckind,
+                                )
+                        }
+                        (Tuple(a_substs), Tuple(b_substs)) => {
+                            a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| {
+                                structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind)
+                            })
+                        }
+                        // For these, it's not quite as easy to define structural-sameness quite so easily.
+                        // For the purposes of this lint, take the conservative approach and mark them as
+                        // not structurally same.
+                        (Dynamic(..), Dynamic(..))
+                        | (Error(..), Error(..))
+                        | (Closure(..), Closure(..))
+                        | (Generator(..), Generator(..))
+                        | (GeneratorWitness(..), GeneratorWitness(..))
+                        | (Projection(..), Projection(..))
+                        | (Opaque(..), Opaque(..)) => false,
+
+                        // These definitely should have been caught above.
+                        (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(),
+
+                        // An Adt and a primitive or pointer type. This can be FFI-safe if non-null
+                        // enum layout optimisation is being applied.
+                        (Adt(..), other_kind) | (other_kind, Adt(..))
+                            if is_primitive_or_pointer(other_kind) =>
+                        {
+                            let (primitive, adt) =
+                                if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) };
+                            if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) {
+                                ty == primitive
+                            } else {
+                                compare_layouts(a, b)
+                            }
+                        }
+                        // Otherwise, just compare the layouts. This may fail to lint for some
+                        // incompatible types, but at the very least, will stop reads into
+                        // uninitialised memory.
+                        _ => compare_layouts(a, b),
                     }
-                }
-                // Otherwise, just compare the layouts. This may fail to lint for some
-                // incompatible types, but at the very least, will stop reads into
-                // uninitialised memory.
-                _ => compare_layouts(a, b),
+                })
             }
         }
+        let mut seen_types = FxHashSet::default();
+        structurally_same_type_impl(&mut seen_types, cx, a, b, ckind)
     }
 }
 

src/test/ui/lint/clashing-extern-fn-recursion.rs

@@ -0,0 +1,119 @@
+// check-pass
+//
+// This tests checks that clashing_extern_declarations handles types that are recursive through a
+// pointer or ref argument. See #75512.
+
+#![crate_type = "lib"]
+
+mod raw_ptr_recursion {
+    mod a {
+        #[repr(C)]
+        struct Pointy {
+            pointy: *const Pointy,
+        }
+
+        extern "C" {
+            fn run_pointy(pointy: Pointy);
+        }
+    }
+    mod b {
+        #[repr(C)]
+        struct Pointy {
+            pointy: *const Pointy,
+        }
+
+        extern "C" {
+            fn run_pointy(pointy: Pointy);
+        }
+    }
+}
+
+mod raw_ptr_recursion_once_removed {
+    mod a {
+        #[repr(C)]
+        struct Pointy1 {
+            pointy_two: *const Pointy2,
+        }
+
+        #[repr(C)]
+        struct Pointy2 {
+            pointy_one: *const Pointy1,
+        }
+
+        extern "C" {
+            fn run_pointy2(pointy: Pointy2);
+        }
+    }
+
+    mod b {
+        #[repr(C)]
+        struct Pointy1 {
+            pointy_two: *const Pointy2,
+        }
+
+        #[repr(C)]
+        struct Pointy2 {
+            pointy_one: *const Pointy1,
+        }
+
+        extern "C" {
+            fn run_pointy2(pointy: Pointy2);
+        }
+    }
+}
+
+mod ref_recursion {
+    mod a {
+        #[repr(C)]
+        struct Reffy<'a> {
+            reffy: &'a Reffy<'a>,
+        }
+
+        extern "C" {
+            fn reffy_recursion(reffy: Reffy);
+        }
+    }
+    mod b {
+        #[repr(C)]
+        struct Reffy<'a> {
+            reffy: &'a Reffy<'a>,
+        }
+
+        extern "C" {
+            fn reffy_recursion(reffy: Reffy);
+        }
+    }
+}
+
+mod ref_recursion_once_removed {
+    mod a {
+        #[repr(C)]
+        struct Reffy1<'a> {
+            reffy: &'a Reffy2<'a>,
+        }
+
+        struct Reffy2<'a> {
+            reffy: &'a Reffy1<'a>,
+        }
+
+        extern "C" {
+            #[allow(improper_ctypes)]
+            fn reffy_once_removed(reffy: Reffy1);
+        }
+    }
+    mod b {
+        #[repr(C)]
+        struct Reffy1<'a> {
+            reffy: &'a Reffy2<'a>,
+        }
+
+        struct Reffy2<'a> {
+            reffy: &'a Reffy1<'a>,
+        }
+
+        extern "C" {
+            #[allow(improper_ctypes)]
+            fn reffy_once_removed(reffy: Reffy1);
+        }
+    }
+}