ImproperCTypes: handle uninhabited types

niacdoial · niacdoial · commit d1ed3c739c28 · 2025-11-01T22:37:22.000+01:00
Add some logic to the type checking to refuse uninhabited types as
arguments,
and treat uninhabited variants of an enum as FFI-safe if at least one
variant is inhabited.
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -391,6 +391,9 @@ lint_improper_ctypes_struct_zst = `{$ty}` contains only zero-sized fields
 lint_improper_ctypes_tuple_help = consider using a struct instead
 lint_improper_ctypes_tuple_reason = tuples have unspecified layout
 
+lint_improper_ctypes_uninhabited_enum = zero-variant enums and other uninhabited types are not allowed in function arguments and static variables
+lint_improper_ctypes_uninhabited_never = the never type (`!`) and other uninhabited types are not allowed in function arguments and static variables
+
 lint_improper_ctypes_union_consider_transparent = `{$ty}` has exactly one non-zero-sized field, consider making it `#[repr(transparent)]` instead
 lint_improper_ctypes_union_fieldless_help = consider adding a member to this union
 lint_improper_ctypes_union_fieldless_reason = this union has no fields
diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs
@@ -14,7 +14,7 @@ use rustc_middle::ty::{
     TypeVisitable, TypeVisitableExt,
 };
 use rustc_session::{declare_lint, declare_lint_pass};
-use rustc_span::def_id::LocalDefId;
+use rustc_span::def_id::{DefId, LocalDefId};
 use rustc_span::{Span, sym};
 use tracing::debug;
 
@@ -360,7 +360,6 @@ impl<'tcx> FfiResult<'tcx> {
     /// if the note at their core reason is one in a provided list.
     /// If the FfiResult is not FfiUnsafe, or if no reasons are plucked,
     /// then return FfiSafe.
-    #[expect(unused)]
     fn take_with_core_note(&mut self, notes: &[DiagMessage]) -> Self {
         match self {
             Self::FfiUnsafe(this) => {
@@ -805,14 +804,30 @@ impl VisitorState {
 /// and ``visit_*`` methods to recurse.
 struct ImproperCTypesVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
+    /// The module id of the item being checked for FFI-safety
+    mod_id: DefId,
     /// To prevent problems with recursive types,
     /// add a types-in-check cache.
     ty_cache: FxHashSet<Ty<'tcx>>,
 }
 
 impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
-    fn new(cx: &'a LateContext<'tcx>) -> Self {
-        Self { cx, ty_cache: FxHashSet::default() }
+    fn new(cx: &'a LateContext<'tcx>, mod_id: DefId) -> Self {
+        Self { cx, mod_id, ty_cache: FxHashSet::default() }
+    }
+
+    /// Checks whether an uninhabited type (one without valid values) is safe-ish to have here.
+    fn visit_uninhabited(&self, state: VisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> {
+        if state.is_in_function_return() {
+            FfiResult::FfiSafe
+        } else {
+            let desc = match ty.kind() {
+                ty::Adt(..) => fluent::lint_improper_ctypes_uninhabited_enum,
+                ty::Never => fluent::lint_improper_ctypes_uninhabited_never,
+                r @ _ => bug!("unexpected ty_kind in uninhabited type handling: {:?}", r),
+            };
+            FfiResult::new_with_reason(ty, desc, None)
+        }
     }
 
     /// Return the right help for Cstring and Cstr-linked unsafety.
@@ -948,6 +963,23 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
             if !matches!(def.adt_kind(), AdtKind::Enum) && def.repr().transparent() {
                 // determine if there is 0 or 1 non-1ZST field, and which it is.
                 // (note: for enums, "transparent" means 1-variant)
+                if !ty.is_inhabited_from(self.cx.tcx, self.mod_id, self.cx.typing_env()) {
+                    // let's consider transparent structs to be maybe unsafe if uninhabited,
+                    // even if that is because of fields otherwise ignored in FFI-safety checks
+                    ffires_accumulator += variant
+                        .fields
+                        .iter()
+                        .map(|field| {
+                            let field_ty = get_type_from_field(self.cx, field, args);
+                            let mut field_res = self.visit_type(state.get_next(ty), field_ty);
+                            field_res.take_with_core_note(&[
+                                fluent::lint_improper_ctypes_uninhabited_enum,
+                                fluent::lint_improper_ctypes_uninhabited_never,
+                            ])
+                        })
+                        .reduce(|r1, r2| r1 + r2)
+                        .unwrap() // if uninhabited, then >0 fields
+                }
                 if let Some(field) = super::transparent_newtype_field(self.cx.tcx, variant) {
                     // Transparent newtypes have at most one non-ZST field which needs to be checked later
                     (false, vec![field])
@@ -1133,8 +1165,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         use FfiResult::*;
 
         if def.variants().is_empty() {
-            // Empty enums are okay... although sort of useless.
-            return FfiSafe;
+            // Empty enums are implicitly handled as the never type:
+            return self.visit_uninhabited(state, ty);
         }
         // Check for a repr() attribute to specify the size of the
         // discriminant.
@@ -1190,18 +1222,33 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                 None,
             )
         } else {
-            let ffires = def
+            // small caveat to checking the variants: we authorise up to n-1 invariants
+            // to be unsafe because uninhabited.
+            // so for now let's isolate those unsafeties
+            let mut variants_uninhabited_ffires = vec![FfiSafe; def.variants().len()];
+
+            let mut ffires = def
                 .variants()
                 .iter()
-                .map(|variant| {
-                    let variant_res = self.visit_variant_fields(state, ty, def, variant, args);
+                .enumerate()
+                .map(|(variant_i, variant)| {
+                    let mut variant_res = self.visit_variant_fields(state, ty, def, variant, args);
+                    variants_uninhabited_ffires[variant_i] = variant_res.take_with_core_note(&[
+                        fluent::lint_improper_ctypes_uninhabited_enum,
+                        fluent::lint_improper_ctypes_uninhabited_never,
+                    ]);
                     // FIXME(ctypes): check that enums allow any (up to all) variants to be phantoms?
                     // (previous code says no, but I don't know why? the problem with phantoms is that they're ZSTs, right?)
                     variant_res.forbid_phantom()
                 })
                 .reduce(|r1, r2| r1 + r2)
                 .unwrap(); // always at least one variant if we hit this branch
 
+            if variants_uninhabited_ffires.iter().all(|res| matches!(res, FfiUnsafe(..))) {
+                // if the enum is uninhabited, because all its variants are uninhabited
+                ffires += variants_uninhabited_ffires.into_iter().reduce(|r1, r2| r1 + r2).unwrap();
+            }
+
             // this enum is visited in the middle of another lint,
             // so we override the "cause type" of the lint
             // (for more detail, see comment in ``visit_struct_union`` before its call to ``ffires.with_overrides``)
@@ -1366,7 +1413,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
 
             ty::Foreign(..) => FfiSafe,
 
-            ty::Never => FfiSafe,
+            ty::Never => self.visit_uninhabited(state, ty),
 
             // While opaque types are checked for earlier, if a projection in a struct field
             // normalizes to an opaque type, then it will reach this branch.
@@ -1443,6 +1490,7 @@ impl<'tcx> ImproperCTypesLint {
             current_depth: usize,
             depths: Vec<usize>,
             decls: Vec<&'tcx hir::FnDecl<'tcx>>,
+            hir_ids: Vec<hir::HirId>,
             tys: Vec<Ty<'tcx>>,
         }
 
@@ -1455,6 +1503,7 @@ impl<'tcx> ImproperCTypesLint {
                 {
                     self.decls.push(*decl);
                     self.depths.push(self.current_depth);
+                    self.hir_ids.push(ty.hir_id);
                 }
 
                 hir::intravisit::walk_ty(self, ty);
@@ -1477,6 +1526,7 @@ impl<'tcx> ImproperCTypesLint {
         }
 
         let mut visitor = FnPtrFinder {
+            hir_ids: Vec::new(),
             tys: Vec::new(),
             decls: Vec::new(),
             depths: Vec::new(),
@@ -1486,12 +1536,15 @@ impl<'tcx> ImproperCTypesLint {
         visitor.visit_ty_unambig(hir_ty);
 
         let all_types = iter::zip(
-            visitor.depths.drain(..),
+            iter::zip(visitor.depths.drain(..), visitor.hir_ids.drain(..)),
             iter::zip(visitor.tys.drain(..), visitor.decls.drain(..)),
         );
-        for (depth, (fn_ptr_ty, decl)) in all_types {
+
+        for ((depth, hir_id), (fn_ptr_ty, decl)) in all_types {
             let mir_sig = get_fn_sig_from_mir_ty(cx, fn_ptr_ty);
-            self.check_foreign_fn(cx, CItemKind::Callback, mir_sig, decl, depth);
+            let mod_id = cx.tcx.parent_module(hir_id).to_def_id();
+
+            self.check_foreign_fn(cx, CItemKind::Callback, mir_sig, decl, mod_id, depth);
         }
     }
 
@@ -1533,9 +1586,10 @@ impl<'tcx> ImproperCTypesLint {
     }
 
     /// Check that an extern "ABI" static variable is of a ffi-safe type.
-    fn check_foreign_static(&mut self, cx: &LateContext<'tcx>, id: hir::OwnerId, span: Span) {
-        let ty = cx.tcx.type_of(id).instantiate_identity();
-        let mut visitor = ImproperCTypesVisitor::new(cx);
+    fn check_foreign_static(&mut self, cx: &LateContext<'tcx>, id: hir::HirId, span: Span) {
+        let ty = cx.tcx.type_of(id.owner).instantiate_identity();
+        let mod_id = cx.tcx.parent_module(id).to_def_id();
+        let mut visitor = ImproperCTypesVisitor::new(cx, mod_id);
         let ffi_res = visitor.check_type(VisitorState::static_var(), ty);
         self.process_ffi_result(cx, span, ffi_res, CItemKind::ImportedExtern);
     }
@@ -1547,22 +1601,23 @@ impl<'tcx> ImproperCTypesLint {
         fn_mode: CItemKind,
         sig: Sig<'tcx>,
         decl: &'tcx hir::FnDecl<'_>,
+        mod_id: DefId,
         depth: usize,
     ) {
         let sig = cx.tcx.instantiate_bound_regions_with_erased(sig);
 
         for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) {
             let mut state = VisitorState::entry_point_from_fnmode(fn_mode, FnPos::Arg);
             state.depth = depth;
-            let mut visitor = ImproperCTypesVisitor::new(cx);
+            let mut visitor = ImproperCTypesVisitor::new(cx, mod_id);
             let ffi_res = visitor.check_type(state, *input_ty);
             self.process_ffi_result(cx, input_hir.span, ffi_res, fn_mode);
         }
 
         if let hir::FnRetTy::Return(ret_hir) = decl.output {
             let mut state = VisitorState::entry_point_from_fnmode(fn_mode, FnPos::Ret);
             state.depth = depth;
-            let mut visitor = ImproperCTypesVisitor::new(cx);
+            let mut visitor = ImproperCTypesVisitor::new(cx, mod_id);
             let ffi_res = visitor.check_type(state, sig.output());
             self.process_ffi_result(cx, ret_hir.span, ffi_res, fn_mode);
         }
@@ -1690,12 +1745,13 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint {
                 // their surroundings, and their type is often declared inline
                 self.check_fn_for_external_abi_fnptr(cx, it.owner_id.def_id, sig.decl);
                 let mir_sig = cx.tcx.fn_sig(it.owner_id.def_id).instantiate_identity();
+                let mod_id = cx.tcx.parent_module_from_def_id(it.owner_id.def_id).to_def_id();
                 if !abi.is_rustic_abi() {
-                    self.check_foreign_fn(cx, CItemKind::ImportedExtern, mir_sig, sig.decl, 0);
+                    self.check_foreign_fn(cx, CItemKind::ImportedExtern, mir_sig, sig.decl, mod_id, 0);
                 }
             }
             hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => {
-                self.check_foreign_static(cx, it.owner_id, ty.span);
+                self.check_foreign_static(cx, it.hir_id(), ty.span);
             }
             hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (),
         }
@@ -1766,9 +1822,10 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint {
         // "the element rendered unsafe" because their unsafety doesn't affect
         // their surroundings, and their type is often declared inline
         self.check_fn_for_external_abi_fnptr(cx, id, decl);
-        let mir_sig = cx.tcx.fn_sig(id).instantiate_identity();
         if !abi.is_rustic_abi() {
-            self.check_foreign_fn(cx, CItemKind::ExportedFunction, mir_sig, decl, 0);
+            let mir_sig = cx.tcx.fn_sig(id).instantiate_identity();
+            let mod_id = cx.tcx.parent_module_from_def_id(id).to_def_id();
+            self.check_foreign_fn(cx, CItemKind::ExportedFunction, mir_sig, decl, mod_id, 0);
         }
     }
 }
diff --git a/tests/ui/lint/improper-ctypes/lint-enum.rs b/tests/ui/lint/improper-ctypes/lint-enum.rs
@@ -78,7 +78,7 @@ struct Field(());
 enum NonExhaustive {}
 
 extern "C" {
-    fn zf(x: Z);
+    fn zf(x: Z); //~ ERROR `extern` block uses type `Z`
     fn uf(x: U); //~ ERROR `extern` block uses type `U`
     fn bf(x: B); //~ ERROR `extern` block uses type `B`
     fn tf(x: T); //~ ERROR `extern` block uses type `T`
diff --git a/tests/ui/lint/improper-ctypes/lint-enum.stderr b/tests/ui/lint/improper-ctypes/lint-enum.stderr
@@ -1,3 +1,21 @@
+error: `extern` block uses type `Z`, which is not FFI-safe
+  --> $DIR/lint-enum.rs:81:14
+   |
+LL |     fn zf(x: Z);
+   |              ^ not FFI-safe
+   |
+   = note: zero-variant enums and other uninhabited types are not allowed in function arguments and static variables
+note: the type is defined here
+  --> $DIR/lint-enum.rs:8:1
+   |
+LL | enum Z {}
+   | ^^^^^^
+note: the lint level is defined here
+  --> $DIR/lint-enum.rs:2:9
+   |
+LL | #![deny(improper_ctypes)]
+   |         ^^^^^^^^^^^^^^^
+
 error: `extern` block uses type `U`, which is not FFI-safe
   --> $DIR/lint-enum.rs:82:14
    |
@@ -11,11 +29,6 @@ note: the type is defined here
    |
 LL | enum U {
    | ^^^^^^
-note: the lint level is defined here
-  --> $DIR/lint-enum.rs:2:9
-   |
-LL | #![deny(improper_ctypes)]
-   |         ^^^^^^^^^^^^^^^
 
 error: `extern` block uses type `B`, which is not FFI-safe
   --> $DIR/lint-enum.rs:83:14
@@ -207,5 +220,5 @@ LL |     fn result_unit_t_e(x: Result<(), ()>);
    = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
    = note: enum has no representation hint
 
-error: aborting due to 21 previous errors
+error: aborting due to 22 previous errors
 
diff --git a/tests/ui/lint/improper-ctypes/lint_uninhabited.rs b/tests/ui/lint/improper-ctypes/lint_uninhabited.rs
@@ -0,0 +1,74 @@
+#![feature(never_type)]
+
+#![allow(dead_code, unused_variables)]
+#![deny(improper_ctypes, improper_ctypes_definitions)]
+
+use std::mem::transmute;
+
+enum Uninhabited{}
+
+#[repr(C)]
+struct AlsoUninhabited{
+    a: Uninhabited,
+    b: i32,
+}
+
+#[repr(C)]
+enum Inhabited{
+    OhNo(Uninhabited),
+    OhYes(i32),
+}
+
+struct EmptyRust;
+
+#[repr(transparent)]
+struct HalfHiddenUninhabited {
+    is_this_a_tuple: (i8,i8),
+    zst_inh: EmptyRust,
+    zst_uninh: !,
+}
+
+extern "C" {
+
+fn bad_entry(e: AlsoUninhabited); //~ ERROR: uses type `AlsoUninhabited`
+fn bad_exit()->AlsoUninhabited;
+
+fn bad0_entry(e: Uninhabited); //~ ERROR: uses type `Uninhabited`
+fn bad0_exit()->Uninhabited;
+
+fn good_entry(e: Inhabited);
+fn good_exit()->Inhabited;
+
+fn never_entry(e:!); //~ ERROR: uses type `!`
+fn never_exit()->!;
+
+}
+
+extern "C" fn impl_bad_entry(e: AlsoUninhabited) {} //~ ERROR: uses type `AlsoUninhabited`
+extern "C" fn impl_bad_exit()->AlsoUninhabited {
+    AlsoUninhabited{
+        a: impl_bad0_exit(),
+        b: 0,
+    }
+}
+
+extern "C" fn impl_bad0_entry(e: Uninhabited) {} //~ ERROR: uses type `Uninhabited`
+extern "C" fn impl_bad0_exit()->Uninhabited {
+    unsafe{transmute(())} //~ WARN: does not permit zero-initialization
+}
+
+extern "C" fn impl_good_entry(e: Inhabited) {}
+extern "C" fn impl_good_exit() -> Inhabited {
+    Inhabited::OhYes(0)
+}
+
+extern "C" fn impl_never_entry(e:!){} //~ ERROR: uses type `!`
+extern "C" fn impl_never_exit()->! {
+    loop{}
+}
+
+extern "C" fn weird_pattern(e:HalfHiddenUninhabited){}
+//~^ ERROR: uses type `HalfHiddenUninhabited`
+
+
+fn main(){}
diff --git a/tests/ui/lint/improper-ctypes/lint_uninhabited.stderr b/tests/ui/lint/improper-ctypes/lint_uninhabited.stderr
diff --git a/tests/ui/structs-enums/foreign-struct.rs b/tests/ui/structs-enums/foreign-struct.rs
