From 0c0e764ff7d0708b90baa1386c6cbe75d70bce7d Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 22 Aug 2025 23:07:40 +0200 Subject: [PATCH 1/4] ImproperCTypes: move code and tests into proper directories --- compiler/rustc_lint/src/types.rs | 867 +----------------- .../rustc_lint/src/types/improper_ctypes.rs | 861 ++++++++++++++++- .../allow-phantomdata-in-ffi.rs | 0 .../lint-113436-1.rs} | 0 .../lint-113436-1.stderr} | 10 +- .../lint-73249-2.rs} | 0 .../lint-73249-2.stderr} | 4 +- .../lint-73249-3.rs} | 0 .../lint-73249-3.stderr} | 4 +- .../lint-73249-5.rs} | 0 .../lint-73249-5.stderr} | 4 +- .../lint-73251-1.rs} | 0 .../lint-73251-1.stderr} | 4 +- .../lint-73251-2.rs} | 0 .../lint-73251-2.stderr} | 4 +- .../lint-94223.rs} | 0 .../lint-94223.stderr} | 34 +- .../lint-cstr.rs} | 0 .../lint-cstr.stderr} | 20 +- .../lint/{ => improper-ctypes}/lint-ctypes.rs | 0 .../{ => improper-ctypes}/lint-ctypes.stderr | 0 .../lint-enum.rs} | 0 .../lint-enum.stderr} | 50 +- .../lint-fn.rs} | 0 .../lint-fn.stderr} | 40 +- .../lint-non-recursion-limit.rs} | 0 .../lint-option-nonnull-unsized.rs} | 0 .../lint-option-nonnull-unsized.stderr} | 4 +- .../mustpass-113436.rs} | 0 .../mustpass-113900.rs} | 0 .../mustpass-134060.rs} | 0 .../mustpass-134060.stderr} | 2 +- .../mustpass-66202.rs} | 0 .../mustpass-73249-1.rs} | 0 .../mustpass-73249-4.rs} | 0 .../mustpass-73249.rs} | 0 .../mustpass-73251.rs} | 0 .../mustpass-73747.rs} | 0 .../repr-rust-is-undefined.rs | 0 .../repr-rust-is-undefined.stderr | 0 40 files changed, 959 insertions(+), 949 deletions(-) rename tests/ui/lint/{improper_ctypes => improper-ctypes}/allow-phantomdata-in-ffi.rs (100%) rename tests/ui/lint/{lint-ctypes-113436-1.rs => improper-ctypes/lint-113436-1.rs} (100%) rename tests/ui/lint/{lint-ctypes-113436-1.stderr => improper-ctypes/lint-113436-1.stderr} (82%) rename tests/ui/lint/{lint-ctypes-73249-2.rs => improper-ctypes/lint-73249-2.rs} (100%) rename tests/ui/lint/{lint-ctypes-73249-2.stderr => improper-ctypes/lint-73249-2.stderr} (81%) rename tests/ui/lint/{lint-ctypes-73249-3.rs => improper-ctypes/lint-73249-3.rs} (100%) rename tests/ui/lint/{lint-ctypes-73249-5.stderr => improper-ctypes/lint-73249-3.stderr} (81%) rename tests/ui/lint/{lint-ctypes-73249-5.rs => improper-ctypes/lint-73249-5.rs} (100%) rename tests/ui/lint/{lint-ctypes-73249-3.stderr => improper-ctypes/lint-73249-5.stderr} (81%) rename tests/ui/lint/{lint-ctypes-73251-1.rs => improper-ctypes/lint-73251-1.rs} (100%) rename tests/ui/lint/{lint-ctypes-73251-1.stderr => improper-ctypes/lint-73251-1.stderr} (82%) rename tests/ui/lint/{lint-ctypes-73251-2.rs => improper-ctypes/lint-73251-2.rs} (100%) rename tests/ui/lint/{lint-ctypes-73251-2.stderr => improper-ctypes/lint-73251-2.stderr} (82%) rename tests/ui/lint/{lint-ctypes-94223.rs => improper-ctypes/lint-94223.rs} (100%) rename tests/ui/lint/{lint-ctypes-94223.stderr => improper-ctypes/lint-94223.stderr} (86%) rename tests/ui/lint/{lint-ctypes-cstr.rs => improper-ctypes/lint-cstr.rs} (100%) rename tests/ui/lint/{lint-ctypes-cstr.stderr => improper-ctypes/lint-cstr.stderr} (88%) rename tests/ui/lint/{ => improper-ctypes}/lint-ctypes.rs (100%) rename tests/ui/lint/{ => improper-ctypes}/lint-ctypes.stderr (100%) rename tests/ui/lint/{lint-ctypes-enum.rs => improper-ctypes/lint-enum.rs} (100%) rename tests/ui/lint/{lint-ctypes-enum.stderr => improper-ctypes/lint-enum.stderr} (90%) rename tests/ui/lint/{lint-ctypes-fn.rs => improper-ctypes/lint-fn.rs} (100%) rename tests/ui/lint/{lint-ctypes-fn.stderr => improper-ctypes/lint-fn.stderr} (88%) rename tests/ui/lint/{lint-ctypes-non-recursion-limit.rs => improper-ctypes/lint-non-recursion-limit.rs} (100%) rename tests/ui/lint/{lint-ctypes-option-nonnull-unsized.rs => improper-ctypes/lint-option-nonnull-unsized.rs} (100%) rename tests/ui/lint/{lint-ctypes-option-nonnull-unsized.stderr => improper-ctypes/lint-option-nonnull-unsized.stderr} (83%) rename tests/ui/lint/{lint-ctypes-113436.rs => improper-ctypes/mustpass-113436.rs} (100%) rename tests/ui/lint/{lint-ctypes-113900.rs => improper-ctypes/mustpass-113900.rs} (100%) rename tests/ui/lint/{improper_ctypes_definitions_ice_134060.rs => improper-ctypes/mustpass-134060.rs} (100%) rename tests/ui/lint/{improper_ctypes_definitions_ice_134060.stderr => improper-ctypes/mustpass-134060.stderr} (85%) rename tests/ui/lint/{lint-ctypes-66202.rs => improper-ctypes/mustpass-66202.rs} (100%) rename tests/ui/lint/{lint-ctypes-73249-1.rs => improper-ctypes/mustpass-73249-1.rs} (100%) rename tests/ui/lint/{lint-ctypes-73249-4.rs => improper-ctypes/mustpass-73249-4.rs} (100%) rename tests/ui/lint/{lint-ctypes-73249.rs => improper-ctypes/mustpass-73249.rs} (100%) rename tests/ui/lint/{lint-ctypes-73251.rs => improper-ctypes/mustpass-73251.rs} (100%) rename tests/ui/lint/{lint-ctypes-73747.rs => improper-ctypes/mustpass-73747.rs} (100%) rename tests/ui/lint/{improper_ctypes => improper-ctypes}/repr-rust-is-undefined.rs (100%) rename tests/ui/lint/{improper_ctypes => improper-ctypes}/repr-rust-is-undefined.stderr (100%) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index f8a692313f030..056ecb70af8a4 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -1,35 +1,30 @@ use std::iter; -use std::ops::ControlFlow; -use rustc_abi::{BackendRepr, TagEncoding, VariantIdx, Variants, WrappingRange}; -use rustc_data_structures::fx::FxHashSet; -use rustc_errors::DiagMessage; -use rustc_hir::intravisit::VisitorExt; -use rustc_hir::{AmbigArg, Expr, ExprKind, HirId, LangItem}; +use rustc_abi::{BackendRepr, TagEncoding, Variants, WrappingRange}; +use rustc_hir::{Expr, ExprKind, HirId, LangItem}; use rustc_middle::bug; use rustc_middle::ty::layout::{LayoutOf, SizeSkeleton}; -use rustc_middle::ty::{ - self, Adt, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, - TypeVisitableExt, -}; +use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt}; use rustc_session::{declare_lint, declare_lint_pass, impl_lint_pass}; -use rustc_span::def_id::LocalDefId; use rustc_span::{Span, Symbol, sym}; use tracing::debug; use {rustc_ast as ast, rustc_hir as hir}; -mod improper_ctypes; +mod improper_ctypes; // these filed do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations +pub(crate) use improper_ctypes::{ + CItemKind, ImproperCTypesDeclarations, ImproperCTypesDefinitions, +}; use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, AmbiguousWidePointerComparisonsAddrSuggestion, AmbiguousWidePointerComparisonsCastSuggestion, AmbiguousWidePointerComparisonsExpectSuggestion, AtomicOrderingFence, AtomicOrderingLoad, - AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons, + AtomicOrderingStore, InvalidAtomicOrderingDiag, InvalidNanComparisons, InvalidNanComparisonsSuggestion, UnpredictableFunctionPointerComparisons, - UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, UsesPowerAlignment, + UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, VariantSizeDifferencesDiag, }; -use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; +use crate::{LateContext, LateLintPass, LintContext}; mod literal; @@ -690,144 +685,6 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits { } } -declare_lint! { - /// The `improper_ctypes` lint detects incorrect use of types in foreign - /// modules. - /// - /// ### Example - /// - /// ```rust - /// unsafe extern "C" { - /// static STATIC: String; - /// } - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// The compiler has several checks to verify that types used in `extern` - /// blocks are safe and follow certain rules to ensure proper - /// compatibility with the foreign interfaces. This lint is issued when it - /// detects a probable mistake in a definition. The lint usually should - /// provide a description of the issue, along with possibly a hint on how - /// to resolve it. - IMPROPER_CTYPES, - Warn, - "proper use of libc types in foreign modules" -} - -declare_lint_pass!(ImproperCTypesDeclarations => [IMPROPER_CTYPES]); - -declare_lint! { - /// The `improper_ctypes_definitions` lint detects incorrect use of - /// [`extern` function] definitions. - /// - /// [`extern` function]: https://doc.rust-lang.org/reference/items/functions.html#extern-function-qualifier - /// - /// ### Example - /// - /// ```rust - /// # #![allow(unused)] - /// pub extern "C" fn str_type(p: &str) { } - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// There are many parameter and return types that may be specified in an - /// `extern` function that are not compatible with the given ABI. This - /// lint is an alert that these types should not be used. The lint usually - /// should provide a description of the issue, along with possibly a hint - /// on how to resolve it. - IMPROPER_CTYPES_DEFINITIONS, - Warn, - "proper use of libc types in foreign item definitions" -} - -declare_lint! { - /// The `uses_power_alignment` lint detects specific `repr(C)` - /// aggregates on AIX. - /// In its platform C ABI, AIX uses the "power" (as in PowerPC) alignment - /// rule (detailed in https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes#alignment), - /// which can also be set for XLC by `#pragma align(power)` or - /// `-qalign=power`. Aggregates with a floating-point type as the - /// recursively first field (as in "at offset 0") modify the layout of - /// *subsequent* fields of the associated structs to use an alignment value - /// where the floating-point type is aligned on a 4-byte boundary. - /// - /// Effectively, subsequent floating-point fields act as-if they are `repr(packed(4))`. This - /// would be unsound to do in a `repr(C)` type without all the restrictions that come with - /// `repr(packed)`. Rust instead chooses a layout that maintains soundness of Rust code, at the - /// expense of incompatibility with C code. - /// - /// ### Example - /// - /// ```rust,ignore (fails on non-powerpc64-ibm-aix) - /// #[repr(C)] - /// pub struct Floats { - /// a: f64, - /// b: u8, - /// c: f64, - /// } - /// ``` - /// - /// This will produce: - /// - /// ```text - /// warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type - /// --> :5:3 - /// | - /// 5 | c: f64, - /// | ^^^^^^ - /// | - /// = note: `#[warn(uses_power_alignment)]` on by default - /// ``` - /// - /// ### Explanation - /// - /// The power alignment rule specifies that the above struct has the - /// following alignment: - /// - offset_of!(Floats, a) == 0 - /// - offset_of!(Floats, b) == 8 - /// - offset_of!(Floats, c) == 12 - /// - /// However, Rust currently aligns `c` at `offset_of!(Floats, c) == 16`. - /// Using offset 12 would be unsound since `f64` generally must be 8-aligned on this target. - /// Thus, a warning is produced for the above struct. - USES_POWER_ALIGNMENT, - Warn, - "Structs do not follow the power alignment rule under repr(C)" -} - -declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS, USES_POWER_ALIGNMENT]); - -#[derive(Clone, Copy)] -pub(crate) enum CItemKind { - Declaration, - Definition, -} - -struct ImproperCTypesVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - mode: CItemKind, -} - -/// Accumulator for recursive ffi type checking -struct CTypesVisitorState<'tcx> { - cache: FxHashSet>, - /// The original type being checked, before we recursed - /// to any other types it contains. - base_ty: Ty<'tcx>, -} - -enum FfiResult<'tcx> { - FfiSafe, - FfiPhantom(Ty<'tcx>), - FfiUnsafe { ty: Ty<'tcx>, reason: DiagMessage, help: Option }, -} - pub(crate) fn nonnull_optimization_guaranteed<'tcx>( tcx: TyCtxt<'tcx>, def: ty::AdtDef<'tcx>, @@ -1076,710 +933,6 @@ fn get_nullable_type_from_pat<'tcx>( } } -impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { - /// Check if the type is array and emit an unsafe type lint. - fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { - if let ty::Array(..) = ty.kind() { - self.emit_ffi_unsafe_type_lint( - ty, - sp, - fluent::lint_improper_ctypes_array_reason, - Some(fluent::lint_improper_ctypes_array_help), - ); - true - } else { - false - } - } - - /// Checks if the given field's type is "ffi-safe". - fn check_field_type_for_ffi( - &self, - acc: &mut CTypesVisitorState<'tcx>, - field: &ty::FieldDef, - args: GenericArgsRef<'tcx>, - ) -> FfiResult<'tcx> { - let field_ty = field.ty(self.cx.tcx, args); - let field_ty = self - .cx - .tcx - .try_normalize_erasing_regions(self.cx.typing_env(), field_ty) - .unwrap_or(field_ty); - self.check_type_for_ffi(acc, field_ty) - } - - /// Checks if the given `VariantDef`'s field types are "ffi-safe". - fn check_variant_for_ffi( - &self, - acc: &mut CTypesVisitorState<'tcx>, - ty: Ty<'tcx>, - def: ty::AdtDef<'tcx>, - variant: &ty::VariantDef, - args: GenericArgsRef<'tcx>, - ) -> FfiResult<'tcx> { - use FfiResult::*; - let transparent_with_all_zst_fields = if def.repr().transparent() { - if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) { - // Transparent newtypes have at most one non-ZST field which needs to be checked.. - match self.check_field_type_for_ffi(acc, field, args) { - FfiUnsafe { ty, .. } if ty.is_unit() => (), - r => return r, - } - - false - } else { - // ..or have only ZST fields, which is FFI-unsafe (unless those fields are all - // `PhantomData`). - true - } - } else { - false - }; - - // We can't completely trust `repr(C)` markings, so make sure the fields are actually safe. - let mut all_phantom = !variant.fields.is_empty(); - for field in &variant.fields { - all_phantom &= match self.check_field_type_for_ffi(acc, field, args) { - FfiSafe => false, - // `()` fields are FFI-safe! - FfiUnsafe { ty, .. } if ty.is_unit() => false, - FfiPhantom(..) => true, - r @ FfiUnsafe { .. } => return r, - } - } - - if all_phantom { - FfiPhantom(ty) - } else if transparent_with_all_zst_fields { - FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None } - } else { - FfiSafe - } - } - - /// Checks if the given type is "ffi-safe" (has a stable, well-defined - /// representation which can be exported to C code). - fn check_type_for_ffi( - &self, - acc: &mut CTypesVisitorState<'tcx>, - ty: Ty<'tcx>, - ) -> FfiResult<'tcx> { - use FfiResult::*; - - let tcx = self.cx.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; - } - - match *ty.kind() { - ty::Adt(def, args) => { - if let Some(boxed) = ty.boxed_ty() - && matches!(self.mode, CItemKind::Definition) - { - if boxed.is_sized(tcx, self.cx.typing_env()) { - return FfiSafe; - } else { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_box, - help: None, - }; - } - } - if def.is_phantom_data() { - return FfiPhantom(ty); - } - match def.adt_kind() { - AdtKind::Struct | AdtKind::Union => { - if let Some(sym::cstring_type | sym::cstr_type) = - tcx.get_diagnostic_name(def.did()) - && !acc.base_ty.is_mutable_ptr() - { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_cstr_reason, - help: Some(fluent::lint_improper_ctypes_cstr_help), - }; - } - - if !def.repr().c() && !def.repr().transparent() { - return FfiUnsafe { - ty, - reason: if def.is_struct() { - fluent::lint_improper_ctypes_struct_layout_reason - } else { - fluent::lint_improper_ctypes_union_layout_reason - }, - help: if def.is_struct() { - Some(fluent::lint_improper_ctypes_struct_layout_help) - } else { - Some(fluent::lint_improper_ctypes_union_layout_help) - }, - }; - } - - if def.non_enum_variant().field_list_has_applicable_non_exhaustive() { - return FfiUnsafe { - ty, - reason: if def.is_struct() { - fluent::lint_improper_ctypes_struct_non_exhaustive - } else { - fluent::lint_improper_ctypes_union_non_exhaustive - }, - help: None, - }; - } - - if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe { - ty, - reason: if def.is_struct() { - fluent::lint_improper_ctypes_struct_fieldless_reason - } else { - fluent::lint_improper_ctypes_union_fieldless_reason - }, - help: if def.is_struct() { - Some(fluent::lint_improper_ctypes_struct_fieldless_help) - } else { - Some(fluent::lint_improper_ctypes_union_fieldless_help) - }, - }; - } - - self.check_variant_for_ffi(acc, ty, def, def.non_enum_variant(), args) - } - AdtKind::Enum => { - if def.variants().is_empty() { - // Empty enums are okay... although sort of useless. - return FfiSafe; - } - // Check for a repr() attribute to specify the size of the - // discriminant. - if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() - { - // Special-case types like `Option` and `Result` - if let Some(ty) = - repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty, self.mode) - { - return self.check_type_for_ffi(acc, ty); - } - - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_enum_repr_reason, - help: Some(fluent::lint_improper_ctypes_enum_repr_help), - }; - } - - use improper_ctypes::check_non_exhaustive_variant; - - let non_exhaustive = def.variant_list_has_applicable_non_exhaustive(); - // Check the contained variants. - let ret = def.variants().iter().try_for_each(|variant| { - check_non_exhaustive_variant(non_exhaustive, variant) - .map_break(|reason| FfiUnsafe { ty, reason, help: None })?; - - match self.check_variant_for_ffi(acc, ty, def, variant, args) { - FfiSafe => ControlFlow::Continue(()), - r => ControlFlow::Break(r), - } - }); - if let ControlFlow::Break(result) = ret { - return result; - } - - FfiSafe - } - } - } - - ty::Char => FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_char_reason, - help: Some(fluent::lint_improper_ctypes_char_help), - }, - - // It's just extra invariants on the type that you need to uphold, - // but only the base type is relevant for being representable in FFI. - ty::Pat(base, ..) => self.check_type_for_ffi(acc, base), - - // Primitive types with a stable representation. - ty::Bool | ty::Int(..) | ty::Uint(..) | ty::Float(..) | ty::Never => FfiSafe, - - ty::Slice(_) => FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_slice_reason, - help: Some(fluent::lint_improper_ctypes_slice_help), - }, - - ty::Dynamic(..) => { - FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_dyn, help: None } - } - - ty::Str => FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_str_reason, - help: Some(fluent::lint_improper_ctypes_str_help), - }, - - ty::Tuple(..) => FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_tuple_reason, - help: Some(fluent::lint_improper_ctypes_tuple_help), - }, - - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) - if { - matches!(self.mode, CItemKind::Definition) - && ty.is_sized(self.cx.tcx, self.cx.typing_env()) - } => - { - FfiSafe - } - - ty::RawPtr(ty, _) - if match ty.kind() { - ty::Tuple(tuple) => tuple.is_empty(), - _ => false, - } => - { - FfiSafe - } - - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(acc, ty), - - ty::Array(inner_ty, _) => self.check_type_for_ffi(acc, inner_ty), - - ty::FnPtr(sig_tys, hdr) => { - let sig = sig_tys.with(hdr); - if sig.abi().is_rustic_abi() { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_fnptr_reason, - help: Some(fluent::lint_improper_ctypes_fnptr_help), - }; - } - - let sig = tcx.instantiate_bound_regions_with_erased(sig); - for arg in sig.inputs() { - match self.check_type_for_ffi(acc, *arg) { - FfiSafe => {} - r => return r, - } - } - - let ret_ty = sig.output(); - if ret_ty.is_unit() { - return FfiSafe; - } - - self.check_type_for_ffi(acc, ret_ty) - } - - ty::Foreign(..) => FfiSafe, - - // 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. - ty::Alias(ty::Opaque, ..) => { - FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_opaque, help: None } - } - - // `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe, - // so they are currently ignored for the purposes of this lint. - ty::Param(..) | ty::Alias(ty::Projection | ty::Inherent, ..) - if matches!(self.mode, CItemKind::Definition) => - { - FfiSafe - } - - ty::UnsafeBinder(_) => todo!("FIXME(unsafe_binder)"), - - ty::Param(..) - | ty::Alias(ty::Projection | ty::Inherent | ty::Free, ..) - | ty::Infer(..) - | ty::Bound(..) - | ty::Error(_) - | ty::Closure(..) - | ty::CoroutineClosure(..) - | ty::Coroutine(..) - | ty::CoroutineWitness(..) - | ty::Placeholder(..) - | ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty), - } - } - - fn emit_ffi_unsafe_type_lint( - &mut self, - ty: Ty<'tcx>, - sp: Span, - note: DiagMessage, - help: Option, - ) { - let lint = match self.mode { - CItemKind::Declaration => IMPROPER_CTYPES, - CItemKind::Definition => IMPROPER_CTYPES_DEFINITIONS, - }; - let desc = match self.mode { - CItemKind::Declaration => "block", - CItemKind::Definition => "fn", - }; - let span_note = if let ty::Adt(def, _) = ty.kind() - && let Some(sp) = self.cx.tcx.hir_span_if_local(def.did()) - { - Some(sp) - } else { - None - }; - self.cx.emit_span_lint( - lint, - sp, - ImproperCTypes { ty, desc, label: sp, help, note, span_note }, - ); - } - - fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { - struct ProhibitOpaqueTypes; - impl<'tcx> ty::TypeVisitor> for ProhibitOpaqueTypes { - type Result = ControlFlow>; - - fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { - if !ty.has_opaque_types() { - return ControlFlow::Continue(()); - } - - if let ty::Alias(ty::Opaque, ..) = ty.kind() { - ControlFlow::Break(ty) - } else { - ty.super_visit_with(self) - } - } - } - - if let Some(ty) = self - .cx - .tcx - .try_normalize_erasing_regions(self.cx.typing_env(), ty) - .unwrap_or(ty) - .visit_with(&mut ProhibitOpaqueTypes) - .break_value() - { - self.emit_ffi_unsafe_type_lint(ty, sp, fluent::lint_improper_ctypes_opaque, None); - true - } else { - false - } - } - - fn check_type_for_ffi_and_report_errors( - &mut self, - sp: Span, - ty: Ty<'tcx>, - is_static: bool, - is_return_type: bool, - ) { - if self.check_for_opaque_ty(sp, ty) { - // We've already emitted an error due to an opaque type. - return; - } - - let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.typing_env(), ty).unwrap_or(ty); - - // C doesn't really support passing arrays by value - the only way to pass an array by value - // is through a struct. So, first test that the top level isn't an array, and then - // recursively check the types inside. - if !is_static && self.check_for_array_ty(sp, ty) { - return; - } - - // Don't report FFI errors for unit return types. This check exists here, and not in - // the caller (where it would make more sense) so that normalization has definitely - // happened. - if is_return_type && ty.is_unit() { - return; - } - - let mut acc = CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty }; - match self.check_type_for_ffi(&mut acc, ty) { - FfiResult::FfiSafe => {} - FfiResult::FfiPhantom(ty) => { - self.emit_ffi_unsafe_type_lint( - ty, - sp, - fluent::lint_improper_ctypes_only_phantomdata, - None, - ); - } - FfiResult::FfiUnsafe { ty, reason, help } => { - self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); - } - } - } - - /// Check if a function's argument types and result type are "ffi-safe". - /// - /// For a external ABI function, argument types and the result type are walked to find fn-ptr - /// types that have external ABIs, as these still need checked. - fn check_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { - let sig = self.cx.tcx.fn_sig(def_id).instantiate_identity(); - let sig = self.cx.tcx.instantiate_bound_regions_with_erased(sig); - - for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { - for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(input_hir, *input_ty) { - self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, false); - } - } - - if let hir::FnRetTy::Return(ret_hir) = decl.output { - for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(ret_hir, sig.output()) { - self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, true); - } - } - } - - /// Check if a function's argument types and result type are "ffi-safe". - fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { - let sig = self.cx.tcx.fn_sig(def_id).instantiate_identity(); - let sig = self.cx.tcx.instantiate_bound_regions_with_erased(sig); - - for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { - self.check_type_for_ffi_and_report_errors(input_hir.span, *input_ty, false, false); - } - - if let hir::FnRetTy::Return(ret_hir) = decl.output { - self.check_type_for_ffi_and_report_errors(ret_hir.span, sig.output(), false, true); - } - } - - fn check_foreign_static(&mut self, id: hir::OwnerId, span: Span) { - let ty = self.cx.tcx.type_of(id).instantiate_identity(); - self.check_type_for_ffi_and_report_errors(span, ty, true, false); - } - - /// Find any fn-ptr types with external ABIs in `ty`. - /// - /// For example, `Option` returns `extern "C" fn()` - fn find_fn_ptr_ty_with_external_abi( - &self, - hir_ty: &hir::Ty<'tcx>, - ty: Ty<'tcx>, - ) -> Vec<(Ty<'tcx>, Span)> { - struct FnPtrFinder<'tcx> { - spans: Vec, - tys: Vec>, - } - - impl<'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'tcx> { - fn visit_ty(&mut self, ty: &'_ hir::Ty<'_, AmbigArg>) { - debug!(?ty); - if let hir::TyKind::FnPtr(hir::FnPtrTy { abi, .. }) = ty.kind - && !abi.is_rustic_abi() - { - self.spans.push(ty.span); - } - - hir::intravisit::walk_ty(self, ty) - } - } - - impl<'tcx> ty::TypeVisitor> for FnPtrFinder<'tcx> { - type Result = (); - - fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { - if let ty::FnPtr(_, hdr) = ty.kind() - && !hdr.abi.is_rustic_abi() - { - self.tys.push(ty); - } - - ty.super_visit_with(self) - } - } - - let mut visitor = FnPtrFinder { spans: Vec::new(), tys: Vec::new() }; - ty.visit_with(&mut visitor); - visitor.visit_ty_unambig(hir_ty); - - iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)).collect() - } -} - -impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations { - fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, it: &hir::ForeignItem<'tcx>) { - let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration }; - let abi = cx.tcx.hir_get_foreign_abi(it.hir_id()); - - match it.kind { - hir::ForeignItemKind::Fn(sig, _, _) => { - if abi.is_rustic_abi() { - vis.check_fn(it.owner_id.def_id, sig.decl) - } else { - vis.check_foreign_fn(it.owner_id.def_id, sig.decl); - } - } - hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => { - vis.check_foreign_static(it.owner_id, ty.span); - } - hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (), - } - } -} - -impl ImproperCTypesDefinitions { - fn check_ty_maybe_containing_foreign_fnptr<'tcx>( - &mut self, - cx: &LateContext<'tcx>, - hir_ty: &'tcx hir::Ty<'_>, - ty: Ty<'tcx>, - ) { - let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; - for (fn_ptr_ty, span) in vis.find_fn_ptr_ty_with_external_abi(hir_ty, ty) { - vis.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, true, false); - } - } - - fn check_arg_for_power_alignment<'tcx>( - &mut self, - cx: &LateContext<'tcx>, - ty: Ty<'tcx>, - ) -> bool { - assert!(cx.tcx.sess.target.os == "aix"); - // Structs (under repr(C)) follow the power alignment rule if: - // - the first field of the struct is a floating-point type that - // is greater than 4-bytes, or - // - the first field of the struct is an aggregate whose - // recursively first field is a floating-point type greater than - // 4 bytes. - if ty.is_floating_point() && ty.primitive_size(cx.tcx).bytes() > 4 { - return true; - } else if let Adt(adt_def, _) = ty.kind() - && adt_def.is_struct() - && adt_def.repr().c() - && !adt_def.repr().packed() - && adt_def.repr().align.is_none() - { - let struct_variant = adt_def.variant(VariantIdx::ZERO); - // Within a nested struct, all fields are examined to correctly - // report if any fields after the nested struct within the - // original struct are misaligned. - for struct_field in &struct_variant.fields { - let field_ty = cx.tcx.type_of(struct_field.did).instantiate_identity(); - if self.check_arg_for_power_alignment(cx, field_ty) { - return true; - } - } - } - return false; - } - - fn check_struct_for_power_alignment<'tcx>( - &mut self, - cx: &LateContext<'tcx>, - item: &'tcx hir::Item<'tcx>, - ) { - let adt_def = cx.tcx.adt_def(item.owner_id.to_def_id()); - // repr(C) structs also with packed or aligned representation - // should be ignored. - if adt_def.repr().c() - && !adt_def.repr().packed() - && adt_def.repr().align.is_none() - && cx.tcx.sess.target.os == "aix" - && !adt_def.all_fields().next().is_none() - { - let struct_variant_data = item.expect_struct().2; - for field_def in struct_variant_data.fields().iter().skip(1) { - // Struct fields (after the first field) are checked for the - // power alignment rule, as fields after the first are likely - // to be the fields that are misaligned. - let def_id = field_def.def_id; - let ty = cx.tcx.type_of(def_id).instantiate_identity(); - if self.check_arg_for_power_alignment(cx, ty) { - cx.emit_span_lint(USES_POWER_ALIGNMENT, field_def.span, UsesPowerAlignment); - } - } - } - } -} - -/// `ImproperCTypesDefinitions` checks items outside of foreign items (e.g. stuff that isn't in -/// `extern "C" { }` blocks): -/// -/// - `extern "" fn` definitions are checked in the same way as the -/// `ImproperCtypesDeclarations` visitor checks functions if `` is external (e.g. "C"). -/// - All other items which contain types (e.g. other functions, struct definitions, etc) are -/// checked for extern fn-ptrs with external ABIs. -impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { - fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { - match item.kind { - hir::ItemKind::Static(_, _, ty, _) - | hir::ItemKind::Const(_, _, ty, _) - | hir::ItemKind::TyAlias(_, _, ty) => { - self.check_ty_maybe_containing_foreign_fnptr( - cx, - ty, - cx.tcx.type_of(item.owner_id).instantiate_identity(), - ); - } - // See `check_fn`.. - hir::ItemKind::Fn { .. } => {} - // Structs are checked based on if they follow the power alignment - // rule (under repr(C)). - hir::ItemKind::Struct(..) => { - self.check_struct_for_power_alignment(cx, item); - } - // See `check_field_def`.. - hir::ItemKind::Union(..) | hir::ItemKind::Enum(..) => {} - // Doesn't define something that can contain a external type to be checked. - hir::ItemKind::Impl(..) - | hir::ItemKind::TraitAlias(..) - | hir::ItemKind::Trait(..) - | hir::ItemKind::GlobalAsm { .. } - | hir::ItemKind::ForeignMod { .. } - | hir::ItemKind::Mod(..) - | hir::ItemKind::Macro(..) - | hir::ItemKind::Use(..) - | hir::ItemKind::ExternCrate(..) => {} - } - } - - fn check_field_def(&mut self, cx: &LateContext<'tcx>, field: &'tcx hir::FieldDef<'tcx>) { - self.check_ty_maybe_containing_foreign_fnptr( - cx, - field.ty, - cx.tcx.type_of(field.def_id).instantiate_identity(), - ); - } - - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - kind: hir::intravisit::FnKind<'tcx>, - decl: &'tcx hir::FnDecl<'_>, - _: &'tcx hir::Body<'_>, - _: Span, - id: LocalDefId, - ) { - use hir::intravisit::FnKind; - - let abi = match kind { - FnKind::ItemFn(_, _, header, ..) => header.abi, - FnKind::Method(_, sig, ..) => sig.header.abi, - _ => return, - }; - - let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; - if abi.is_rustic_abi() { - vis.check_fn(id, decl); - } else { - vis.check_foreign_fn(id, decl); - } - } -} - declare_lint_pass!(VariantSizeDifferences => [VARIANT_SIZE_DIFFERENCES]); impl<'tcx> LateLintPass<'tcx> for VariantSizeDifferences { diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 13afa540afcf6..6f732fc2a50c8 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -1,10 +1,140 @@ +use std::iter; use std::ops::ControlFlow; +use rustc_abi::VariantIdx; +use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; use rustc_hir::def::CtorKind; -use rustc_middle::ty; +use rustc_hir::intravisit::VisitorExt; +use rustc_hir::{self as hir, AmbigArg}; +use rustc_middle::bug; +use rustc_middle::ty::{ + self, Adt, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, + TypeVisitableExt, +}; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::def_id::LocalDefId; +use rustc_span::{Span, sym}; +use tracing::debug; + +use super::repr_nullable_ptr; +use crate::lints::{ImproperCTypes, UsesPowerAlignment}; +use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; + + +declare_lint! { + /// The `improper_ctypes` lint detects incorrect use of types in foreign + /// modules. + /// + /// ### Example + /// + /// ```rust + /// unsafe extern "C" { + /// static STATIC: String; + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// The compiler has several checks to verify that types used in `extern` + /// blocks are safe and follow certain rules to ensure proper + /// compatibility with the foreign interfaces. This lint is issued when it + /// detects a probable mistake in a definition. The lint usually should + /// provide a description of the issue, along with possibly a hint on how + /// to resolve it. + IMPROPER_CTYPES, + Warn, + "proper use of libc types in foreign modules" +} + +declare_lint_pass!(ImproperCTypesDeclarations => [IMPROPER_CTYPES]); + +declare_lint! { + /// The `improper_ctypes_definitions` lint detects incorrect use of + /// [`extern` function] definitions. + /// + /// [`extern` function]: https://doc.rust-lang.org/reference/items/functions.html#extern-function-qualifier + /// + /// ### Example + /// + /// ```rust + /// # #![allow(unused)] + /// pub extern "C" fn str_type(p: &str) { } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// There are many parameter and return types that may be specified in an + /// `extern` function that are not compatible with the given ABI. This + /// lint is an alert that these types should not be used. The lint usually + /// should provide a description of the issue, along with possibly a hint + /// on how to resolve it. + IMPROPER_CTYPES_DEFINITIONS, + Warn, + "proper use of libc types in foreign item definitions" +} + +declare_lint! { + /// The `uses_power_alignment` lint detects specific `repr(C)` + /// aggregates on AIX. + /// In its platform C ABI, AIX uses the "power" (as in PowerPC) alignment + /// rule (detailed in https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes#alignment), + /// which can also be set for XLC by `#pragma align(power)` or + /// `-qalign=power`. Aggregates with a floating-point type as the + /// recursively first field (as in "at offset 0") modify the layout of + /// *subsequent* fields of the associated structs to use an alignment value + /// where the floating-point type is aligned on a 4-byte boundary. + /// + /// Effectively, subsequent floating-point fields act as-if they are `repr(packed(4))`. This + /// would be unsound to do in a `repr(C)` type without all the restrictions that come with + /// `repr(packed)`. Rust instead chooses a layout that maintains soundness of Rust code, at the + /// expense of incompatibility with C code. + /// + /// ### Example + /// + /// ```rust,ignore (fails on non-powerpc64-ibm-aix) + /// #[repr(C)] + /// pub struct Floats { + /// a: f64, + /// b: u8, + /// c: f64, + /// } + /// ``` + /// + /// This will produce: + /// + /// ```text + /// warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type + /// --> :5:3 + /// | + /// 5 | c: f64, + /// | ^^^^^^ + /// | + /// = note: `#[warn(uses_power_alignment)]` on by default + /// ``` + /// + /// ### Explanation + /// + /// The power alignment rule specifies that the above struct has the + /// following alignment: + /// - offset_of!(Floats, a) == 0 + /// - offset_of!(Floats, b) == 8 + /// - offset_of!(Floats, c) == 12 + /// + /// However, Rust currently aligns `c` at `offset_of!(Floats, c) == 16`. + /// Using offset 12 would be unsound since `f64` generally must be 8-aligned on this target. + /// Thus, a warning is produced for the above struct. + USES_POWER_ALIGNMENT, + Warn, + "Structs do not follow the power alignment rule under repr(C)" +} + +declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS, USES_POWER_ALIGNMENT]); -use crate::fluent_generated as fluent; /// Check a variant of a non-exhaustive enum for improper ctypes /// @@ -41,3 +171,730 @@ fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool { // CtorKind::Const means a "unit" ctor !matches!(variant.ctor_kind(), Some(CtorKind::Const)) } + +#[derive(Clone, Copy)] +pub(crate) enum CItemKind { + Declaration, + Definition, +} + +enum FfiResult<'tcx> { + FfiSafe, + FfiPhantom(Ty<'tcx>), + FfiUnsafe { ty: Ty<'tcx>, reason: DiagMessage, help: Option }, +} + +struct ImproperCTypesVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + mode: CItemKind, +} + +/// Accumulator for recursive ffi type checking +struct CTypesVisitorState<'tcx> { + cache: FxHashSet>, + /// The original type being checked, before we recursed + /// to any other types it contains. + base_ty: Ty<'tcx>, +} + +impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { + /// Check if the type is array and emit an unsafe type lint. + fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { + if let ty::Array(..) = ty.kind() { + self.emit_ffi_unsafe_type_lint( + ty, + sp, + fluent::lint_improper_ctypes_array_reason, + Some(fluent::lint_improper_ctypes_array_help), + ); + true + } else { + false + } + } + + /// Checks if the given field's type is "ffi-safe". + fn check_field_type_for_ffi( + &self, + acc: &mut CTypesVisitorState<'tcx>, + field: &ty::FieldDef, + args: GenericArgsRef<'tcx>, + ) -> FfiResult<'tcx> { + let field_ty = field.ty(self.cx.tcx, args); + let field_ty = self + .cx + .tcx + .try_normalize_erasing_regions(self.cx.typing_env(), field_ty) + .unwrap_or(field_ty); + self.check_type_for_ffi(acc, field_ty) + } + + /// Checks if the given `VariantDef`'s field types are "ffi-safe". + fn check_variant_for_ffi( + &self, + acc: &mut CTypesVisitorState<'tcx>, + ty: Ty<'tcx>, + def: ty::AdtDef<'tcx>, + variant: &ty::VariantDef, + args: GenericArgsRef<'tcx>, + ) -> FfiResult<'tcx> { + use FfiResult::*; + let transparent_with_all_zst_fields = if def.repr().transparent() { + 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.. + match self.check_field_type_for_ffi(acc, field, args) { + FfiUnsafe { ty, .. } if ty.is_unit() => (), + r => return r, + } + + false + } else { + // ..or have only ZST fields, which is FFI-unsafe (unless those fields are all + // `PhantomData`). + true + } + } else { + false + }; + + // We can't completely trust `repr(C)` markings, so make sure the fields are actually safe. + let mut all_phantom = !variant.fields.is_empty(); + for field in &variant.fields { + all_phantom &= match self.check_field_type_for_ffi(acc, field, args) { + FfiSafe => false, + // `()` fields are FFI-safe! + FfiUnsafe { ty, .. } if ty.is_unit() => false, + FfiPhantom(..) => true, + r @ FfiUnsafe { .. } => return r, + } + } + + if all_phantom { + FfiPhantom(ty) + } else if transparent_with_all_zst_fields { + FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None } + } else { + FfiSafe + } + } + + /// Checks if the given type is "ffi-safe" (has a stable, well-defined + /// representation which can be exported to C code). + fn check_type_for_ffi( + &self, + acc: &mut CTypesVisitorState<'tcx>, + ty: Ty<'tcx>, + ) -> FfiResult<'tcx> { + use FfiResult::*; + + let tcx = self.cx.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; + } + + match *ty.kind() { + ty::Adt(def, args) => { + if let Some(boxed) = ty.boxed_ty() + && matches!(self.mode, CItemKind::Definition) + { + if boxed.is_sized(tcx, self.cx.typing_env()) { + return FfiSafe; + } else { + return FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_box, + help: None, + }; + } + } + if def.is_phantom_data() { + return FfiPhantom(ty); + } + match def.adt_kind() { + AdtKind::Struct | AdtKind::Union => { + if let Some(sym::cstring_type | sym::cstr_type) = + tcx.get_diagnostic_name(def.did()) + && !acc.base_ty.is_mutable_ptr() + { + return FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_cstr_reason, + help: Some(fluent::lint_improper_ctypes_cstr_help), + }; + } + + if !def.repr().c() && !def.repr().transparent() { + return FfiUnsafe { + ty, + reason: if def.is_struct() { + fluent::lint_improper_ctypes_struct_layout_reason + } else { + fluent::lint_improper_ctypes_union_layout_reason + }, + help: if def.is_struct() { + Some(fluent::lint_improper_ctypes_struct_layout_help) + } else { + Some(fluent::lint_improper_ctypes_union_layout_help) + }, + }; + } + + if def.non_enum_variant().field_list_has_applicable_non_exhaustive() { + return FfiUnsafe { + ty, + reason: if def.is_struct() { + fluent::lint_improper_ctypes_struct_non_exhaustive + } else { + fluent::lint_improper_ctypes_union_non_exhaustive + }, + help: None, + }; + } + + if def.non_enum_variant().fields.is_empty() { + return FfiUnsafe { + ty, + reason: if def.is_struct() { + fluent::lint_improper_ctypes_struct_fieldless_reason + } else { + fluent::lint_improper_ctypes_union_fieldless_reason + }, + help: if def.is_struct() { + Some(fluent::lint_improper_ctypes_struct_fieldless_help) + } else { + Some(fluent::lint_improper_ctypes_union_fieldless_help) + }, + }; + } + + self.check_variant_for_ffi(acc, ty, def, def.non_enum_variant(), args) + } + AdtKind::Enum => { + if def.variants().is_empty() { + // Empty enums are okay... although sort of useless. + return FfiSafe; + } + // Check for a repr() attribute to specify the size of the + // discriminant. + if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() + { + // Special-case types like `Option` and `Result` + if let Some(ty) = + repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty, self.mode) + { + return self.check_type_for_ffi(acc, ty); + } + + return FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_enum_repr_reason, + help: Some(fluent::lint_improper_ctypes_enum_repr_help), + }; + } + + let non_exhaustive = def.variant_list_has_applicable_non_exhaustive(); + // Check the contained variants. + let ret = def.variants().iter().try_for_each(|variant| { + check_non_exhaustive_variant(non_exhaustive, variant) + .map_break(|reason| FfiUnsafe { ty, reason, help: None })?; + + match self.check_variant_for_ffi(acc, ty, def, variant, args) { + FfiSafe => ControlFlow::Continue(()), + r => ControlFlow::Break(r), + } + }); + if let ControlFlow::Break(result) = ret { + return result; + } + + FfiSafe + } + } + } + + ty::Char => FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_char_reason, + help: Some(fluent::lint_improper_ctypes_char_help), + }, + + // It's just extra invariants on the type that you need to uphold, + // but only the base type is relevant for being representable in FFI. + ty::Pat(base, ..) => self.check_type_for_ffi(acc, base), + + // Primitive types with a stable representation. + ty::Bool | ty::Int(..) | ty::Uint(..) | ty::Float(..) | ty::Never => FfiSafe, + + ty::Slice(_) => FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_slice_reason, + help: Some(fluent::lint_improper_ctypes_slice_help), + }, + + ty::Dynamic(..) => { + FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_dyn, help: None } + } + + ty::Str => FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_str_reason, + help: Some(fluent::lint_improper_ctypes_str_help), + }, + + ty::Tuple(..) => FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_tuple_reason, + help: Some(fluent::lint_improper_ctypes_tuple_help), + }, + + ty::RawPtr(ty, _) | ty::Ref(_, ty, _) + if { + matches!(self.mode, CItemKind::Definition) + && ty.is_sized(self.cx.tcx, self.cx.typing_env()) + } => + { + FfiSafe + } + + ty::RawPtr(ty, _) + if match ty.kind() { + ty::Tuple(tuple) => tuple.is_empty(), + _ => false, + } => + { + FfiSafe + } + + ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(acc, ty), + + ty::Array(inner_ty, _) => self.check_type_for_ffi(acc, inner_ty), + + ty::FnPtr(sig_tys, hdr) => { + let sig = sig_tys.with(hdr); + if sig.abi().is_rustic_abi() { + return FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_fnptr_reason, + help: Some(fluent::lint_improper_ctypes_fnptr_help), + }; + } + + let sig = tcx.instantiate_bound_regions_with_erased(sig); + for arg in sig.inputs() { + match self.check_type_for_ffi(acc, *arg) { + FfiSafe => {} + r => return r, + } + } + + let ret_ty = sig.output(); + if ret_ty.is_unit() { + return FfiSafe; + } + + self.check_type_for_ffi(acc, ret_ty) + } + + ty::Foreign(..) => FfiSafe, + + // 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. + ty::Alias(ty::Opaque, ..) => { + FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_opaque, help: None } + } + + // `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe, + // so they are currently ignored for the purposes of this lint. + ty::Param(..) | ty::Alias(ty::Projection | ty::Inherent, ..) + if matches!(self.mode, CItemKind::Definition) => + { + FfiSafe + } + + ty::UnsafeBinder(_) => todo!("FIXME(unsafe_binder)"), + + ty::Param(..) + | ty::Alias(ty::Projection | ty::Inherent | ty::Free, ..) + | ty::Infer(..) + | ty::Bound(..) + | ty::Error(_) + | ty::Closure(..) + | ty::CoroutineClosure(..) + | ty::Coroutine(..) + | ty::CoroutineWitness(..) + | ty::Placeholder(..) + | ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty), + } + } + + fn emit_ffi_unsafe_type_lint( + &mut self, + ty: Ty<'tcx>, + sp: Span, + note: DiagMessage, + help: Option, + ) { + let lint = match self.mode { + CItemKind::Declaration => IMPROPER_CTYPES, + CItemKind::Definition => IMPROPER_CTYPES_DEFINITIONS, + }; + let desc = match self.mode { + CItemKind::Declaration => "block", + CItemKind::Definition => "fn", + }; + let span_note = if let ty::Adt(def, _) = ty.kind() + && let Some(sp) = self.cx.tcx.hir_span_if_local(def.did()) + { + Some(sp) + } else { + None + }; + self.cx.emit_span_lint( + lint, + sp, + ImproperCTypes { ty, desc, label: sp, help, note, span_note }, + ); + } + + fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { + struct ProhibitOpaqueTypes; + impl<'tcx> ty::TypeVisitor> for ProhibitOpaqueTypes { + type Result = ControlFlow>; + + fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { + if !ty.has_opaque_types() { + return ControlFlow::Continue(()); + } + + if let ty::Alias(ty::Opaque, ..) = ty.kind() { + ControlFlow::Break(ty) + } else { + ty.super_visit_with(self) + } + } + } + + if let Some(ty) = self + .cx + .tcx + .try_normalize_erasing_regions(self.cx.typing_env(), ty) + .unwrap_or(ty) + .visit_with(&mut ProhibitOpaqueTypes) + .break_value() + { + self.emit_ffi_unsafe_type_lint(ty, sp, fluent::lint_improper_ctypes_opaque, None); + true + } else { + false + } + } + + fn check_type_for_ffi_and_report_errors( + &mut self, + sp: Span, + ty: Ty<'tcx>, + is_static: bool, + is_return_type: bool, + ) { + if self.check_for_opaque_ty(sp, ty) { + // We've already emitted an error due to an opaque type. + return; + } + + let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.typing_env(), ty).unwrap_or(ty); + + // C doesn't really support passing arrays by value - the only way to pass an array by value + // is through a struct. So, first test that the top level isn't an array, and then + // recursively check the types inside. + if !is_static && self.check_for_array_ty(sp, ty) { + return; + } + + // Don't report FFI errors for unit return types. This check exists here, and not in + // the caller (where it would make more sense) so that normalization has definitely + // happened. + if is_return_type && ty.is_unit() { + return; + } + + let mut acc = CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty }; + match self.check_type_for_ffi(&mut acc, ty) { + FfiResult::FfiSafe => {} + FfiResult::FfiPhantom(ty) => { + self.emit_ffi_unsafe_type_lint( + ty, + sp, + fluent::lint_improper_ctypes_only_phantomdata, + None, + ); + } + FfiResult::FfiUnsafe { ty, reason, help } => { + self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); + } + } + } + + /// Check if a function's argument types and result type are "ffi-safe". + /// + /// For a external ABI function, argument types and the result type are walked to find fn-ptr + /// types that have external ABIs, as these still need checked. + fn check_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { + let sig = self.cx.tcx.fn_sig(def_id).instantiate_identity(); + let sig = self.cx.tcx.instantiate_bound_regions_with_erased(sig); + + for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { + for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(input_hir, *input_ty) { + self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, false); + } + } + + if let hir::FnRetTy::Return(ret_hir) = decl.output { + for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(ret_hir, sig.output()) { + self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, true); + } + } + } + + /// Check if a function's argument types and result type are "ffi-safe". + fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { + let sig = self.cx.tcx.fn_sig(def_id).instantiate_identity(); + let sig = self.cx.tcx.instantiate_bound_regions_with_erased(sig); + + for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { + self.check_type_for_ffi_and_report_errors(input_hir.span, *input_ty, false, false); + } + + if let hir::FnRetTy::Return(ret_hir) = decl.output { + self.check_type_for_ffi_and_report_errors(ret_hir.span, sig.output(), false, true); + } + } + + fn check_foreign_static(&mut self, id: hir::OwnerId, span: Span) { + let ty = self.cx.tcx.type_of(id).instantiate_identity(); + self.check_type_for_ffi_and_report_errors(span, ty, true, false); + } + + /// Find any fn-ptr types with external ABIs in `ty`. + /// + /// For example, `Option` returns `extern "C" fn()` + fn find_fn_ptr_ty_with_external_abi( + &self, + hir_ty: &hir::Ty<'tcx>, + ty: Ty<'tcx>, + ) -> Vec<(Ty<'tcx>, Span)> { + struct FnPtrFinder<'tcx> { + spans: Vec, + tys: Vec>, + } + + impl<'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'tcx> { + fn visit_ty(&mut self, ty: &'_ hir::Ty<'_, AmbigArg>) { + debug!(?ty); + if let hir::TyKind::FnPtr(hir::FnPtrTy { abi, .. }) = ty.kind + && !abi.is_rustic_abi() + { + self.spans.push(ty.span); + } + + hir::intravisit::walk_ty(self, ty) + } + } + + impl<'tcx> ty::TypeVisitor> for FnPtrFinder<'tcx> { + type Result = (); + + fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { + if let ty::FnPtr(_, hdr) = ty.kind() + && !hdr.abi.is_rustic_abi() + { + self.tys.push(ty); + } + + ty.super_visit_with(self) + } + } + + let mut visitor = FnPtrFinder { spans: Vec::new(), tys: Vec::new() }; + ty.visit_with(&mut visitor); + visitor.visit_ty_unambig(hir_ty); + + iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)).collect() + } +} + +impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations { + fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, it: &hir::ForeignItem<'tcx>) { + let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration }; + let abi = cx.tcx.hir_get_foreign_abi(it.hir_id()); + + match it.kind { + hir::ForeignItemKind::Fn(sig, _, _) => { + if abi.is_rustic_abi() { + vis.check_fn(it.owner_id.def_id, sig.decl) + } else { + vis.check_foreign_fn(it.owner_id.def_id, sig.decl); + } + } + hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => { + vis.check_foreign_static(it.owner_id, ty.span); + } + hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (), + } + } +} + +impl ImproperCTypesDefinitions { + fn check_ty_maybe_containing_foreign_fnptr<'tcx>( + &mut self, + cx: &LateContext<'tcx>, + hir_ty: &'tcx hir::Ty<'_>, + ty: Ty<'tcx>, + ) { + let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; + for (fn_ptr_ty, span) in vis.find_fn_ptr_ty_with_external_abi(hir_ty, ty) { + vis.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, true, false); + } + } + + fn check_arg_for_power_alignment<'tcx>( + &mut self, + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + ) -> bool { + assert!(cx.tcx.sess.target.os == "aix"); + // Structs (under repr(C)) follow the power alignment rule if: + // - the first field of the struct is a floating-point type that + // is greater than 4-bytes, or + // - the first field of the struct is an aggregate whose + // recursively first field is a floating-point type greater than + // 4 bytes. + if ty.is_floating_point() && ty.primitive_size(cx.tcx).bytes() > 4 { + return true; + } else if let Adt(adt_def, _) = ty.kind() + && adt_def.is_struct() + && adt_def.repr().c() + && !adt_def.repr().packed() + && adt_def.repr().align.is_none() + { + let struct_variant = adt_def.variant(VariantIdx::ZERO); + // Within a nested struct, all fields are examined to correctly + // report if any fields after the nested struct within the + // original struct are misaligned. + for struct_field in &struct_variant.fields { + let field_ty = cx.tcx.type_of(struct_field.did).instantiate_identity(); + if self.check_arg_for_power_alignment(cx, field_ty) { + return true; + } + } + } + return false; + } + + fn check_struct_for_power_alignment<'tcx>( + &mut self, + cx: &LateContext<'tcx>, + item: &'tcx hir::Item<'tcx>, + ) { + let adt_def = cx.tcx.adt_def(item.owner_id.to_def_id()); + // repr(C) structs also with packed or aligned representation + // should be ignored. + if adt_def.repr().c() + && !adt_def.repr().packed() + && adt_def.repr().align.is_none() + && cx.tcx.sess.target.os == "aix" + && !adt_def.all_fields().next().is_none() + { + let struct_variant_data = item.expect_struct().2; + for field_def in struct_variant_data.fields().iter().skip(1) { + // Struct fields (after the first field) are checked for the + // power alignment rule, as fields after the first are likely + // to be the fields that are misaligned. + let def_id = field_def.def_id; + let ty = cx.tcx.type_of(def_id).instantiate_identity(); + if self.check_arg_for_power_alignment(cx, ty) { + cx.emit_span_lint(USES_POWER_ALIGNMENT, field_def.span, UsesPowerAlignment); + } + } + } + } +} + +/// `ImproperCTypesDefinitions` checks items outside of foreign items (e.g. stuff that isn't in +/// `extern "C" { }` blocks): +/// +/// - `extern "" fn` definitions are checked in the same way as the +/// `ImproperCtypesDeclarations` visitor checks functions if `` is external (e.g. "C"). +/// - All other items which contain types (e.g. other functions, struct definitions, etc) are +/// checked for extern fn-ptrs with external ABIs. +impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { + match item.kind { + hir::ItemKind::Static(_, _, ty, _) + | hir::ItemKind::Const(_, _, ty, _) + | hir::ItemKind::TyAlias(_, _, ty) => { + self.check_ty_maybe_containing_foreign_fnptr( + cx, + ty, + cx.tcx.type_of(item.owner_id).instantiate_identity(), + ); + } + // See `check_fn`.. + hir::ItemKind::Fn { .. } => {} + // Structs are checked based on if they follow the power alignment + // rule (under repr(C)). + hir::ItemKind::Struct(..) => { + self.check_struct_for_power_alignment(cx, item); + } + // See `check_field_def`.. + hir::ItemKind::Union(..) | hir::ItemKind::Enum(..) => {} + // Doesn't define something that can contain a external type to be checked. + hir::ItemKind::Impl(..) + | hir::ItemKind::TraitAlias(..) + | hir::ItemKind::Trait(..) + | hir::ItemKind::GlobalAsm { .. } + | hir::ItemKind::ForeignMod { .. } + | hir::ItemKind::Mod(..) + | hir::ItemKind::Macro(..) + | hir::ItemKind::Use(..) + | hir::ItemKind::ExternCrate(..) => {} + } + } + + fn check_field_def(&mut self, cx: &LateContext<'tcx>, field: &'tcx hir::FieldDef<'tcx>) { + self.check_ty_maybe_containing_foreign_fnptr( + cx, + field.ty, + cx.tcx.type_of(field.def_id).instantiate_identity(), + ); + } + + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: hir::intravisit::FnKind<'tcx>, + decl: &'tcx hir::FnDecl<'_>, + _: &'tcx hir::Body<'_>, + _: Span, + id: LocalDefId, + ) { + use hir::intravisit::FnKind; + + let abi = match kind { + FnKind::ItemFn(_, _, header, ..) => header.abi, + FnKind::Method(_, sig, ..) => sig.header.abi, + _ => return, + }; + + let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; + if abi.is_rustic_abi() { + vis.check_fn(id, decl); + } else { + vis.check_foreign_fn(id, decl); + } + } +} diff --git a/tests/ui/lint/improper_ctypes/allow-phantomdata-in-ffi.rs b/tests/ui/lint/improper-ctypes/allow-phantomdata-in-ffi.rs similarity index 100% rename from tests/ui/lint/improper_ctypes/allow-phantomdata-in-ffi.rs rename to tests/ui/lint/improper-ctypes/allow-phantomdata-in-ffi.rs diff --git a/tests/ui/lint/lint-ctypes-113436-1.rs b/tests/ui/lint/improper-ctypes/lint-113436-1.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-113436-1.rs rename to tests/ui/lint/improper-ctypes/lint-113436-1.rs diff --git a/tests/ui/lint/lint-ctypes-113436-1.stderr b/tests/ui/lint/improper-ctypes/lint-113436-1.stderr similarity index 82% rename from tests/ui/lint/lint-ctypes-113436-1.stderr rename to tests/ui/lint/improper-ctypes/lint-113436-1.stderr index 7b63043f05756..f01dc3b6e0d1e 100644 --- a/tests/ui/lint/lint-ctypes-113436-1.stderr +++ b/tests/ui/lint/improper-ctypes/lint-113436-1.stderr @@ -1,5 +1,5 @@ error: `extern` fn uses type `NotSafe`, which is not FFI-safe - --> $DIR/lint-ctypes-113436-1.rs:22:22 + --> $DIR/lint-113436-1.rs:22:22 | LL | extern "C" fn bar(x: Bar) -> Bar { | ^^^ not FFI-safe @@ -7,18 +7,18 @@ LL | extern "C" fn bar(x: Bar) -> Bar { = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here - --> $DIR/lint-ctypes-113436-1.rs:13:1 + --> $DIR/lint-113436-1.rs:13:1 | LL | struct NotSafe(u32); | ^^^^^^^^^^^^^^ note: the lint level is defined here - --> $DIR/lint-ctypes-113436-1.rs:1:9 + --> $DIR/lint-113436-1.rs:1:9 | LL | #![deny(improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `NotSafe`, which is not FFI-safe - --> $DIR/lint-ctypes-113436-1.rs:22:30 + --> $DIR/lint-113436-1.rs:22:30 | LL | extern "C" fn bar(x: Bar) -> Bar { | ^^^ not FFI-safe @@ -26,7 +26,7 @@ LL | extern "C" fn bar(x: Bar) -> Bar { = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here - --> $DIR/lint-ctypes-113436-1.rs:13:1 + --> $DIR/lint-113436-1.rs:13:1 | LL | struct NotSafe(u32); | ^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-73249-2.rs b/tests/ui/lint/improper-ctypes/lint-73249-2.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73249-2.rs rename to tests/ui/lint/improper-ctypes/lint-73249-2.rs diff --git a/tests/ui/lint/lint-ctypes-73249-2.stderr b/tests/ui/lint/improper-ctypes/lint-73249-2.stderr similarity index 81% rename from tests/ui/lint/lint-ctypes-73249-2.stderr rename to tests/ui/lint/improper-ctypes/lint-73249-2.stderr index 2d0dfe94f097e..d6c1cec2bd6c1 100644 --- a/tests/ui/lint/lint-ctypes-73249-2.stderr +++ b/tests/ui/lint/improper-ctypes/lint-73249-2.stderr @@ -1,12 +1,12 @@ error: `extern` block uses type `Qux`, which is not FFI-safe - --> $DIR/lint-ctypes-73249-2.rs:27:21 + --> $DIR/lint-73249-2.rs:27:21 | LL | fn lint_me() -> A<()>; | ^^^^^ not FFI-safe | = note: opaque types have no C equivalent note: the lint level is defined here - --> $DIR/lint-ctypes-73249-2.rs:2:9 + --> $DIR/lint-73249-2.rs:2:9 | LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-73249-3.rs b/tests/ui/lint/improper-ctypes/lint-73249-3.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73249-3.rs rename to tests/ui/lint/improper-ctypes/lint-73249-3.rs diff --git a/tests/ui/lint/lint-ctypes-73249-5.stderr b/tests/ui/lint/improper-ctypes/lint-73249-3.stderr similarity index 81% rename from tests/ui/lint/lint-ctypes-73249-5.stderr rename to tests/ui/lint/improper-ctypes/lint-73249-3.stderr index c4fa955de05ed..ebc9eb5eb8274 100644 --- a/tests/ui/lint/lint-ctypes-73249-5.stderr +++ b/tests/ui/lint/improper-ctypes/lint-73249-3.stderr @@ -1,12 +1,12 @@ error: `extern` block uses type `Qux`, which is not FFI-safe - --> $DIR/lint-ctypes-73249-5.rs:21:25 + --> $DIR/lint-73249-3.rs:21:25 | LL | pub fn lint_me() -> A; | ^ not FFI-safe | = note: opaque types have no C equivalent note: the lint level is defined here - --> $DIR/lint-ctypes-73249-5.rs:2:9 + --> $DIR/lint-73249-3.rs:2:9 | LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-73249-5.rs b/tests/ui/lint/improper-ctypes/lint-73249-5.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73249-5.rs rename to tests/ui/lint/improper-ctypes/lint-73249-5.rs diff --git a/tests/ui/lint/lint-ctypes-73249-3.stderr b/tests/ui/lint/improper-ctypes/lint-73249-5.stderr similarity index 81% rename from tests/ui/lint/lint-ctypes-73249-3.stderr rename to tests/ui/lint/improper-ctypes/lint-73249-5.stderr index e1a313a290651..484927f57fead 100644 --- a/tests/ui/lint/lint-ctypes-73249-3.stderr +++ b/tests/ui/lint/improper-ctypes/lint-73249-5.stderr @@ -1,12 +1,12 @@ error: `extern` block uses type `Qux`, which is not FFI-safe - --> $DIR/lint-ctypes-73249-3.rs:21:25 + --> $DIR/lint-73249-5.rs:21:25 | LL | pub fn lint_me() -> A; | ^ not FFI-safe | = note: opaque types have no C equivalent note: the lint level is defined here - --> $DIR/lint-ctypes-73249-3.rs:2:9 + --> $DIR/lint-73249-5.rs:2:9 | LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-73251-1.rs b/tests/ui/lint/improper-ctypes/lint-73251-1.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73251-1.rs rename to tests/ui/lint/improper-ctypes/lint-73251-1.rs diff --git a/tests/ui/lint/lint-ctypes-73251-1.stderr b/tests/ui/lint/improper-ctypes/lint-73251-1.stderr similarity index 82% rename from tests/ui/lint/lint-ctypes-73251-1.stderr rename to tests/ui/lint/improper-ctypes/lint-73251-1.stderr index 675a9de51cd02..749722f0e2203 100644 --- a/tests/ui/lint/lint-ctypes-73251-1.stderr +++ b/tests/ui/lint/improper-ctypes/lint-73251-1.stderr @@ -1,12 +1,12 @@ error: `extern` block uses type `Qux`, which is not FFI-safe - --> $DIR/lint-ctypes-73251-1.rs:24:21 + --> $DIR/lint-73251-1.rs:24:21 | LL | fn lint_me() -> ::Assoc; | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | = note: opaque types have no C equivalent note: the lint level is defined here - --> $DIR/lint-ctypes-73251-1.rs:2:9 + --> $DIR/lint-73251-1.rs:2:9 | LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-73251-2.rs b/tests/ui/lint/improper-ctypes/lint-73251-2.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73251-2.rs rename to tests/ui/lint/improper-ctypes/lint-73251-2.rs diff --git a/tests/ui/lint/lint-ctypes-73251-2.stderr b/tests/ui/lint/improper-ctypes/lint-73251-2.stderr similarity index 82% rename from tests/ui/lint/lint-ctypes-73251-2.stderr rename to tests/ui/lint/improper-ctypes/lint-73251-2.stderr index 634950b29ed43..3770b7d789f67 100644 --- a/tests/ui/lint/lint-ctypes-73251-2.stderr +++ b/tests/ui/lint/improper-ctypes/lint-73251-2.stderr @@ -1,12 +1,12 @@ error: `extern` block uses type `AliasA`, which is not FFI-safe - --> $DIR/lint-ctypes-73251-2.rs:38:21 + --> $DIR/lint-73251-2.rs:38:21 | LL | fn lint_me() -> ::Assoc; | ^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = note: opaque types have no C equivalent note: the lint level is defined here - --> $DIR/lint-ctypes-73251-2.rs:2:9 + --> $DIR/lint-73251-2.rs:2:9 | LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-94223.rs b/tests/ui/lint/improper-ctypes/lint-94223.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-94223.rs rename to tests/ui/lint/improper-ctypes/lint-94223.rs diff --git a/tests/ui/lint/lint-ctypes-94223.stderr b/tests/ui/lint/improper-ctypes/lint-94223.stderr similarity index 86% rename from tests/ui/lint/lint-ctypes-94223.stderr rename to tests/ui/lint/improper-ctypes/lint-94223.stderr index bd127cf60044c..008debf8f010a 100644 --- a/tests/ui/lint/lint-ctypes-94223.stderr +++ b/tests/ui/lint/improper-ctypes/lint-94223.stderr @@ -1,5 +1,5 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:4:15 + --> $DIR/lint-94223.rs:4:15 | LL | pub fn bad(f: extern "C" fn([u8])) {} | ^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -7,13 +7,13 @@ LL | pub fn bad(f: extern "C" fn([u8])) {} = help: consider using a raw pointer instead = note: slices have no C equivalent note: the lint level is defined here - --> $DIR/lint-ctypes-94223.rs:2:9 + --> $DIR/lint-94223.rs:2:9 | LL | #![deny(improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `[u8]`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:7:28 + --> $DIR/lint-94223.rs:7:28 | LL | pub fn bad_twice(f: Result) {} | ^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -22,7 +22,7 @@ LL | pub fn bad_twice(f: Result) {} = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:7:49 + --> $DIR/lint-94223.rs:7:49 | LL | pub fn bad_twice(f: Result) {} | ^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -31,7 +31,7 @@ LL | pub fn bad_twice(f: Result) {} = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:11:18 + --> $DIR/lint-94223.rs:11:18 | LL | struct BadStruct(extern "C" fn([u8])); | ^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -40,7 +40,7 @@ LL | struct BadStruct(extern "C" fn([u8])); = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:15:7 + --> $DIR/lint-94223.rs:15:7 | LL | A(extern "C" fn([u8])), | ^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -49,7 +49,7 @@ LL | A(extern "C" fn([u8])), = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:20:7 + --> $DIR/lint-94223.rs:20:7 | LL | A(extern "C" fn([u8])), | ^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -58,7 +58,7 @@ LL | A(extern "C" fn([u8])), = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:24:12 + --> $DIR/lint-94223.rs:24:12 | LL | type Foo = extern "C" fn([u8]); | ^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -67,7 +67,7 @@ LL | type Foo = extern "C" fn([u8]); = note: slices have no C equivalent error: `extern` fn uses type `Option<&::FooType>`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:31:20 + --> $DIR/lint-94223.rs:31:20 | LL | pub type Foo2 = extern "C" fn(Option<&::FooType>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -76,7 +76,7 @@ LL | pub type Foo2 = extern "C" fn(Option<&::FooType>); = note: enum has no representation hint error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:41:17 + --> $DIR/lint-94223.rs:41:17 | LL | pub static BAD: extern "C" fn(FfiUnsafe) = f; | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -84,13 +84,13 @@ LL | pub static BAD: extern "C" fn(FfiUnsafe) = f; = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here - --> $DIR/lint-ctypes-94223.rs:34:1 + --> $DIR/lint-94223.rs:34:1 | LL | pub struct FfiUnsafe; | ^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:44:30 + --> $DIR/lint-94223.rs:44:30 | LL | pub static BAD_TWICE: Result = Ok(f); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -98,13 +98,13 @@ LL | pub static BAD_TWICE: Result $DIR/lint-ctypes-94223.rs:34:1 + --> $DIR/lint-94223.rs:34:1 | LL | pub struct FfiUnsafe; | ^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:44:56 + --> $DIR/lint-94223.rs:44:56 | LL | pub static BAD_TWICE: Result = Ok(f); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -112,13 +112,13 @@ LL | pub static BAD_TWICE: Result $DIR/lint-ctypes-94223.rs:34:1 + --> $DIR/lint-94223.rs:34:1 | LL | pub struct FfiUnsafe; | ^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe - --> $DIR/lint-ctypes-94223.rs:48:22 + --> $DIR/lint-94223.rs:48:22 | LL | pub const BAD_CONST: extern "C" fn(FfiUnsafe) = f; | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -126,7 +126,7 @@ LL | pub const BAD_CONST: extern "C" fn(FfiUnsafe) = f; = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here - --> $DIR/lint-ctypes-94223.rs:34:1 + --> $DIR/lint-94223.rs:34:1 | LL | pub struct FfiUnsafe; | ^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-cstr.rs b/tests/ui/lint/improper-ctypes/lint-cstr.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-cstr.rs rename to tests/ui/lint/improper-ctypes/lint-cstr.rs diff --git a/tests/ui/lint/lint-ctypes-cstr.stderr b/tests/ui/lint/improper-ctypes/lint-cstr.stderr similarity index 88% rename from tests/ui/lint/lint-ctypes-cstr.stderr rename to tests/ui/lint/improper-ctypes/lint-cstr.stderr index 8957758d57732..da26306584311 100644 --- a/tests/ui/lint/lint-ctypes-cstr.stderr +++ b/tests/ui/lint/improper-ctypes/lint-cstr.stderr @@ -1,5 +1,5 @@ error: `extern` block uses type `CStr`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:7:21 + --> $DIR/lint-cstr.rs:7:21 | LL | fn take_cstr(s: CStr); | ^^^^ not FFI-safe @@ -7,13 +7,13 @@ LL | fn take_cstr(s: CStr); = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` = note: `CStr`/`CString` do not have a guaranteed layout note: the lint level is defined here - --> $DIR/lint-ctypes-cstr.rs:2:9 + --> $DIR/lint-cstr.rs:2:9 | LL | #![deny(improper_ctypes, improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^ error: `extern` block uses type `CStr`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:10:25 + --> $DIR/lint-cstr.rs:10:25 | LL | fn take_cstr_ref(s: &CStr); | ^^^^^ not FFI-safe @@ -22,7 +22,7 @@ LL | fn take_cstr_ref(s: &CStr); = note: `CStr`/`CString` do not have a guaranteed layout error: `extern` block uses type `CString`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:13:24 + --> $DIR/lint-cstr.rs:13:24 | LL | fn take_cstring(s: CString); | ^^^^^^^ not FFI-safe @@ -31,7 +31,7 @@ LL | fn take_cstring(s: CString); = note: `CStr`/`CString` do not have a guaranteed layout error: `extern` block uses type `CString`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:16:28 + --> $DIR/lint-cstr.rs:16:28 | LL | fn take_cstring_ref(s: &CString); | ^^^^^^^^ not FFI-safe @@ -40,7 +40,7 @@ LL | fn take_cstring_ref(s: &CString); = note: `CStr`/`CString` do not have a guaranteed layout error: `extern` block uses type `CString`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:20:43 + --> $DIR/lint-cstr.rs:20:43 | LL | fn no_special_help_for_mut_cstring(s: *mut CString); | ^^^^^^^^^^^^ not FFI-safe @@ -49,7 +49,7 @@ LL | fn no_special_help_for_mut_cstring(s: *mut CString); = note: this struct has unspecified layout error: `extern` block uses type `CString`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:24:47 + --> $DIR/lint-cstr.rs:24:47 | LL | fn no_special_help_for_mut_cstring_ref(s: &mut CString); | ^^^^^^^^^^^^ not FFI-safe @@ -58,7 +58,7 @@ LL | fn no_special_help_for_mut_cstring_ref(s: &mut CString); = note: this struct has unspecified layout error: `extern` fn uses type `CStr`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:29:37 + --> $DIR/lint-cstr.rs:29:37 | LL | extern "C" fn rust_take_cstr_ref(s: &CStr) {} | ^^^^^ not FFI-safe @@ -66,13 +66,13 @@ LL | extern "C" fn rust_take_cstr_ref(s: &CStr) {} = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` = note: `CStr`/`CString` do not have a guaranteed layout note: the lint level is defined here - --> $DIR/lint-ctypes-cstr.rs:2:26 + --> $DIR/lint-cstr.rs:2:26 | LL | #![deny(improper_ctypes, improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `CString`, which is not FFI-safe - --> $DIR/lint-ctypes-cstr.rs:32:36 + --> $DIR/lint-cstr.rs:32:36 | LL | extern "C" fn rust_take_cstring(s: CString) {} | ^^^^^^^ not FFI-safe diff --git a/tests/ui/lint/lint-ctypes.rs b/tests/ui/lint/improper-ctypes/lint-ctypes.rs similarity index 100% rename from tests/ui/lint/lint-ctypes.rs rename to tests/ui/lint/improper-ctypes/lint-ctypes.rs diff --git a/tests/ui/lint/lint-ctypes.stderr b/tests/ui/lint/improper-ctypes/lint-ctypes.stderr similarity index 100% rename from tests/ui/lint/lint-ctypes.stderr rename to tests/ui/lint/improper-ctypes/lint-ctypes.stderr diff --git a/tests/ui/lint/lint-ctypes-enum.rs b/tests/ui/lint/improper-ctypes/lint-enum.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-enum.rs rename to tests/ui/lint/improper-ctypes/lint-enum.rs diff --git a/tests/ui/lint/lint-ctypes-enum.stderr b/tests/ui/lint/improper-ctypes/lint-enum.stderr similarity index 90% rename from tests/ui/lint/lint-ctypes-enum.stderr rename to tests/ui/lint/improper-ctypes/lint-enum.stderr index 40d22723309fb..35d1dcb87fd82 100644 --- a/tests/ui/lint/lint-ctypes-enum.stderr +++ b/tests/ui/lint/improper-ctypes/lint-enum.stderr @@ -1,5 +1,5 @@ error: `extern` block uses type `U`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:82:14 + --> $DIR/lint-enum.rs:82:14 | LL | fn uf(x: U); | ^ not FFI-safe @@ -7,18 +7,18 @@ LL | fn uf(x: U); = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint note: the type is defined here - --> $DIR/lint-ctypes-enum.rs:9:1 + --> $DIR/lint-enum.rs:9:1 | LL | enum U { | ^^^^^^ note: the lint level is defined here - --> $DIR/lint-ctypes-enum.rs:2:9 + --> $DIR/lint-enum.rs:2:9 | LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ error: `extern` block uses type `B`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:83:14 + --> $DIR/lint-enum.rs:83:14 | LL | fn bf(x: B); | ^ not FFI-safe @@ -26,13 +26,13 @@ LL | fn bf(x: B); = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint note: the type is defined here - --> $DIR/lint-ctypes-enum.rs:12:1 + --> $DIR/lint-enum.rs:12:1 | LL | enum B { | ^^^^^^ error: `extern` block uses type `T`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:84:14 + --> $DIR/lint-enum.rs:84:14 | LL | fn tf(x: T); | ^ not FFI-safe @@ -40,13 +40,13 @@ LL | fn tf(x: T); = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint note: the type is defined here - --> $DIR/lint-ctypes-enum.rs:16:1 + --> $DIR/lint-enum.rs:16:1 | LL | enum T { | ^^^^^^ error: `extern` block uses type `Option>>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:108:36 + --> $DIR/lint-enum.rs:108:36 | LL | fn option_transparent_union(x: Option>>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -55,7 +55,7 @@ LL | fn option_transparent_union(x: Option = note: enum has no representation hint error: `extern` block uses type `Option>>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:110:28 + --> $DIR/lint-enum.rs:110:28 | LL | fn option_repr_rust(x: Option>>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -64,7 +64,7 @@ LL | fn option_repr_rust(x: Option>>); = note: enum has no representation hint error: `extern` block uses type `Option`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:111:21 + --> $DIR/lint-enum.rs:111:21 | LL | fn option_u8(x: Option); | ^^^^^^^^^^ not FFI-safe @@ -73,7 +73,7 @@ LL | fn option_u8(x: Option); = note: enum has no representation hint error: `extern` block uses type `Result>, ()>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:131:38 + --> $DIR/lint-enum.rs:131:38 | LL | fn result_transparent_union_t(x: Result>, ()>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -82,7 +82,7 @@ LL | fn result_transparent_union_t(x: Result>, ()>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:133:30 + --> $DIR/lint-enum.rs:133:30 | LL | fn result_repr_rust_t(x: Result>, ()>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -91,7 +91,7 @@ LL | fn result_repr_rust_t(x: Result>, ()>); = note: enum has no representation hint error: `extern` block uses type `Result, U>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:137:51 + --> $DIR/lint-enum.rs:137:51 | LL | fn result_1zst_exhaustive_single_variant_t(x: Result, U>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -100,7 +100,7 @@ LL | fn result_1zst_exhaustive_single_variant_t(x: Result, = note: enum has no representation hint error: `extern` block uses type `Result, B>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:139:53 + --> $DIR/lint-enum.rs:139:53 | LL | fn result_1zst_exhaustive_multiple_variant_t(x: Result, B>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -109,7 +109,7 @@ LL | fn result_1zst_exhaustive_multiple_variant_t(x: Result = note: enum has no representation hint error: `extern` block uses type `Result, NonExhaustive>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:141:51 + --> $DIR/lint-enum.rs:141:51 | LL | fn result_1zst_non_exhaustive_no_variant_t(x: Result, NonExhaustive>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -118,7 +118,7 @@ LL | fn result_1zst_non_exhaustive_no_variant_t(x: Result, = note: enum has no representation hint error: `extern` block uses type `Result, Field>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:144:49 + --> $DIR/lint-enum.rs:144:49 | LL | fn result_1zst_exhaustive_single_field_t(x: Result, Field>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -127,7 +127,7 @@ LL | fn result_1zst_exhaustive_single_field_t(x: Result, Fi = note: enum has no representation hint error: `extern` block uses type `Result>, ()>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:146:30 + --> $DIR/lint-enum.rs:146:30 | LL | fn result_cascading_t(x: Result>, ()>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -136,7 +136,7 @@ LL | fn result_cascading_t(x: Result>, ()>); = note: enum has no representation hint error: `extern` block uses type `Result<(), TransparentUnion>>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:167:38 + --> $DIR/lint-enum.rs:167:38 | LL | fn result_transparent_union_e(x: Result<(), TransparentUnion>>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -145,7 +145,7 @@ LL | fn result_transparent_union_e(x: Result<(), TransparentUnion>>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:169:30 + --> $DIR/lint-enum.rs:169:30 | LL | fn result_repr_rust_e(x: Result<(), Rust>>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -154,7 +154,7 @@ LL | fn result_repr_rust_e(x: Result<(), Rust>>); = note: enum has no representation hint error: `extern` block uses type `Result>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:173:51 + --> $DIR/lint-enum.rs:173:51 | LL | fn result_1zst_exhaustive_single_variant_e(x: Result>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -163,7 +163,7 @@ LL | fn result_1zst_exhaustive_single_variant_e(x: Result>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:175:53 + --> $DIR/lint-enum.rs:175:53 | LL | fn result_1zst_exhaustive_multiple_variant_e(x: Result>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -172,7 +172,7 @@ LL | fn result_1zst_exhaustive_multiple_variant_e(x: Result>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:177:51 + --> $DIR/lint-enum.rs:177:51 | LL | fn result_1zst_non_exhaustive_no_variant_e(x: Result>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -181,7 +181,7 @@ LL | fn result_1zst_non_exhaustive_no_variant_e(x: Result>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:180:49 + --> $DIR/lint-enum.rs:180:49 | LL | fn result_1zst_exhaustive_single_field_e(x: Result>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -190,7 +190,7 @@ LL | fn result_1zst_exhaustive_single_field_e(x: Result>>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:182:30 + --> $DIR/lint-enum.rs:182:30 | LL | fn result_cascading_e(x: Result<(), Result<(), num::NonZero>>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -199,7 +199,7 @@ LL | fn result_cascading_e(x: Result<(), Result<(), num::NonZero>>); = note: enum has no representation hint error: `extern` block uses type `Result<(), ()>`, which is not FFI-safe - --> $DIR/lint-ctypes-enum.rs:184:27 + --> $DIR/lint-enum.rs:184:27 | LL | fn result_unit_t_e(x: Result<(), ()>); | ^^^^^^^^^^^^^^ not FFI-safe diff --git a/tests/ui/lint/lint-ctypes-fn.rs b/tests/ui/lint/improper-ctypes/lint-fn.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-fn.rs rename to tests/ui/lint/improper-ctypes/lint-fn.rs diff --git a/tests/ui/lint/lint-ctypes-fn.stderr b/tests/ui/lint/improper-ctypes/lint-fn.stderr similarity index 88% rename from tests/ui/lint/lint-ctypes-fn.stderr rename to tests/ui/lint/improper-ctypes/lint-fn.stderr index a19c04a63b566..34e3bd021b92d 100644 --- a/tests/ui/lint/lint-ctypes-fn.stderr +++ b/tests/ui/lint/improper-ctypes/lint-fn.stderr @@ -1,5 +1,5 @@ error: `extern` fn uses type `[u32]`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:70:33 + --> $DIR/lint-fn.rs:70:33 | LL | pub extern "C" fn slice_type(p: &[u32]) { } | ^^^^^^ not FFI-safe @@ -7,13 +7,13 @@ LL | pub extern "C" fn slice_type(p: &[u32]) { } = help: consider using a raw pointer instead = note: slices have no C equivalent note: the lint level is defined here - --> $DIR/lint-ctypes-fn.rs:2:9 + --> $DIR/lint-fn.rs:2:9 | LL | #![deny(improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `str`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:73:31 + --> $DIR/lint-fn.rs:73:31 | LL | pub extern "C" fn str_type(p: &str) { } | ^^^^ not FFI-safe @@ -22,7 +22,7 @@ LL | pub extern "C" fn str_type(p: &str) { } = note: string slices have no C equivalent error: `extern` fn uses type `Box<[u8]>`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:80:34 + --> $DIR/lint-fn.rs:80:34 | LL | pub extern "C" fn boxed_slice(p: Box<[u8]>) { } | ^^^^^^^^^ not FFI-safe @@ -30,7 +30,7 @@ LL | pub extern "C" fn boxed_slice(p: Box<[u8]>) { } = note: box cannot be represented as a single pointer error: `extern` fn uses type `Box`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:83:35 + --> $DIR/lint-fn.rs:83:35 | LL | pub extern "C" fn boxed_string(p: Box) { } | ^^^^^^^^ not FFI-safe @@ -38,7 +38,7 @@ LL | pub extern "C" fn boxed_string(p: Box) { } = note: box cannot be represented as a single pointer error: `extern` fn uses type `Box`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:86:34 + --> $DIR/lint-fn.rs:86:34 | LL | pub extern "C" fn boxed_trait(p: Box) { } | ^^^^^^^^^^^^^^ not FFI-safe @@ -46,7 +46,7 @@ LL | pub extern "C" fn boxed_trait(p: Box) { } = note: box cannot be represented as a single pointer error: `extern` fn uses type `char`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:89:32 + --> $DIR/lint-fn.rs:89:32 | LL | pub extern "C" fn char_type(p: char) { } | ^^^^ not FFI-safe @@ -55,7 +55,7 @@ LL | pub extern "C" fn char_type(p: char) { } = note: the `char` type has no C equivalent error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:92:33 + --> $DIR/lint-fn.rs:92:33 | LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } | ^^^^^^^^^^ not FFI-safe @@ -64,7 +64,7 @@ LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } = note: tuples have unspecified layout error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:95:34 + --> $DIR/lint-fn.rs:95:34 | LL | pub extern "C" fn tuple_type2(p: I32Pair) { } | ^^^^^^^ not FFI-safe @@ -73,7 +73,7 @@ LL | pub extern "C" fn tuple_type2(p: I32Pair) { } = note: tuples have unspecified layout error: `extern` fn uses type `ZeroSize`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:98:32 + --> $DIR/lint-fn.rs:98:32 | LL | pub extern "C" fn zero_size(p: ZeroSize) { } | ^^^^^^^^ not FFI-safe @@ -81,26 +81,26 @@ LL | pub extern "C" fn zero_size(p: ZeroSize) { } = help: consider adding a member to this struct = note: this struct has no fields note: the type is defined here - --> $DIR/lint-ctypes-fn.rs:25:1 + --> $DIR/lint-fn.rs:25:1 | LL | pub struct ZeroSize; | ^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `ZeroSizeWithPhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:101:40 + --> $DIR/lint-fn.rs:101:40 | LL | pub extern "C" fn zero_size_phantom(p: ZeroSizeWithPhantomData) { } | ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = note: composed only of `PhantomData` note: the type is defined here - --> $DIR/lint-ctypes-fn.rs:60:1 + --> $DIR/lint-fn.rs:60:1 | LL | pub struct ZeroSizeWithPhantomData(PhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` fn uses type `PhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:104:51 + --> $DIR/lint-fn.rs:104:51 | LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { | ^^^^^^^^^^^^^^^^^ not FFI-safe @@ -108,7 +108,7 @@ LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { = note: composed only of `PhantomData` error: `extern` fn uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:109:30 + --> $DIR/lint-fn.rs:109:30 | LL | pub extern "C" fn fn_type(p: RustFn) { } | ^^^^^^ not FFI-safe @@ -117,7 +117,7 @@ LL | pub extern "C" fn fn_type(p: RustFn) { } = note: this function pointer has Rust-specific calling convention error: `extern` fn uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:112:31 + --> $DIR/lint-fn.rs:112:31 | LL | pub extern "C" fn fn_type2(p: fn()) { } | ^^^^ not FFI-safe @@ -126,7 +126,7 @@ LL | pub extern "C" fn fn_type2(p: fn()) { } = note: this function pointer has Rust-specific calling convention error: `extern` fn uses type `str`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:117:38 + --> $DIR/lint-fn.rs:117:38 | LL | pub extern "C" fn transparent_str(p: TransparentStr) { } | ^^^^^^^^^^^^^^ not FFI-safe @@ -135,7 +135,7 @@ LL | pub extern "C" fn transparent_str(p: TransparentStr) { } = note: string slices have no C equivalent error: `extern` fn uses type `PhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:169:43 + --> $DIR/lint-fn.rs:169:43 | LL | pub extern "C" fn unused_generic2() -> PhantomData { | ^^^^^^^^^^^^^^^^^ not FFI-safe @@ -143,7 +143,7 @@ LL | pub extern "C" fn unused_generic2() -> PhantomData { = note: composed only of `PhantomData` error: `extern` fn uses type `Vec`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:182:39 + --> $DIR/lint-fn.rs:182:39 | LL | pub extern "C" fn used_generic4(x: Vec) { } | ^^^^^^ not FFI-safe @@ -152,7 +152,7 @@ LL | pub extern "C" fn used_generic4(x: Vec) { } = note: this struct has unspecified layout error: `extern` fn uses type `Vec`, which is not FFI-safe - --> $DIR/lint-ctypes-fn.rs:185:41 + --> $DIR/lint-fn.rs:185:41 | LL | pub extern "C" fn used_generic5() -> Vec { | ^^^^^^ not FFI-safe diff --git a/tests/ui/lint/lint-ctypes-non-recursion-limit.rs b/tests/ui/lint/improper-ctypes/lint-non-recursion-limit.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-non-recursion-limit.rs rename to tests/ui/lint/improper-ctypes/lint-non-recursion-limit.rs diff --git a/tests/ui/lint/lint-ctypes-option-nonnull-unsized.rs b/tests/ui/lint/improper-ctypes/lint-option-nonnull-unsized.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-option-nonnull-unsized.rs rename to tests/ui/lint/improper-ctypes/lint-option-nonnull-unsized.rs diff --git a/tests/ui/lint/lint-ctypes-option-nonnull-unsized.stderr b/tests/ui/lint/improper-ctypes/lint-option-nonnull-unsized.stderr similarity index 83% rename from tests/ui/lint/lint-ctypes-option-nonnull-unsized.stderr rename to tests/ui/lint/improper-ctypes/lint-option-nonnull-unsized.stderr index 746304694167f..b17fb6bd6145d 100644 --- a/tests/ui/lint/lint-ctypes-option-nonnull-unsized.stderr +++ b/tests/ui/lint/improper-ctypes/lint-option-nonnull-unsized.stderr @@ -1,5 +1,5 @@ error: `extern` fn uses type `Option<&T>`, which is not FFI-safe - --> $DIR/lint-ctypes-option-nonnull-unsized.rs:3:45 + --> $DIR/lint-option-nonnull-unsized.rs:3:45 | LL | extern "C" fn foo() -> Option<&'static T> { | ^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -7,7 +7,7 @@ LL | extern "C" fn foo() -> Option<&'static T> { = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint note: the lint level is defined here - --> $DIR/lint-ctypes-option-nonnull-unsized.rs:1:9 + --> $DIR/lint-option-nonnull-unsized.rs:1:9 | LL | #![deny(improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-ctypes-113436.rs b/tests/ui/lint/improper-ctypes/mustpass-113436.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-113436.rs rename to tests/ui/lint/improper-ctypes/mustpass-113436.rs diff --git a/tests/ui/lint/lint-ctypes-113900.rs b/tests/ui/lint/improper-ctypes/mustpass-113900.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-113900.rs rename to tests/ui/lint/improper-ctypes/mustpass-113900.rs diff --git a/tests/ui/lint/improper_ctypes_definitions_ice_134060.rs b/tests/ui/lint/improper-ctypes/mustpass-134060.rs similarity index 100% rename from tests/ui/lint/improper_ctypes_definitions_ice_134060.rs rename to tests/ui/lint/improper-ctypes/mustpass-134060.rs diff --git a/tests/ui/lint/improper_ctypes_definitions_ice_134060.stderr b/tests/ui/lint/improper-ctypes/mustpass-134060.stderr similarity index 85% rename from tests/ui/lint/improper_ctypes_definitions_ice_134060.stderr rename to tests/ui/lint/improper-ctypes/mustpass-134060.stderr index f6ac9a92cd5f0..791b2f7370983 100644 --- a/tests/ui/lint/improper_ctypes_definitions_ice_134060.stderr +++ b/tests/ui/lint/improper-ctypes/mustpass-134060.stderr @@ -1,5 +1,5 @@ warning: `extern` fn uses type `()`, which is not FFI-safe - --> $DIR/improper_ctypes_definitions_ice_134060.rs:11:34 + --> $DIR/mustpass-134060.rs:11:34 | LL | extern "C" fn foo_(&self, _: ()) -> i64 { | ^^ not FFI-safe diff --git a/tests/ui/lint/lint-ctypes-66202.rs b/tests/ui/lint/improper-ctypes/mustpass-66202.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-66202.rs rename to tests/ui/lint/improper-ctypes/mustpass-66202.rs diff --git a/tests/ui/lint/lint-ctypes-73249-1.rs b/tests/ui/lint/improper-ctypes/mustpass-73249-1.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73249-1.rs rename to tests/ui/lint/improper-ctypes/mustpass-73249-1.rs diff --git a/tests/ui/lint/lint-ctypes-73249-4.rs b/tests/ui/lint/improper-ctypes/mustpass-73249-4.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73249-4.rs rename to tests/ui/lint/improper-ctypes/mustpass-73249-4.rs diff --git a/tests/ui/lint/lint-ctypes-73249.rs b/tests/ui/lint/improper-ctypes/mustpass-73249.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73249.rs rename to tests/ui/lint/improper-ctypes/mustpass-73249.rs diff --git a/tests/ui/lint/lint-ctypes-73251.rs b/tests/ui/lint/improper-ctypes/mustpass-73251.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73251.rs rename to tests/ui/lint/improper-ctypes/mustpass-73251.rs diff --git a/tests/ui/lint/lint-ctypes-73747.rs b/tests/ui/lint/improper-ctypes/mustpass-73747.rs similarity index 100% rename from tests/ui/lint/lint-ctypes-73747.rs rename to tests/ui/lint/improper-ctypes/mustpass-73747.rs diff --git a/tests/ui/lint/improper_ctypes/repr-rust-is-undefined.rs b/tests/ui/lint/improper-ctypes/repr-rust-is-undefined.rs similarity index 100% rename from tests/ui/lint/improper_ctypes/repr-rust-is-undefined.rs rename to tests/ui/lint/improper-ctypes/repr-rust-is-undefined.rs diff --git a/tests/ui/lint/improper_ctypes/repr-rust-is-undefined.stderr b/tests/ui/lint/improper-ctypes/repr-rust-is-undefined.stderr similarity index 100% rename from tests/ui/lint/improper_ctypes/repr-rust-is-undefined.stderr rename to tests/ui/lint/improper-ctypes/repr-rust-is-undefined.stderr From 33943d183207980a56387ca78dc7865b21b99c10 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 22 Aug 2025 23:39:07 +0200 Subject: [PATCH 2/4] ImproperCTypes: more pre-emptive cleanup Mainly, we realise that the non-null assumption on a Box<_> argument does not depend on what side of the FFI boundary the function is on. And anyway, this is not the way to deal with this assumption being maybe violated. --- compiler/rustc_lint/src/foreign_modules.rs | 27 ++++-------- compiler/rustc_lint/src/types.rs | 14 +++---- .../rustc_lint/src/types/improper_ctypes.rs | 4 +- tests/ui/lint/improper-ctypes/lint-ctypes.rs | 1 - .../lint/improper-ctypes/lint-ctypes.stderr | 41 ++++++++----------- 5 files changed, 31 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_lint/src/foreign_modules.rs b/compiler/rustc_lint/src/foreign_modules.rs index 3267e70f1de26..ad73e15e31f37 100644 --- a/compiler/rustc_lint/src/foreign_modules.rs +++ b/compiler/rustc_lint/src/foreign_modules.rs @@ -136,7 +136,6 @@ impl ClashingExternDeclarations { ty::TypingEnv::non_body_analysis(tcx, this_fi.owner_id), existing_decl_ty, this_decl_ty, - types::CItemKind::Declaration, ) { let orig = name_of_extern_decl(tcx, existing_did); @@ -214,10 +213,9 @@ fn structurally_same_type<'tcx>( typing_env: ty::TypingEnv<'tcx>, a: Ty<'tcx>, b: Ty<'tcx>, - ckind: types::CItemKind, ) -> bool { let mut seen_types = UnordSet::default(); - let result = structurally_same_type_impl(&mut seen_types, tcx, typing_env, a, b, ckind); + let result = structurally_same_type_impl(&mut seen_types, tcx, typing_env, a, b); if cfg!(debug_assertions) && result { // Sanity-check: must have same ABI, size and alignment. // `extern` blocks cannot be generic, so we'll always get a layout here. @@ -236,7 +234,6 @@ fn structurally_same_type_impl<'tcx>( typing_env: ty::TypingEnv<'tcx>, a: Ty<'tcx>, b: Ty<'tcx>, - ckind: types::CItemKind, ) -> bool { debug!("structurally_same_type_impl(tcx, a = {:?}, b = {:?})", a, b); @@ -307,7 +304,6 @@ fn structurally_same_type_impl<'tcx>( typing_env, tcx.type_of(a_did).instantiate(tcx, a_gen_args), tcx.type_of(b_did).instantiate(tcx, b_gen_args), - ckind, ) }, ) @@ -315,25 +311,19 @@ fn structurally_same_type_impl<'tcx>( (ty::Array(a_ty, a_len), ty::Array(b_ty, b_len)) => { // For arrays, we also check the length. a_len == b_len - && structurally_same_type_impl( - seen_types, tcx, typing_env, *a_ty, *b_ty, ckind, - ) + && structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty) } (ty::Slice(a_ty), ty::Slice(b_ty)) => { - structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty, ckind) + structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty) } (ty::RawPtr(a_ty, a_mutbl), ty::RawPtr(b_ty, b_mutbl)) => { a_mutbl == b_mutbl - && structurally_same_type_impl( - seen_types, tcx, typing_env, *a_ty, *b_ty, ckind, - ) + && structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty) } (ty::Ref(_a_region, a_ty, a_mut), ty::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, tcx, typing_env, *a_ty, *b_ty, ckind, - ) + && structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty) } (ty::FnDef(..), ty::FnDef(..)) => { let a_poly_sig = a.fn_sig(tcx); @@ -347,7 +337,7 @@ fn structurally_same_type_impl<'tcx>( (a_sig.abi, a_sig.safety, a_sig.c_variadic) == (b_sig.abi, b_sig.safety, b_sig.c_variadic) && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { - structurally_same_type_impl(seen_types, tcx, typing_env, *a, *b, ckind) + structurally_same_type_impl(seen_types, tcx, typing_env, *a, *b) }) && structurally_same_type_impl( seen_types, @@ -355,7 +345,6 @@ fn structurally_same_type_impl<'tcx>( typing_env, a_sig.output(), b_sig.output(), - ckind, ) } (ty::Tuple(..), ty::Tuple(..)) => { @@ -383,14 +372,14 @@ fn structurally_same_type_impl<'tcx>( // An Adt and a primitive or pointer type. This can be FFI-safe if non-null // enum layout optimisation is being applied. (ty::Adt(..) | ty::Pat(..), _) if is_primitive_or_pointer(b) => { - if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a, ckind) { + if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a) { a_inner == b } else { false } } (_, ty::Adt(..) | ty::Pat(..)) if is_primitive_or_pointer(a) => { - if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b, ckind) { + if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b) { b_inner == a } else { false diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 056ecb70af8a4..979a863170100 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -11,9 +11,7 @@ use tracing::debug; use {rustc_ast as ast, rustc_hir as hir}; mod improper_ctypes; // these filed do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations -pub(crate) use improper_ctypes::{ - CItemKind, ImproperCTypesDeclarations, ImproperCTypesDefinitions, -}; +pub(crate) use improper_ctypes::{ImproperCTypesDeclarations, ImproperCTypesDefinitions}; use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, @@ -712,14 +710,13 @@ fn ty_is_known_nonnull<'tcx>( tcx: TyCtxt<'tcx>, typing_env: ty::TypingEnv<'tcx>, ty: Ty<'tcx>, - mode: CItemKind, ) -> bool { let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty); match ty.kind() { ty::FnPtr(..) => true, ty::Ref(..) => true, - ty::Adt(def, _) if def.is_box() && matches!(mode, CItemKind::Definition) => true, + ty::Adt(def, _) if def.is_box() => true, ty::Adt(def, args) if def.repr().transparent() && !def.is_union() => { let marked_non_null = nonnull_optimization_guaranteed(tcx, *def); @@ -735,10 +732,10 @@ fn ty_is_known_nonnull<'tcx>( def.variants() .iter() .filter_map(|variant| transparent_newtype_field(tcx, variant)) - .any(|field| ty_is_known_nonnull(tcx, typing_env, field.ty(tcx, args), mode)) + .any(|field| ty_is_known_nonnull(tcx, typing_env, field.ty(tcx, args))) } ty::Pat(base, pat) => { - ty_is_known_nonnull(tcx, typing_env, *base, mode) + ty_is_known_nonnull(tcx, typing_env, *base) || pat_ty_is_known_nonnull(tcx, typing_env, *pat) } _ => false, @@ -849,7 +846,6 @@ pub(crate) fn repr_nullable_ptr<'tcx>( tcx: TyCtxt<'tcx>, typing_env: ty::TypingEnv<'tcx>, ty: Ty<'tcx>, - ckind: CItemKind, ) -> Option> { debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty); match ty.kind() { @@ -874,7 +870,7 @@ pub(crate) fn repr_nullable_ptr<'tcx>( _ => return None, }; - if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) { + if !ty_is_known_nonnull(tcx, typing_env, field_ty) { return None; } diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 6f732fc2a50c8..7d7d86f93fc9a 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -173,7 +173,7 @@ fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool { } #[derive(Clone, Copy)] -pub(crate) enum CItemKind { +enum CItemKind { Declaration, Definition, } @@ -385,7 +385,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { { // Special-case types like `Option` and `Result` if let Some(ty) = - repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty, self.mode) + repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty) { return self.check_type_for_ffi(acc, ty); } diff --git a/tests/ui/lint/improper-ctypes/lint-ctypes.rs b/tests/ui/lint/improper-ctypes/lint-ctypes.rs index 47586c826ab86..7dc06079fa32c 100644 --- a/tests/ui/lint/improper-ctypes/lint-ctypes.rs +++ b/tests/ui/lint/improper-ctypes/lint-ctypes.rs @@ -52,7 +52,6 @@ extern "C" { pub fn str_type(p: &str); //~ ERROR: uses type `str` pub fn box_type(p: Box); //~ ERROR uses type `Box` pub fn opt_box_type(p: Option>); - //~^ ERROR uses type `Option>` pub fn char_type(p: char); //~ ERROR uses type `char` pub fn trait_type(p: &dyn Bar); //~ ERROR uses type `dyn Bar` pub fn tuple_type(p: (i32, i32)); //~ ERROR uses type `(i32, i32)` diff --git a/tests/ui/lint/improper-ctypes/lint-ctypes.stderr b/tests/ui/lint/improper-ctypes/lint-ctypes.stderr index 3fb36647d4f1c..6f8b951c53d70 100644 --- a/tests/ui/lint/improper-ctypes/lint-ctypes.stderr +++ b/tests/ui/lint/improper-ctypes/lint-ctypes.stderr @@ -67,17 +67,8 @@ LL | pub fn box_type(p: Box); = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:54:28 - | -LL | pub fn opt_box_type(p: Option>); - | ^^^^^^^^^^^^^^^^ not FFI-safe - | - = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum - = note: enum has no representation hint - error: `extern` block uses type `char`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:56:25 + --> $DIR/lint-ctypes.rs:55:25 | LL | pub fn char_type(p: char); | ^^^^ not FFI-safe @@ -86,7 +77,7 @@ LL | pub fn char_type(p: char); = note: the `char` type has no C equivalent error: `extern` block uses type `dyn Bar`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:57:26 + --> $DIR/lint-ctypes.rs:56:26 | LL | pub fn trait_type(p: &dyn Bar); | ^^^^^^^^ not FFI-safe @@ -94,7 +85,7 @@ LL | pub fn trait_type(p: &dyn Bar); = note: trait objects have no C equivalent error: `extern` block uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:58:26 + --> $DIR/lint-ctypes.rs:57:26 | LL | pub fn tuple_type(p: (i32, i32)); | ^^^^^^^^^^ not FFI-safe @@ -103,7 +94,7 @@ LL | pub fn tuple_type(p: (i32, i32)); = note: tuples have unspecified layout error: `extern` block uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:59:27 + --> $DIR/lint-ctypes.rs:58:27 | LL | pub fn tuple_type2(p: I32Pair); | ^^^^^^^ not FFI-safe @@ -112,7 +103,7 @@ LL | pub fn tuple_type2(p: I32Pair); = note: tuples have unspecified layout error: `extern` block uses type `ZeroSize`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:60:25 + --> $DIR/lint-ctypes.rs:59:25 | LL | pub fn zero_size(p: ZeroSize); | ^^^^^^^^ not FFI-safe @@ -126,7 +117,7 @@ LL | pub struct ZeroSize; | ^^^^^^^^^^^^^^^^^^^ error: `extern` block uses type `ZeroSizeWithPhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:61:33 + --> $DIR/lint-ctypes.rs:60:33 | LL | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -139,7 +130,7 @@ LL | pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` block uses type `PhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:64:12 + --> $DIR/lint-ctypes.rs:63:12 | LL | -> ::std::marker::PhantomData; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -147,7 +138,7 @@ LL | -> ::std::marker::PhantomData; = note: composed only of `PhantomData` error: `extern` block uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:65:23 + --> $DIR/lint-ctypes.rs:64:23 | LL | pub fn fn_type(p: RustFn); | ^^^^^^ not FFI-safe @@ -156,7 +147,7 @@ LL | pub fn fn_type(p: RustFn); = note: this function pointer has Rust-specific calling convention error: `extern` block uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:66:24 + --> $DIR/lint-ctypes.rs:65:24 | LL | pub fn fn_type2(p: fn()); | ^^^^ not FFI-safe @@ -165,7 +156,7 @@ LL | pub fn fn_type2(p: fn()); = note: this function pointer has Rust-specific calling convention error: `extern` block uses type `Box`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:67:28 + --> $DIR/lint-ctypes.rs:66:28 | LL | pub fn fn_contained(p: RustBadRet); | ^^^^^^^^^^ not FFI-safe @@ -174,7 +165,7 @@ LL | pub fn fn_contained(p: RustBadRet); = note: this struct has unspecified layout error: `extern` block uses type `str`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:68:31 + --> $DIR/lint-ctypes.rs:67:31 | LL | pub fn transparent_str(p: TransparentStr); | ^^^^^^^^^^^^^^ not FFI-safe @@ -183,7 +174,7 @@ LL | pub fn transparent_str(p: TransparentStr); = note: string slices have no C equivalent error: `extern` block uses type `Box`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:69:30 + --> $DIR/lint-ctypes.rs:68:30 | LL | pub fn transparent_fn(p: TransparentBadFn); | ^^^^^^^^^^^^^^^^ not FFI-safe @@ -192,7 +183,7 @@ LL | pub fn transparent_fn(p: TransparentBadFn); = note: this struct has unspecified layout error: `extern` block uses type `[u8; 8]`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:70:27 + --> $DIR/lint-ctypes.rs:69:27 | LL | pub fn raw_array(arr: [u8; 8]); | ^^^^^^^ not FFI-safe @@ -201,7 +192,7 @@ LL | pub fn raw_array(arr: [u8; 8]); = note: passing raw arrays by value is not FFI-safe error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:72:26 + --> $DIR/lint-ctypes.rs:71:26 | LL | pub fn no_niche_a(a: Option>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -210,7 +201,7 @@ LL | pub fn no_niche_a(a: Option>); = note: enum has no representation hint error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:74:26 + --> $DIR/lint-ctypes.rs:73:26 | LL | pub fn no_niche_b(b: Option>); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -218,5 +209,5 @@ LL | pub fn no_niche_b(b: Option>); = 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 22 previous errors +error: aborting due to 21 previous errors From 050c1197841082f1251251b717cad26690627a11 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sun, 24 Aug 2025 00:05:14 +0200 Subject: [PATCH 3/4] ImproperCTypes: re-separate linting and checking no visible changes to rust users, just making the inner architecture of the ImproperCTypes lints more sensible, with a clean separation between the struct (now singular) that interacts with the linting system and the struct (now singular) that visits the types to check FFI-safety --- compiler/rustc_lint/src/lib.rs | 3 +- compiler/rustc_lint/src/types.rs | 2 +- .../rustc_lint/src/types/improper_ctypes.rs | 511 ++++++++++-------- 3 files changed, 292 insertions(+), 224 deletions(-) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index bdbac7fc4d13f..9bb53fea54a18 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -194,8 +194,7 @@ late_lint_methods!( DefaultCouldBeDerived: DefaultCouldBeDerived::default(), DerefIntoDynSupertrait: DerefIntoDynSupertrait, DropForgetUseless: DropForgetUseless, - ImproperCTypesDeclarations: ImproperCTypesDeclarations, - ImproperCTypesDefinitions: ImproperCTypesDefinitions, + ImproperCTypesLint: ImproperCTypesLint, InvalidFromUtf8: InvalidFromUtf8, VariantSizeDifferences: VariantSizeDifferences, PathStatements: PathStatements, diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 979a863170100..a72b802eb5d13 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -11,7 +11,7 @@ use tracing::debug; use {rustc_ast as ast, rustc_hir as hir}; mod improper_ctypes; // these filed do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations -pub(crate) use improper_ctypes::{ImproperCTypesDeclarations, ImproperCTypesDefinitions}; +pub(crate) use improper_ctypes::ImproperCTypesLint; use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 7d7d86f93fc9a..0b6938affa10b 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -9,7 +9,7 @@ use rustc_hir::intravisit::VisitorExt; use rustc_hir::{self as hir, AmbigArg}; use rustc_middle::bug; use rustc_middle::ty::{ - self, Adt, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, + self, Adt, AdtDef, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, }; use rustc_session::{declare_lint, declare_lint_pass}; @@ -49,8 +49,6 @@ declare_lint! { "proper use of libc types in foreign modules" } -declare_lint_pass!(ImproperCTypesDeclarations => [IMPROPER_CTYPES]); - declare_lint! { /// The `improper_ctypes_definitions` lint detects incorrect use of /// [`extern` function] definitions. @@ -133,8 +131,11 @@ declare_lint! { "Structs do not follow the power alignment rule under repr(C)" } -declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS, USES_POWER_ALIGNMENT]); - +declare_lint_pass!(ImproperCTypesLint => [ + IMPROPER_CTYPES, + IMPROPER_CTYPES_DEFINITIONS, + USES_POWER_ALIGNMENT +]); /// Check a variant of a non-exhaustive enum for improper ctypes /// @@ -184,12 +185,17 @@ enum FfiResult<'tcx> { FfiUnsafe { ty: Ty<'tcx>, reason: DiagMessage, help: Option }, } +/// The result when a type has been checked but perhaps not completely. `None` indicates that +/// FFI safety/unsafety has not yet been determined, `Some(res)` indicates that the safety/unsafety +/// in the `FfiResult` is final. +type PartialFfiResult<'tcx> = Option>; + struct ImproperCTypesVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, mode: CItemKind, } -/// Accumulator for recursive ffi type checking +/// Accumulator for recursive ffi type checking. struct CTypesVisitorState<'tcx> { cache: FxHashSet>, /// The original type being checked, before we recursed @@ -198,21 +204,6 @@ struct CTypesVisitorState<'tcx> { } impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { - /// Check if the type is array and emit an unsafe type lint. - fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { - if let ty::Array(..) = ty.kind() { - self.emit_ffi_unsafe_type_lint( - ty, - sp, - fluent::lint_improper_ctypes_array_reason, - Some(fluent::lint_improper_ctypes_array_help), - ); - true - } else { - false - } - } - /// Checks if the given field's type is "ffi-safe". fn check_field_type_for_ffi( &self, @@ -226,7 +217,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { .tcx .try_normalize_erasing_regions(self.cx.typing_env(), field_ty) .unwrap_or(field_ty); - self.check_type_for_ffi(acc, field_ty) + self.visit_type(acc, field_ty) } /// Checks if the given `VariantDef`'s field types are "ffi-safe". @@ -280,11 +271,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { /// Checks if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). - fn check_type_for_ffi( - &self, - acc: &mut CTypesVisitorState<'tcx>, - ty: Ty<'tcx>, - ) -> FfiResult<'tcx> { + fn visit_type(&self, acc: &mut CTypesVisitorState<'tcx>, ty: Ty<'tcx>) -> FfiResult<'tcx> { use FfiResult::*; let tcx = self.cx.tcx; @@ -387,7 +374,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if let Some(ty) = repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty) { - return self.check_type_for_ffi(acc, ty); + return self.visit_type(acc, ty); } return FfiUnsafe { @@ -425,7 +412,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // It's just extra invariants on the type that you need to uphold, // but only the base type is relevant for being representable in FFI. - ty::Pat(base, ..) => self.check_type_for_ffi(acc, base), + ty::Pat(base, ..) => self.visit_type(acc, base), // Primitive types with a stable representation. ty::Bool | ty::Int(..) | ty::Uint(..) | ty::Float(..) | ty::Never => FfiSafe, @@ -470,9 +457,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiSafe } - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(acc, ty), + ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.visit_type(acc, ty), - ty::Array(inner_ty, _) => self.check_type_for_ffi(acc, inner_ty), + ty::Array(inner_ty, _) => self.visit_type(acc, inner_ty), ty::FnPtr(sig_tys, hdr) => { let sig = sig_tys.with(hdr); @@ -486,7 +473,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { let sig = tcx.instantiate_bound_regions_with_erased(sig); for arg in sig.inputs() { - match self.check_type_for_ffi(acc, *arg) { + match self.visit_type(acc, *arg) { FfiSafe => {} r => return r, } @@ -497,7 +484,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { return FfiSafe; } - self.check_type_for_ffi(acc, ret_ty) + self.visit_type(acc, ret_ty) } ty::Foreign(..) => FfiSafe, @@ -532,36 +519,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - fn emit_ffi_unsafe_type_lint( - &mut self, - ty: Ty<'tcx>, - sp: Span, - note: DiagMessage, - help: Option, - ) { - let lint = match self.mode { - CItemKind::Declaration => IMPROPER_CTYPES, - CItemKind::Definition => IMPROPER_CTYPES_DEFINITIONS, - }; - let desc = match self.mode { - CItemKind::Declaration => "block", - CItemKind::Definition => "fn", - }; - let span_note = if let ty::Adt(def, _) = ty.kind() - && let Some(sp) = self.cx.tcx.hir_span_if_local(def.did()) - { - Some(sp) - } else { - None - }; - self.cx.emit_span_lint( - lint, - sp, - ImproperCTypes { ty, desc, label: sp, help, note, span_note }, - ); - } - - fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { + fn visit_for_opaque_ty(&mut self, ty: Ty<'tcx>) -> PartialFfiResult<'tcx> { struct ProhibitOpaqueTypes; impl<'tcx> ty::TypeVisitor> for ProhibitOpaqueTypes { type Result = ControlFlow>; @@ -587,23 +545,37 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { .visit_with(&mut ProhibitOpaqueTypes) .break_value() { - self.emit_ffi_unsafe_type_lint(ty, sp, fluent::lint_improper_ctypes_opaque, None); - true + Some(FfiResult::FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_opaque, help: None }) } else { - false + None + } + } + + /// Check if the type is array and emit an unsafe type lint. + fn check_for_array_ty(&mut self, ty: Ty<'tcx>) -> PartialFfiResult<'tcx> { + if let ty::Array(..) = ty.kind() { + Some(FfiResult::FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_array_reason, + help: Some(fluent::lint_improper_ctypes_array_help), + }) + } else { + None } } - fn check_type_for_ffi_and_report_errors( + /// Determine the FFI-safety of a single (MIR) type, given the context of how it is used. + fn check_type( &mut self, - sp: Span, ty: Ty<'tcx>, is_static: bool, is_return_type: bool, - ) { - if self.check_for_opaque_ty(sp, ty) { - // We've already emitted an error due to an opaque type. - return; + ) -> FfiResult<'tcx> { + match self.visit_for_opaque_ty(ty) { + None => {} + Some(res) => { + return res; + } } let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.typing_env(), ty).unwrap_or(ty); @@ -611,82 +583,109 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // C doesn't really support passing arrays by value - the only way to pass an array by value // is through a struct. So, first test that the top level isn't an array, and then // recursively check the types inside. - if !is_static && self.check_for_array_ty(sp, ty) { - return; + if !is_static { + match self.check_for_array_ty(ty) { + None => {} + Some(res) => { + return res; + } + } } // Don't report FFI errors for unit return types. This check exists here, and not in // the caller (where it would make more sense) so that normalization has definitely // happened. if is_return_type && ty.is_unit() { - return; + return FfiResult::FfiSafe; } - let mut acc = CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty }; - match self.check_type_for_ffi(&mut acc, ty) { - FfiResult::FfiSafe => {} - FfiResult::FfiPhantom(ty) => { - self.emit_ffi_unsafe_type_lint( - ty, - sp, - fluent::lint_improper_ctypes_only_phantomdata, - None, - ); - } - FfiResult::FfiUnsafe { ty, reason, help } => { - self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); - } - } + let mut acc = CTypesVisitorState { cache: Default::default(), base_ty: ty }; + self.visit_type(&mut acc, ty) } - /// Check if a function's argument types and result type are "ffi-safe". - /// - /// For a external ABI function, argument types and the result type are walked to find fn-ptr - /// types that have external ABIs, as these still need checked. - fn check_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { - let sig = self.cx.tcx.fn_sig(def_id).instantiate_identity(); - let sig = self.cx.tcx.instantiate_bound_regions_with_erased(sig); - - for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { - for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(input_hir, *input_ty) { - self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, false); - } - } - - if let hir::FnRetTy::Return(ret_hir) = decl.output { - for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(ret_hir, sig.output()) { - self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, true); + /// Per-struct-field function that checks if a struct definition follows + /// the Power alignment Rule (see the `check_struct_for_power_alignment` method). + fn check_arg_for_power_alignment(&mut self, ty: Ty<'tcx>) -> bool { + let tcx = self.cx.tcx; + assert!(tcx.sess.target.os == "aix"); + // Structs (under repr(C)) follow the power alignment rule if: + // - the first field of the struct is a floating-point type that + // is greater than 4-bytes, or + // - the first field of the struct is an aggregate whose + // recursively first field is a floating-point type greater than + // 4 bytes. + if ty.is_floating_point() && ty.primitive_size(tcx).bytes() > 4 { + return true; + } else if let Adt(adt_def, _) = ty.kind() + && adt_def.is_struct() + && adt_def.repr().c() + && !adt_def.repr().packed() + && adt_def.repr().align.is_none() + { + let struct_variant = adt_def.variant(VariantIdx::ZERO); + // Within a nested struct, all fields are examined to correctly + // report if any fields after the nested struct within the + // original struct are misaligned. + for struct_field in &struct_variant.fields { + let field_ty = tcx.type_of(struct_field.did).instantiate_identity(); + if self.check_arg_for_power_alignment(field_ty) { + return true; + } } } + return false; } - /// Check if a function's argument types and result type are "ffi-safe". - fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { - let sig = self.cx.tcx.fn_sig(def_id).instantiate_identity(); - let sig = self.cx.tcx.instantiate_bound_regions_with_erased(sig); - - for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { - self.check_type_for_ffi_and_report_errors(input_hir.span, *input_ty, false, false); - } - - if let hir::FnRetTy::Return(ret_hir) = decl.output { - self.check_type_for_ffi_and_report_errors(ret_hir.span, sig.output(), false, true); + /// Check a struct definition for respect of the Power alignment Rule (as in PowerPC), + /// which should be respected in the "aix" target OS. + /// To do so, we must follow one of the two following conditions: + /// - The first field of the struct must be floating-point type that + /// is greater than 4-bytes. + /// - The first field of the struct must be an aggregate whose + /// recursively first field is a floating-point type greater than + /// 4 bytes. + fn check_struct_for_power_alignment(&mut self, item: &'tcx hir::Item<'tcx>) { + let tcx = self.cx.tcx; + let adt_def = tcx.adt_def(item.owner_id.to_def_id()); + // repr(C) structs also with packed or aligned representation + // should be ignored. + if adt_def.repr().c() + && !adt_def.repr().packed() + && adt_def.repr().align.is_none() + && tcx.sess.target.os == "aix" + && !adt_def.all_fields().next().is_none() + { + let struct_variant_data = item.expect_struct().2; + for field_def in struct_variant_data.fields().iter().skip(1) { + // Struct fields (after the first field) are checked for the + // power alignment rule, as fields after the first are likely + // to be the fields that are misaligned. + let def_id = field_def.def_id; + let ty = tcx.type_of(def_id).instantiate_identity(); + if self.check_arg_for_power_alignment(ty) { + self.cx.emit_span_lint( + USES_POWER_ALIGNMENT, + field_def.span, + UsesPowerAlignment, + ); + } + } } } +} - fn check_foreign_static(&mut self, id: hir::OwnerId, span: Span) { - let ty = self.cx.tcx.type_of(id).instantiate_identity(); - self.check_type_for_ffi_and_report_errors(span, ty, true, false); - } - - /// Find any fn-ptr types with external ABIs in `ty`. - /// - /// For example, `Option` returns `extern "C" fn()` - fn find_fn_ptr_ty_with_external_abi( - &self, +impl<'tcx> ImproperCTypesLint { + /// Find any fn-ptr types with external ABIs in `ty`, and FFI-checks them. + /// For example, `Option` FFI-checks `extern "C" fn()`. + fn check_type_for_external_abi_fnptr( + &mut self, + cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'tcx>, ty: Ty<'tcx>, - ) -> Vec<(Ty<'tcx>, Span)> { + fn_mode: CItemKind, + is_static: bool, + is_return: bool, + ) { struct FnPtrFinder<'tcx> { spans: Vec, tys: Vec>, @@ -723,105 +722,138 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty.visit_with(&mut visitor); visitor.visit_ty_unambig(hir_ty); - iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)).collect() + let all_types = iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)); + for (fn_ptr_ty, span) in all_types { + let mut visitor = ImproperCTypesVisitor { cx, mode: fn_mode }; + // TODO: make a check_for_fnptr + let ffi_res = visitor.check_type(fn_ptr_ty, is_static, is_return); + + self.process_ffi_result(cx, span, ffi_res, fn_mode); + } } -} -impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations { - fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, it: &hir::ForeignItem<'tcx>) { - let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration }; - let abi = cx.tcx.hir_get_foreign_abi(it.hir_id()); + /// Regardless of a function's need to be "ffi-safe", look for fn-ptr argument/return types + /// that need to be checked for ffi-safety. + fn check_fn_for_external_abi_fnptr( + &mut self, + cx: &LateContext<'tcx>, + fn_mode: CItemKind, + def_id: LocalDefId, + decl: &'tcx hir::FnDecl<'_>, + ) { + let sig = cx.tcx.fn_sig(def_id).instantiate_identity(); + let sig = cx.tcx.instantiate_bound_regions_with_erased(sig); - match it.kind { - hir::ForeignItemKind::Fn(sig, _, _) => { - if abi.is_rustic_abi() { - vis.check_fn(it.owner_id.def_id, sig.decl) - } else { - vis.check_foreign_fn(it.owner_id.def_id, sig.decl); - } - } - hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => { - vis.check_foreign_static(it.owner_id, ty.span); - } - hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (), + for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { + self.check_type_for_external_abi_fnptr(cx, input_hir, *input_ty, fn_mode, false, false); + } + + if let hir::FnRetTy::Return(ret_hir) = decl.output { + self.check_type_for_external_abi_fnptr(cx, ret_hir, sig.output(), fn_mode, false, true); } } -} -impl ImproperCTypesDefinitions { - fn check_ty_maybe_containing_foreign_fnptr<'tcx>( + /// For a local definition of a #[repr(C)] struct/enum/union, check that it is indeed FFI-safe. + fn check_reprc_adt( &mut self, cx: &LateContext<'tcx>, - hir_ty: &'tcx hir::Ty<'_>, - ty: Ty<'tcx>, + item: &'tcx hir::Item<'tcx>, + adt_def: AdtDef<'tcx>, ) { - let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; - for (fn_ptr_ty, span) in vis.find_fn_ptr_ty_with_external_abi(hir_ty, ty) { - vis.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, true, false); - } + debug_assert!( + adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none() + ); + + let mut visitor = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration }; + + // FIXME(ctypes): this following call is awkward. + // is there a way to perform its logic in MIR space rather than HIR space? + // (so that its logic can be absorbed into visitor.visit_struct_or_union) + visitor.check_struct_for_power_alignment(item); + } + + 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 { cx, mode: CItemKind::Declaration }; + let ffi_res = visitor.check_type(ty, true, false); + self.process_ffi_result(cx, span, ffi_res, CItemKind::Declaration); } - fn check_arg_for_power_alignment<'tcx>( + /// Check if a function's argument types and result type are "ffi-safe". + fn check_foreign_fn( &mut self, cx: &LateContext<'tcx>, - ty: Ty<'tcx>, - ) -> bool { - assert!(cx.tcx.sess.target.os == "aix"); - // Structs (under repr(C)) follow the power alignment rule if: - // - the first field of the struct is a floating-point type that - // is greater than 4-bytes, or - // - the first field of the struct is an aggregate whose - // recursively first field is a floating-point type greater than - // 4 bytes. - if ty.is_floating_point() && ty.primitive_size(cx.tcx).bytes() > 4 { - return true; - } else if let Adt(adt_def, _) = ty.kind() - && adt_def.is_struct() - && adt_def.repr().c() - && !adt_def.repr().packed() - && adt_def.repr().align.is_none() - { - let struct_variant = adt_def.variant(VariantIdx::ZERO); - // Within a nested struct, all fields are examined to correctly - // report if any fields after the nested struct within the - // original struct are misaligned. - for struct_field in &struct_variant.fields { - let field_ty = cx.tcx.type_of(struct_field.did).instantiate_identity(); - if self.check_arg_for_power_alignment(cx, field_ty) { - return true; - } - } + fn_mode: CItemKind, + def_id: LocalDefId, + decl: &'tcx hir::FnDecl<'_>, + ) { + let sig = cx.tcx.fn_sig(def_id).instantiate_identity(); + let sig = cx.tcx.instantiate_bound_regions_with_erased(sig); + + for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { + let mut visitor = ImproperCTypesVisitor { cx, mode: fn_mode }; + let ffi_res = visitor.check_type(*input_ty, false, false); + self.process_ffi_result(cx, input_hir.span, ffi_res, fn_mode); + } + + if let hir::FnRetTy::Return(ret_hir) = decl.output { + let mut visitor = ImproperCTypesVisitor { cx, mode: fn_mode }; + let ffi_res = visitor.check_type(sig.output(), false, true); + self.process_ffi_result(cx, ret_hir.span, ffi_res, fn_mode); } - return false; } - fn check_struct_for_power_alignment<'tcx>( - &mut self, + fn process_ffi_result( + &self, cx: &LateContext<'tcx>, - item: &'tcx hir::Item<'tcx>, + sp: Span, + res: FfiResult<'tcx>, + fn_mode: CItemKind, ) { - let adt_def = cx.tcx.adt_def(item.owner_id.to_def_id()); - // repr(C) structs also with packed or aligned representation - // should be ignored. - if adt_def.repr().c() - && !adt_def.repr().packed() - && adt_def.repr().align.is_none() - && cx.tcx.sess.target.os == "aix" - && !adt_def.all_fields().next().is_none() - { - let struct_variant_data = item.expect_struct().2; - for field_def in struct_variant_data.fields().iter().skip(1) { - // Struct fields (after the first field) are checked for the - // power alignment rule, as fields after the first are likely - // to be the fields that are misaligned. - let def_id = field_def.def_id; - let ty = cx.tcx.type_of(def_id).instantiate_identity(); - if self.check_arg_for_power_alignment(cx, ty) { - cx.emit_span_lint(USES_POWER_ALIGNMENT, field_def.span, UsesPowerAlignment); - } + match res { + FfiResult::FfiSafe => {} + FfiResult::FfiPhantom(ty) => { + self.emit_ffi_unsafe_type_lint( + cx, + ty, + sp, + fluent::lint_improper_ctypes_only_phantomdata, + None, + fn_mode, + ); + } + FfiResult::FfiUnsafe { ty, reason, help } => { + self.emit_ffi_unsafe_type_lint(cx, ty, sp, reason, help, fn_mode); } } } + + fn emit_ffi_unsafe_type_lint( + &self, + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + sp: Span, + note: DiagMessage, + help: Option, + fn_mode: CItemKind, + ) { + let lint = match fn_mode { + CItemKind::Declaration => IMPROPER_CTYPES, + CItemKind::Definition => IMPROPER_CTYPES_DEFINITIONS, + }; + let desc = match fn_mode { + CItemKind::Declaration => "block", + CItemKind::Definition => "fn", + }; + let span_note = if let ty::Adt(def, _) = ty.kind() + && let Some(sp) = cx.tcx.hir_span_if_local(def.did()) + { + Some(sp) + } else { + None + }; + cx.emit_span_lint(lint, sp, ImproperCTypes { ty, desc, label: sp, help, note, span_note }); + } } /// `ImproperCTypesDefinitions` checks items outside of foreign items (e.g. stuff that isn't in @@ -831,27 +863,59 @@ impl ImproperCTypesDefinitions { /// `ImproperCtypesDeclarations` visitor checks functions if `` is external (e.g. "C"). /// - All other items which contain types (e.g. other functions, struct definitions, etc) are /// checked for extern fn-ptrs with external ABIs. -impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { +impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint { + fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, it: &hir::ForeignItem<'tcx>) { + let abi = cx.tcx.hir_get_foreign_abi(it.hir_id()); + + match it.kind { + hir::ForeignItemKind::Fn(sig, _, _) => { + // fnptrs are a special case, they always need to be treated as + // "the element rendered unsafe" because their unsafety doesn't affect + // their surroundings, and their type is often declared inline + if !abi.is_rustic_abi() { + self.check_foreign_fn(cx, CItemKind::Declaration, it.owner_id.def_id, sig.decl); + } else { + self.check_fn_for_external_abi_fnptr( + cx, + CItemKind::Declaration, + it.owner_id.def_id, + sig.decl, + ); + } + } + hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => { + self.check_foreign_static(cx, it.owner_id, ty.span); + } + hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (), + } + } + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { match item.kind { hir::ItemKind::Static(_, _, ty, _) | hir::ItemKind::Const(_, _, ty, _) | hir::ItemKind::TyAlias(_, _, ty) => { - self.check_ty_maybe_containing_foreign_fnptr( + self.check_type_for_external_abi_fnptr( cx, ty, cx.tcx.type_of(item.owner_id).instantiate_identity(), + CItemKind::Definition, + true, + false, ); } - // See `check_fn`.. + // See `check_fn` for declarations, `check_foreign_items` for definitions in extern blocks hir::ItemKind::Fn { .. } => {} - // Structs are checked based on if they follow the power alignment - // rule (under repr(C)). - hir::ItemKind::Struct(..) => { - self.check_struct_for_power_alignment(cx, item); + hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) | hir::ItemKind::Enum(..) => { + // looking for extern FnPtr:s is delegated to `check_field_def`. + let adt_def: AdtDef<'tcx> = cx.tcx.adt_def(item.owner_id.to_def_id()); + + if adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none() + { + self.check_reprc_adt(cx, item, adt_def); + } } - // See `check_field_def`.. - hir::ItemKind::Union(..) | hir::ItemKind::Enum(..) => {} + // Doesn't define something that can contain a external type to be checked. hir::ItemKind::Impl(..) | hir::ItemKind::TraitAlias(..) @@ -866,10 +930,13 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { } fn check_field_def(&mut self, cx: &LateContext<'tcx>, field: &'tcx hir::FieldDef<'tcx>) { - self.check_ty_maybe_containing_foreign_fnptr( + self.check_type_for_external_abi_fnptr( cx, field.ty, cx.tcx.type_of(field.def_id).instantiate_identity(), + CItemKind::Definition, + true, + false, ); } @@ -890,11 +957,13 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { _ => return, }; - let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; - if abi.is_rustic_abi() { - vis.check_fn(id, decl); + // fnptrs are a special case, they always need to be treated as + // "the element rendered unsafe" because their unsafety doesn't affect + // their surroundings, and their type is often declared inline + if !abi.is_rustic_abi() { + self.check_foreign_fn(cx, CItemKind::Definition, id, decl); } else { - vis.check_foreign_fn(id, decl); + self.check_fn_for_external_abi_fnptr(cx, CItemKind::Definition, id, decl); } } } From 20f050b44431ac1e0dd2a6285b736a0d5c046ce3 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sun, 24 Aug 2025 14:23:24 +0200 Subject: [PATCH 4/4] ImproperCTypes: redo state tracking No changes should be visible by rustc users This is just some architecture changes to the type checking to facilitate FFI-safety decisions that depend on how the type is used (the change here is not complete, there are still bits of "legacy" state passing for this, but since this is a retconned commit, I can tell you those bits will disappear before the end of the commit chain) (there is at least one bit where the decision making code is weird, but that this is because we do not want to change the lint's behaviour this early in the chain) --- Cargo.lock | 1 + compiler/rustc_lint/Cargo.toml | 1 + .../rustc_lint/src/types/improper_ctypes.rs | 362 +++++++++++------- 3 files changed, 226 insertions(+), 138 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c4c96f1569b6f..6db512c3137d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4022,6 +4022,7 @@ dependencies = [ name = "rustc_lint" version = "0.0.0" dependencies = [ + "bitflags", "rustc_abi", "rustc_ast", "rustc_ast_pretty", diff --git a/compiler/rustc_lint/Cargo.toml b/compiler/rustc_lint/Cargo.toml index 7718f16984dab..3a50aac50cb3e 100644 --- a/compiler/rustc_lint/Cargo.toml +++ b/compiler/rustc_lint/Cargo.toml @@ -5,6 +5,7 @@ edition = "2024" [dependencies] # tidy-alphabetical-start +bitflags = "2.4.1" rustc_abi = { path = "../rustc_abi" } rustc_ast = { path = "../rustc_ast" } rustc_ast_pretty = { path = "../rustc_ast_pretty" } diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 0b6938affa10b..7ca57b0094ee3 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -1,6 +1,7 @@ use std::iter; use std::ops::ControlFlow; +use bitflags::bitflags; use rustc_abi::VariantIdx; use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; @@ -21,7 +22,6 @@ use super::repr_nullable_ptr; use crate::lints::{ImproperCTypes, UsesPowerAlignment}; use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; - declare_lint! { /// The `improper_ctypes` lint detects incorrect use of types in foreign /// modules. @@ -173,6 +173,75 @@ fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool { !matches!(variant.ctor_kind(), Some(CtorKind::Const)) } +/// Per-struct-field function that checks if a struct definition follows +/// the Power alignment Rule (see the `check_struct_for_power_alignment` function). +fn check_arg_for_power_alignment<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + let tcx = cx.tcx; + assert!(tcx.sess.target.os == "aix"); + // Structs (under repr(C)) follow the power alignment rule if: + // - the first field of the struct is a floating-point type that + // is greater than 4-bytes, or + // - the first field of the struct is an aggregate whose + // recursively first field is a floating-point type greater than + // 4 bytes. + if ty.is_floating_point() && ty.primitive_size(tcx).bytes() > 4 { + return true; + } else if let Adt(adt_def, _) = ty.kind() + && adt_def.is_struct() + && adt_def.repr().c() + && !adt_def.repr().packed() + && adt_def.repr().align.is_none() + { + let struct_variant = adt_def.variant(VariantIdx::ZERO); + // Within a nested struct, all fields are examined to correctly + // report if any fields after the nested struct within the + // original struct are misaligned. + for struct_field in &struct_variant.fields { + let field_ty = tcx.type_of(struct_field.did).instantiate_identity(); + if check_arg_for_power_alignment(cx, field_ty) { + return true; + } + } + } + return false; +} + +/// Check a struct definition for respect of the Power alignment Rule (as in PowerPC), +/// which should be respected in the "aix" target OS. +/// To do so, we must follow one of the two following conditions: +/// - The first field of the struct must be floating-point type that +/// is greater than 4-bytes. +/// - The first field of the struct must be an aggregate whose +/// recursively first field is a floating-point type greater than +/// 4 bytes. +fn check_struct_for_power_alignment<'tcx>( + cx: &LateContext<'tcx>, + item: &'tcx hir::Item<'tcx>, + adt_def: AdtDef<'tcx>, +) { + let tcx = cx.tcx; + // repr(C) structs also with packed or aligned representation + // should be ignored. + if adt_def.repr().c() + && !adt_def.repr().packed() + && adt_def.repr().align.is_none() + && tcx.sess.target.os == "aix" + && !adt_def.all_fields().next().is_none() + { + let struct_variant_data = item.expect_struct().2; + for field_def in struct_variant_data.fields().iter().skip(1) { + // Struct fields (after the first field) are checked for the + // power alignment rule, as fields after the first are likely + // to be the fields that are misaligned. + let def_id = field_def.def_id; + let ty = tcx.type_of(def_id).instantiate_identity(); + if check_arg_for_power_alignment(cx, ty) { + cx.emit_span_lint(USES_POWER_ALIGNMENT, field_def.span, UsesPowerAlignment); + } + } + } +} + #[derive(Clone, Copy)] enum CItemKind { Declaration, @@ -190,24 +259,115 @@ enum FfiResult<'tcx> { /// in the `FfiResult` is final. type PartialFfiResult<'tcx> = Option>; -struct ImproperCTypesVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - mode: CItemKind, +bitflags! { + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + struct VisitorState: u8 { + /// For use in (externally-linked) static variables. + const STATIC = 0b000001; + /// For use in functions in general. + const FUNC = 0b000010; + /// For variables in function returns (implicitly: not for static variables). + const FN_RETURN = 0b000100; + /// For variables in functions/variables which are defined in rust. + const DEFINED = 0b001000; + /// For times where we are only defining the type of something + /// (struct/enum/union definitions, FnPtrs). + const THEORETICAL = 0b010000; + } } -/// Accumulator for recursive ffi type checking. -struct CTypesVisitorState<'tcx> { +impl VisitorState { + // The values that can be set. + const STATIC_TY: Self = Self::STATIC; + const ARGUMENT_TY_IN_DEFINITION: Self = + Self::from_bits(Self::FUNC.bits() | Self::DEFINED.bits()).unwrap(); + const RETURN_TY_IN_DEFINITION: Self = + Self::from_bits(Self::FUNC.bits() | Self::FN_RETURN.bits() | Self::DEFINED.bits()).unwrap(); + const ARGUMENT_TY_IN_DECLARATION: Self = Self::FUNC; + const RETURN_TY_IN_DECLARATION: Self = + Self::from_bits(Self::FUNC.bits() | Self::FN_RETURN.bits()).unwrap(); + const ARGUMENT_TY_IN_FNPTR: Self = + Self::from_bits(Self::FUNC.bits() | Self::THEORETICAL.bits()).unwrap(); + const RETURN_TY_IN_FNPTR: Self = + Self::from_bits(Self::FUNC.bits() | Self::THEORETICAL.bits() | Self::FN_RETURN.bits()) + .unwrap(); + + /// Get the proper visitor state for a given function's arguments. + fn argument_from_fnmode(fn_mode: CItemKind) -> Self { + match fn_mode { + CItemKind::Definition => VisitorState::ARGUMENT_TY_IN_DEFINITION, + CItemKind::Declaration => VisitorState::ARGUMENT_TY_IN_DECLARATION, + } + } + + /// Get the proper visitor state for a given function's return type. + fn return_from_fnmode(fn_mode: CItemKind) -> Self { + match fn_mode { + CItemKind::Definition => VisitorState::RETURN_TY_IN_DEFINITION, + CItemKind::Declaration => VisitorState::RETURN_TY_IN_DECLARATION, + } + } + + /// Whether the type is used in a function. + fn is_in_function(self) -> bool { + let ret = self.contains(Self::FUNC); + if ret { + debug_assert!(!self.contains(Self::STATIC)); + } + ret + } + /// Whether the type is used (directly or not) in a function, in return position. + fn is_in_function_return(self) -> bool { + let ret = self.contains(Self::FN_RETURN); + if ret { + debug_assert!(self.is_in_function()); + } + ret + } + /// Whether the type is used (directly or not) in a defined function. + /// In other words, whether or not we allow non-FFI-safe types behind a C pointer, + /// to be treated as an opaque type on the other side of the FFI boundary. + fn is_in_defined_function(self) -> bool { + self.contains(Self::DEFINED) && self.is_in_function() + } + + /// Whether the type is used (directly or not) in a function pointer type. + /// Here, we also allow non-FFI-safe types behind a C pointer, + /// to be treated as an opaque type on the other side of the FFI boundary. + fn is_in_fnptr(self) -> bool { + self.contains(Self::THEORETICAL) && self.is_in_function() + } + + /// Whether we can expect type parameters and co in a given type. + fn can_expect_ty_params(self) -> bool { + // rust-defined functions, as well as FnPtrs + self.contains(Self::THEORETICAL) || self.is_in_defined_function() + } +} + +/// Visitor used to recursively traverse MIR types and evaluate FFI-safety. +/// It uses ``check_*`` methods as entrypoints to be called elsewhere, +/// and ``visit_*`` methods to recurse. +struct ImproperCTypesVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + /// To prevent problems with recursive types, + /// add a types-in-check cache. cache: FxHashSet>, /// The original type being checked, before we recursed /// to any other types it contains. base_ty: Ty<'tcx>, + base_fn_mode: CItemKind, } impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { + fn new(cx: &'a LateContext<'tcx>, base_ty: Ty<'tcx>, base_fn_mode: CItemKind) -> Self { + Self { cx, base_ty, base_fn_mode, cache: FxHashSet::default() } + } + /// Checks if the given field's type is "ffi-safe". fn check_field_type_for_ffi( - &self, - acc: &mut CTypesVisitorState<'tcx>, + &mut self, + state: VisitorState, field: &ty::FieldDef, args: GenericArgsRef<'tcx>, ) -> FfiResult<'tcx> { @@ -217,13 +377,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { .tcx .try_normalize_erasing_regions(self.cx.typing_env(), field_ty) .unwrap_or(field_ty); - self.visit_type(acc, field_ty) + self.visit_type(state, field_ty) } /// Checks if the given `VariantDef`'s field types are "ffi-safe". fn check_variant_for_ffi( - &self, - acc: &mut CTypesVisitorState<'tcx>, + &mut self, + state: VisitorState, ty: Ty<'tcx>, def: ty::AdtDef<'tcx>, variant: &ty::VariantDef, @@ -233,7 +393,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { let transparent_with_all_zst_fields = if def.repr().transparent() { 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.. - match self.check_field_type_for_ffi(acc, field, args) { + match self.check_field_type_for_ffi(state, field, args) { FfiUnsafe { ty, .. } if ty.is_unit() => (), r => return r, } @@ -251,7 +411,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // We can't completely trust `repr(C)` markings, so make sure the fields are actually safe. let mut all_phantom = !variant.fields.is_empty(); for field in &variant.fields { - all_phantom &= match self.check_field_type_for_ffi(acc, field, args) { + all_phantom &= match self.check_field_type_for_ffi(state, field, args) { FfiSafe => false, // `()` fields are FFI-safe! FfiUnsafe { ty, .. } if ty.is_unit() => false, @@ -271,7 +431,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { /// Checks if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). - fn visit_type(&self, acc: &mut CTypesVisitorState<'tcx>, ty: Ty<'tcx>) -> FfiResult<'tcx> { + fn visit_type(&mut self, state: VisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> { use FfiResult::*; let tcx = self.cx.tcx; @@ -280,14 +440,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // `struct S(*mut S);`. // FIXME: A recursion limit is necessary as well, for irregular // recursive types. - if !acc.cache.insert(ty) { + if !self.cache.insert(ty) { return FfiSafe; } match *ty.kind() { ty::Adt(def, args) => { if let Some(boxed) = ty.boxed_ty() - && matches!(self.mode, CItemKind::Definition) + && ( + // FIXME(ctypes): this logic is broken, but it still fits the current tests + state.is_in_defined_function() + || (state.is_in_fnptr() + && matches!(self.base_fn_mode, CItemKind::Definition)) + ) { if boxed.is_sized(tcx, self.cx.typing_env()) { return FfiSafe; @@ -306,7 +471,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { AdtKind::Struct | AdtKind::Union => { if let Some(sym::cstring_type | sym::cstr_type) = tcx.get_diagnostic_name(def.did()) - && !acc.base_ty.is_mutable_ptr() + && !self.base_ty.is_mutable_ptr() { return FfiUnsafe { ty, @@ -359,7 +524,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }; } - self.check_variant_for_ffi(acc, ty, def, def.non_enum_variant(), args) + self.check_variant_for_ffi(state, ty, def, def.non_enum_variant(), args) } AdtKind::Enum => { if def.variants().is_empty() { @@ -374,7 +539,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if let Some(ty) = repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty) { - return self.visit_type(acc, ty); + return self.visit_type(state, ty); } return FfiUnsafe { @@ -390,7 +555,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { check_non_exhaustive_variant(non_exhaustive, variant) .map_break(|reason| FfiUnsafe { ty, reason, help: None })?; - match self.check_variant_for_ffi(acc, ty, def, variant, args) { + match self.check_variant_for_ffi(state, ty, def, variant, args) { FfiSafe => ControlFlow::Continue(()), r => ControlFlow::Break(r), } @@ -412,7 +577,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // It's just extra invariants on the type that you need to uphold, // but only the base type is relevant for being representable in FFI. - ty::Pat(base, ..) => self.visit_type(acc, base), + ty::Pat(base, ..) => self.visit_type(state, base), // Primitive types with a stable representation. ty::Bool | ty::Int(..) | ty::Uint(..) | ty::Float(..) | ty::Never => FfiSafe, @@ -441,7 +606,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::RawPtr(ty, _) | ty::Ref(_, ty, _) if { - matches!(self.mode, CItemKind::Definition) + (state.is_in_defined_function() || state.is_in_fnptr()) && ty.is_sized(self.cx.tcx, self.cx.typing_env()) } => { @@ -457,9 +622,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiSafe } - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.visit_type(acc, ty), + ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.visit_type(state, ty), - ty::Array(inner_ty, _) => self.visit_type(acc, inner_ty), + ty::Array(inner_ty, _) => self.visit_type(state, inner_ty), ty::FnPtr(sig_tys, hdr) => { let sig = sig_tys.with(hdr); @@ -473,7 +638,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { let sig = tcx.instantiate_bound_regions_with_erased(sig); for arg in sig.inputs() { - match self.visit_type(acc, *arg) { + match self.visit_type(VisitorState::ARGUMENT_TY_IN_FNPTR, *arg) { FfiSafe => {} r => return r, } @@ -484,7 +649,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { return FfiSafe; } - self.visit_type(acc, ret_ty) + self.visit_type(VisitorState::RETURN_TY_IN_FNPTR, ret_ty) } ty::Foreign(..) => FfiSafe, @@ -498,7 +663,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe, // so they are currently ignored for the purposes of this lint. ty::Param(..) | ty::Alias(ty::Projection | ty::Inherent, ..) - if matches!(self.mode, CItemKind::Definition) => + if state.can_expect_ty_params() => { FfiSafe } @@ -545,7 +710,11 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { .visit_with(&mut ProhibitOpaqueTypes) .break_value() { - Some(FfiResult::FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_opaque, help: None }) + Some(FfiResult::FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_opaque, + help: None, + }) } else { None } @@ -565,17 +734,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } /// Determine the FFI-safety of a single (MIR) type, given the context of how it is used. - fn check_type( - &mut self, - ty: Ty<'tcx>, - is_static: bool, - is_return_type: bool, - ) -> FfiResult<'tcx> { - match self.visit_for_opaque_ty(ty) { - None => {} - Some(res) => { - return res; - } + fn check_type(&mut self, state: VisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> { + if let Some(res) = self.visit_for_opaque_ty(ty) { + return res; } let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.typing_env(), ty).unwrap_or(ty); @@ -583,94 +744,20 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // C doesn't really support passing arrays by value - the only way to pass an array by value // is through a struct. So, first test that the top level isn't an array, and then // recursively check the types inside. - if !is_static { - match self.check_for_array_ty(ty) { - None => {} - Some(res) => { - return res; - } + if state.is_in_function() { + if let Some(res) = self.check_for_array_ty(ty) { + return res; } } // Don't report FFI errors for unit return types. This check exists here, and not in // the caller (where it would make more sense) so that normalization has definitely // happened. - if is_return_type && ty.is_unit() { + if state.is_in_function_return() && ty.is_unit() { return FfiResult::FfiSafe; } - let mut acc = CTypesVisitorState { cache: Default::default(), base_ty: ty }; - self.visit_type(&mut acc, ty) - } - - /// Per-struct-field function that checks if a struct definition follows - /// the Power alignment Rule (see the `check_struct_for_power_alignment` method). - fn check_arg_for_power_alignment(&mut self, ty: Ty<'tcx>) -> bool { - let tcx = self.cx.tcx; - assert!(tcx.sess.target.os == "aix"); - // Structs (under repr(C)) follow the power alignment rule if: - // - the first field of the struct is a floating-point type that - // is greater than 4-bytes, or - // - the first field of the struct is an aggregate whose - // recursively first field is a floating-point type greater than - // 4 bytes. - if ty.is_floating_point() && ty.primitive_size(tcx).bytes() > 4 { - return true; - } else if let Adt(adt_def, _) = ty.kind() - && adt_def.is_struct() - && adt_def.repr().c() - && !adt_def.repr().packed() - && adt_def.repr().align.is_none() - { - let struct_variant = adt_def.variant(VariantIdx::ZERO); - // Within a nested struct, all fields are examined to correctly - // report if any fields after the nested struct within the - // original struct are misaligned. - for struct_field in &struct_variant.fields { - let field_ty = tcx.type_of(struct_field.did).instantiate_identity(); - if self.check_arg_for_power_alignment(field_ty) { - return true; - } - } - } - return false; - } - - /// Check a struct definition for respect of the Power alignment Rule (as in PowerPC), - /// which should be respected in the "aix" target OS. - /// To do so, we must follow one of the two following conditions: - /// - The first field of the struct must be floating-point type that - /// is greater than 4-bytes. - /// - The first field of the struct must be an aggregate whose - /// recursively first field is a floating-point type greater than - /// 4 bytes. - fn check_struct_for_power_alignment(&mut self, item: &'tcx hir::Item<'tcx>) { - let tcx = self.cx.tcx; - let adt_def = tcx.adt_def(item.owner_id.to_def_id()); - // repr(C) structs also with packed or aligned representation - // should be ignored. - if adt_def.repr().c() - && !adt_def.repr().packed() - && adt_def.repr().align.is_none() - && tcx.sess.target.os == "aix" - && !adt_def.all_fields().next().is_none() - { - let struct_variant_data = item.expect_struct().2; - for field_def in struct_variant_data.fields().iter().skip(1) { - // Struct fields (after the first field) are checked for the - // power alignment rule, as fields after the first are likely - // to be the fields that are misaligned. - let def_id = field_def.def_id; - let ty = tcx.type_of(def_id).instantiate_identity(); - if self.check_arg_for_power_alignment(ty) { - self.cx.emit_span_lint( - USES_POWER_ALIGNMENT, - field_def.span, - UsesPowerAlignment, - ); - } - } - } + self.visit_type(state, ty) } } @@ -680,11 +767,10 @@ impl<'tcx> ImproperCTypesLint { fn check_type_for_external_abi_fnptr( &mut self, cx: &LateContext<'tcx>, + state: VisitorState, hir_ty: &hir::Ty<'tcx>, ty: Ty<'tcx>, fn_mode: CItemKind, - is_static: bool, - is_return: bool, ) { struct FnPtrFinder<'tcx> { spans: Vec, @@ -724,9 +810,9 @@ impl<'tcx> ImproperCTypesLint { let all_types = iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)); for (fn_ptr_ty, span) in all_types { - let mut visitor = ImproperCTypesVisitor { cx, mode: fn_mode }; - // TODO: make a check_for_fnptr - let ffi_res = visitor.check_type(fn_ptr_ty, is_static, is_return); + let mut visitor = ImproperCTypesVisitor::new(cx, fn_ptr_ty, fn_mode); + // FIXME(ctypes): make a check_for_fnptr + let ffi_res = visitor.check_type(state, fn_ptr_ty); self.process_ffi_result(cx, span, ffi_res, fn_mode); } @@ -745,11 +831,13 @@ impl<'tcx> ImproperCTypesLint { let sig = cx.tcx.instantiate_bound_regions_with_erased(sig); for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { - self.check_type_for_external_abi_fnptr(cx, input_hir, *input_ty, fn_mode, false, false); + let state = VisitorState::argument_from_fnmode(fn_mode); + self.check_type_for_external_abi_fnptr(cx, state, input_hir, *input_ty, fn_mode); } if let hir::FnRetTy::Return(ret_hir) = decl.output { - self.check_type_for_external_abi_fnptr(cx, ret_hir, sig.output(), fn_mode, false, true); + let state = VisitorState::return_from_fnmode(fn_mode); + self.check_type_for_external_abi_fnptr(cx, state, ret_hir, sig.output(), fn_mode); } } @@ -764,18 +852,16 @@ impl<'tcx> ImproperCTypesLint { adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none() ); - let mut visitor = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration }; - // FIXME(ctypes): this following call is awkward. // is there a way to perform its logic in MIR space rather than HIR space? // (so that its logic can be absorbed into visitor.visit_struct_or_union) - visitor.check_struct_for_power_alignment(item); + check_struct_for_power_alignment(cx, item, adt_def); } 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 { cx, mode: CItemKind::Declaration }; - let ffi_res = visitor.check_type(ty, true, false); + let mut visitor = ImproperCTypesVisitor::new(cx, ty, CItemKind::Declaration); + let ffi_res = visitor.check_type(VisitorState::STATIC_TY, ty); self.process_ffi_result(cx, span, ffi_res, CItemKind::Declaration); } @@ -791,14 +877,16 @@ impl<'tcx> ImproperCTypesLint { let sig = cx.tcx.instantiate_bound_regions_with_erased(sig); for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { - let mut visitor = ImproperCTypesVisitor { cx, mode: fn_mode }; - let ffi_res = visitor.check_type(*input_ty, false, false); + let state = VisitorState::argument_from_fnmode(fn_mode); + let mut visitor = ImproperCTypesVisitor::new(cx, *input_ty, fn_mode); + 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 visitor = ImproperCTypesVisitor { cx, mode: fn_mode }; - let ffi_res = visitor.check_type(sig.output(), false, true); + let state = VisitorState::return_from_fnmode(fn_mode); + let mut visitor = ImproperCTypesVisitor::new(cx, sig.output(), fn_mode); + let ffi_res = visitor.check_type(state, sig.output()); self.process_ffi_result(cx, ret_hir.span, ffi_res, fn_mode); } } @@ -897,11 +985,10 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint { | hir::ItemKind::TyAlias(_, _, ty) => { self.check_type_for_external_abi_fnptr( cx, + VisitorState::STATIC_TY, ty, cx.tcx.type_of(item.owner_id).instantiate_identity(), CItemKind::Definition, - true, - false, ); } // See `check_fn` for declarations, `check_foreign_items` for definitions in extern blocks @@ -932,11 +1019,10 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint { fn check_field_def(&mut self, cx: &LateContext<'tcx>, field: &'tcx hir::FieldDef<'tcx>) { self.check_type_for_external_abi_fnptr( cx, + VisitorState::STATIC_TY, field.ty, cx.tcx.type_of(field.def_id).instantiate_identity(), CItemKind::Definition, - true, - false, ); }