From 59c22c3a03fe9b7700839b0b4e4815e793aecf55 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 29 Jul 2024 15:19:00 +0300 Subject: [PATCH] Lint against &T to &mut T and &T to &UnsafeCell transmutes This adds the lint against `&T`->`&UnsafeCell` transmutes, and also check in struct fields, and reference casts (`&*(&a as *const u8 as *const UnsafeCell)`). The code is quite complex; I've tried my best to simplify and comment it. This is missing one parts: array transmutes. When transmuting an array, this only consider the first element. The reason for that is that the code is already quite complex, and I didn't want to complicate it more. This catches the most common pattern of transmuting an array into an array of the same length with type of the same size; more complex cases are likely not properly handled. We could take a bigger sample, for example the first and last elements to increase the chance that the lint will catch mistakes, but then the runtime complexity becomes exponential with the nesting of the arrays (`[[[[[T; 2]; 2]; 2]; 2]; 2]` has complexity of O(2**5), for instance). --- compiler/rustc_lint/messages.ftl | 10 + compiler/rustc_lint/src/builtin.rs | 78 +- compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 27 +- compiler/rustc_lint/src/mutable_transmutes.rs | 715 ++++++++++++++++++ .../concurrency/read_only_atomic_cmpxchg.rs | 1 + .../read_only_atomic_load_acquire.rs | 1 + .../read_only_atomic_load_large.rs | 1 + .../miri/tests/pass/atomic-readonly-load.rs | 1 + .../pass/tree_borrows/transmute-unsafecell.rs | 1 + .../allowed-cli-deny-by-default-lint.stderr | 1 + .../allowed-deny-by-default-lint.stderr | 1 + .../force-warn/deny-by-default-lint.stderr | 1 + .../mutable_transmutes/allow_mut_to_mut.rs | 7 + .../allow_mut_to_unsafecell.rs | 9 + .../allow_unsafecell_to_unsafecell.rs | 25 + tests/ui/lint/mutable_transmutes/array.rs | 6 + tests/ui/lint/mutable_transmutes/array.stderr | 11 + .../diag_includes_field_names.rs | 45 ++ .../diag_includes_field_names.stderr | 27 + .../mutable_transmutes/imm_to_mut.rs} | 0 .../mutable_transmutes/imm_to_mut.stderr} | 3 +- .../mutable_transmutes/imm_to_unsafecelll.rs | 7 + .../imm_to_unsafecelll.stderr | 11 + .../lint/mutable_transmutes/nested_field.rs | 22 + .../mutable_transmutes/nested_field.stderr | 20 + .../lint/mutable_transmutes/non_c_layout.rs | 23 + .../partially_overlapping.rs | 18 + .../partially_overlapping.stderr | 20 + .../report_only_first_error.rs | 12 + .../report_only_first_error.stderr | 11 + .../unsafe_cell_reference_casting.rs | 21 + .../unsafe_cell_reference_casting.stderr | 11 + tests/ui/lint/mutable_transmutes/unsized.rs | 19 + .../ui/lint/mutable_transmutes/unsized.stderr | 11 + 35 files changed, 1105 insertions(+), 75 deletions(-) create mode 100644 compiler/rustc_lint/src/mutable_transmutes.rs create mode 100644 tests/ui/lint/mutable_transmutes/allow_mut_to_mut.rs create mode 100644 tests/ui/lint/mutable_transmutes/allow_mut_to_unsafecell.rs create mode 100644 tests/ui/lint/mutable_transmutes/allow_unsafecell_to_unsafecell.rs create mode 100644 tests/ui/lint/mutable_transmutes/array.rs create mode 100644 tests/ui/lint/mutable_transmutes/array.stderr create mode 100644 tests/ui/lint/mutable_transmutes/diag_includes_field_names.rs create mode 100644 tests/ui/lint/mutable_transmutes/diag_includes_field_names.stderr rename tests/ui/{transmute/transmute-imut-to-mut.rs => lint/mutable_transmutes/imm_to_mut.rs} (100%) rename tests/ui/{transmute/transmute-imut-to-mut.stderr => lint/mutable_transmutes/imm_to_mut.stderr} (81%) create mode 100644 tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.rs create mode 100644 tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.stderr create mode 100644 tests/ui/lint/mutable_transmutes/nested_field.rs create mode 100644 tests/ui/lint/mutable_transmutes/nested_field.stderr create mode 100644 tests/ui/lint/mutable_transmutes/non_c_layout.rs create mode 100644 tests/ui/lint/mutable_transmutes/partially_overlapping.rs create mode 100644 tests/ui/lint/mutable_transmutes/partially_overlapping.stderr create mode 100644 tests/ui/lint/mutable_transmutes/report_only_first_error.rs create mode 100644 tests/ui/lint/mutable_transmutes/report_only_first_error.stderr create mode 100644 tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.rs create mode 100644 tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.stderr create mode 100644 tests/ui/lint/mutable_transmutes/unsized.rs create mode 100644 tests/ui/lint/mutable_transmutes/unsized.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 987dbf6db630a..328491450fcbb 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -119,6 +119,7 @@ lint_builtin_missing_doc = missing documentation for {$article} {$desc} lint_builtin_mutable_transmutes = transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell + .note = transmute from `{$from}` to `{$to}` lint_builtin_no_mangle_fn = declaration of a `no_mangle` function lint_builtin_no_mangle_generic = functions generic over types or consts must be mangled @@ -169,6 +170,10 @@ lint_builtin_unreachable_pub = unreachable `pub` {$what} lint_builtin_unsafe_block = usage of an `unsafe` block +lint_builtin_unsafe_cell_transmutes = + transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + .note = transmute from `{$from}` to `{$to}` + lint_builtin_unsafe_extern_block = usage of an `unsafe extern` block lint_builtin_unsafe_impl = implementation of an `unsafe` trait @@ -862,6 +867,11 @@ lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe .label = usage of unsafe attribute lint_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)` +lint_unsafe_cell_reference_casting = + casting `&T` to `&UnsafeCell` is undefined behavior, even if the reference is unused, consider using an `UnsafeCell` on the original data + .label = casting happend here + .note = cast from `{$from}` to `{$to}` + lint_unsupported_group = `{$lint_group}` lint group is not supported with ´--force-warn´ lint_untranslatable_diag = diagnostics should be created using translatable messages diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index d8674817cb5eb..c5d6bee854091 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -61,12 +61,11 @@ use crate::lints::{ BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents, - BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinMutablesTransmutes, - BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed, - BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller, - BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, - BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, - BuiltinWhileTrue, InvalidAsmLabel, + BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinNoMangleGeneric, + BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, + BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, + BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, + BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel, }; use crate::nonstandard_style::{method_context, MethodLateContext}; use crate::{ @@ -1100,72 +1099,6 @@ impl<'tcx> LateLintPass<'tcx> for InvalidNoMangleItems { } } -declare_lint! { - /// The `mutable_transmutes` lint catches transmuting from `&T` to `&mut - /// T` because it is [undefined behavior]. - /// - /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html - /// - /// ### Example - /// - /// ```rust,compile_fail - /// unsafe { - /// let y = std::mem::transmute::<&i32, &mut i32>(&5); - /// } - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// Certain assumptions are made about aliasing of data, and this transmute - /// violates those assumptions. Consider using [`UnsafeCell`] instead. - /// - /// [`UnsafeCell`]: https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html - MUTABLE_TRANSMUTES, - Deny, - "transmuting &T to &mut T is undefined behavior, even if the reference is unused" -} - -declare_lint_pass!(MutableTransmutes => [MUTABLE_TRANSMUTES]); - -impl<'tcx> LateLintPass<'tcx> for MutableTransmutes { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) { - if let Some((&ty::Ref(_, _, from_mutbl), &ty::Ref(_, _, to_mutbl))) = - get_transmute_from_to(cx, expr).map(|(ty1, ty2)| (ty1.kind(), ty2.kind())) - { - if from_mutbl < to_mutbl { - cx.emit_span_lint(MUTABLE_TRANSMUTES, expr.span, BuiltinMutablesTransmutes); - } - } - - fn get_transmute_from_to<'tcx>( - cx: &LateContext<'tcx>, - expr: &hir::Expr<'_>, - ) -> Option<(Ty<'tcx>, Ty<'tcx>)> { - let def = if let hir::ExprKind::Path(ref qpath) = expr.kind { - cx.qpath_res(qpath, expr.hir_id) - } else { - return None; - }; - if let Res::Def(DefKind::Fn, did) = def { - if !def_id_is_transmute(cx, did) { - return None; - } - let sig = cx.typeck_results().node_type(expr.hir_id).fn_sig(cx.tcx); - let from = sig.inputs().skip_binder()[0]; - let to = sig.output().skip_binder(); - return Some((from, to)); - } - None - } - - fn def_id_is_transmute(cx: &LateContext<'_>, def_id: DefId) -> bool { - cx.tcx.is_intrinsic(def_id, sym::transmute) - } - } -} - declare_lint! { /// The `unstable_features` lint detects uses of `#![feature]`. /// @@ -1612,7 +1545,6 @@ declare_lint_pass!( UNUSED_DOC_COMMENTS, NO_MANGLE_CONST_ITEMS, NO_MANGLE_GENERIC_ITEMS, - MUTABLE_TRANSMUTES, UNSTABLE_FEATURES, UNREACHABLE_PUB, TYPE_ALIAS_BOUNDS, diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 196b8fd52d53f..255428f318031 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -65,6 +65,7 @@ mod macro_expr_fragment_specifier_2024_migration; mod map_unit_fn; mod methods; mod multiple_supertrait_upcastable; +mod mutable_transmutes; mod non_ascii_idents; mod non_fmt_panic; mod non_local_def; @@ -99,6 +100,7 @@ use macro_expr_fragment_specifier_2024_migration::*; use map_unit_fn::*; use methods::*; use multiple_supertrait_upcastable::*; +use mutable_transmutes::*; use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; use non_local_def::*; @@ -209,6 +211,7 @@ late_lint_methods!( // Depends on referenced function signatures in expressions PtrNullChecks: PtrNullChecks, MutableTransmutes: MutableTransmutes, + UnsafeCellReferenceCasting: UnsafeCellReferenceCasting, TypeAliasBounds: TypeAliasBounds, TrivialConstraints: TrivialConstraints, TypeLimits: TypeLimits::new(), diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 46e7655a656b3..9c1530777aca7 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -224,9 +224,34 @@ pub struct BuiltinConstNoMangle { pub suggestion: Span, } +// mutable_transmutes.rs #[derive(LintDiagnostic)] #[diag(lint_builtin_mutable_transmutes)] -pub struct BuiltinMutablesTransmutes; +#[note] +pub struct BuiltinMutablesTransmutes { + pub from: String, + pub to: String, +} + +// mutable_transmutes.rs +#[derive(LintDiagnostic)] +#[diag(lint_builtin_unsafe_cell_transmutes)] +#[note] +pub struct BuiltinUnsafeCellTransmutes { + pub from: String, + pub to: String, +} + +// mutable_transmutes.rs +#[derive(LintDiagnostic)] +#[diag(lint_unsafe_cell_reference_casting)] +#[note] +pub struct UnsafeCellReferenceCastingDiag { + #[label] + pub orig_cast: Option, + pub from: String, + pub to: String, +} #[derive(LintDiagnostic)] #[diag(lint_builtin_unstable_features)] diff --git a/compiler/rustc_lint/src/mutable_transmutes.rs b/compiler/rustc_lint/src/mutable_transmutes.rs new file mode 100644 index 0000000000000..a53cc60c7bf07 --- /dev/null +++ b/compiler/rustc_lint/src/mutable_transmutes.rs @@ -0,0 +1,715 @@ +//! This module check that we are not transmuting `&T` to `&mut T` or `&UnsafeCell`. +//! +//! The complexity comes from the fact that we want to lint against this cast even when the `&T`, the `&mut T` +//! or the `&UnsafeCell` (either the `&` or the `UnsafeCell` part of it) are hidden in fields. +//! +//! The general idea is to first isolate potential candidates, then check if they are really problematic. +//! +//! That is, we first collect all instances of `&mut` references in the target type, then we check if any +//! of them overlap with a `&` reference in the source type. +//! +//! We do a similar thing to `UnsafeCell`. Here it is a little more complicated, since we operate two layers deep: +//! first, we collect all `&` references in the target type. Then, we check if any of them overlaps with a `&` reference +//! in the source type (not a `&mut` reference, since it is perfectly fine to transmute `&mut T` to `&UnsafeCell`). +//! If we found such overlap, we collect all instances of `UnsafeCell` under the reference in the target type. +//! If we found any, we try to find if it overlaps with things that are not `UnsafeCell` under the reference +//! in the source type. + +use std::fmt::Write; +use std::ops::ControlFlow; + +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; +use rustc_hir::{BorrowKind, Expr, ExprKind, UnOp}; +use rustc_index::IndexSlice; +use rustc_middle::bug; +use rustc_middle::ty::layout::LayoutOf; +use rustc_middle::ty::{self, AdtDef, GenericArgsRef, List, Mutability, Ty}; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::symbol::sym; +use rustc_target::abi::{FieldIdx, FieldsShape, HasDataLayout, Layout, Size}; + +use crate::lints::{ + BuiltinMutablesTransmutes, BuiltinUnsafeCellTransmutes, UnsafeCellReferenceCastingDiag, +}; +use crate::{LateContext, LateLintPass, LintContext}; + +declare_lint! { + /// The `mutable_transmutes` lint catches transmuting from `&T` to `&mut + /// T` because it is [undefined behavior]. + /// + /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html + /// + /// ### Example + /// + /// ```rust,compile_fail + /// unsafe { + /// let y = std::mem::transmute::<&i32, &mut i32>(&5); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Certain assumptions are made about aliasing of data, and this transmute + /// violates those assumptions. Consider using [`UnsafeCell`] instead. + /// + /// [`UnsafeCell`]: https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html + pub MUTABLE_TRANSMUTES, + Deny, + "transmuting &T to &mut T is undefined behavior, even if the reference is unused" +} + +declare_lint! { + /// The `unsafe_cell_transmutes` lint catches transmuting from `&T` to [`&UnsafeCell`] + /// because it is [undefined behavior]. + /// + /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html + /// + /// ### Example + /// + /// ```rust,compile_fail + /// use std::cell::UnsafeCell; + /// + /// unsafe { + /// let y = std::mem::transmute::<&i32, &UnsafeCell>(&5); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Certain assumptions are made about aliasing of data, and this transmute + /// violates those assumptions. Consider using `UnsafeCell` for the original data. + /// + /// [`&UnsafeCell`]: https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html + pub UNSAFE_CELL_TRANSMUTES, + Deny, + "transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused" +} + +declare_lint_pass!(MutableTransmutes => [MUTABLE_TRANSMUTES, UNSAFE_CELL_TRANSMUTES]); + +fn struct_fields<'a, 'tcx>( + cx: &'a LateContext<'tcx>, + layout: Layout<'tcx>, + adt_def: AdtDef<'tcx>, + substs: GenericArgsRef<'tcx>, +) -> impl Iterator)> + 'a { + let field_tys = adt_def + .non_enum_variant() + .fields + .iter() + .map(|field| cx.tcx.normalize_erasing_regions(cx.param_env, field.ty(cx.tcx, substs))); + // I thought this could panic, but apparently SIMD vectors are structs but also have primitive layout. So... + let field_offsets = match &layout.fields { + FieldsShape::Arbitrary { offsets, .. } => offsets.as_slice(), + _ => IndexSlice::empty(), + }; + std::iter::zip(field_offsets.iter_enumerated(), field_tys) + .map(|((idx, &offset), ty)| (idx, offset, ty)) +} + +fn tuple_fields<'tcx>( + layout: Layout<'tcx>, + field_tys: &'tcx List>, +) -> impl Iterator)> + 'tcx { + let field_offsets = match &layout.fields { + FieldsShape::Arbitrary { offsets, .. } => offsets, + _ => bug!("expected FieldsShape::Arbitrary for tuples"), + }; + std::iter::zip(field_offsets.iter_enumerated(), field_tys) + .map(|((idx, &offset), ty)| (idx, offset, ty)) +} + +#[derive(Debug, Default, Clone)] +struct FieldsBreadcrumbs(Vec); + +impl FieldsBreadcrumbs { + fn push_with(&mut self, field: FieldIdx, f: impl FnOnce(&mut FieldsBreadcrumbs) -> R) -> R { + self.0.push(field); + let result = f(self); + self.0.pop(); + result + } + + fn translate_for_diagnostics<'tcx>( + &self, + cx: &LateContext<'tcx>, + output: &mut String, + mut ty: Ty<'tcx>, + ) { + for ¤t_breadcrumb in &self.0 { + ty = match ty.kind() { + &ty::Adt(adt_def, substs) if adt_def.is_struct() => { + let field = &adt_def.non_enum_variant().fields[current_breadcrumb]; + let field_ty = + cx.tcx.normalize_erasing_regions(cx.param_env, field.ty(cx.tcx, substs)); + output.push_str("."); + output.push_str(field.name.as_str()); + + field_ty + } + + &ty::Tuple(tys) => { + let field_ty = tys[current_breadcrumb.as_usize()]; + write!(output, ".{}", current_breadcrumb.as_u32()).unwrap(); + + field_ty + } + + &ty::Array(element_ty, _) => { + output.push_str("[0]"); + + element_ty + } + + _ => bug!("field breadcrumb on an unknown type"), + } + } + } + + // Formats the breadcrumbs as `(*path.to.reference).path.to.unsafecell`; + fn unsafe_cell_breadcrumbs<'tcx>( + cx: &LateContext<'tcx>, + reference: &FieldsBreadcrumbs, + top_ty: Ty<'tcx>, + referent: &FieldsBreadcrumbs, + reference_ty: Ty<'tcx>, + ) -> String { + let mut result; + if !referent.0.is_empty() { + result = format!("(*{top_ty}"); + reference.translate_for_diagnostics(cx, &mut result, top_ty); + result.push_str(")"); + referent.translate_for_diagnostics(cx, &mut result, reference_ty); + } else { + result = format!("*{top_ty}"); + reference.translate_for_diagnostics(cx, &mut result, top_ty); + } + result + } +} + +#[derive(PartialEq)] +enum CollectFields { + Yes, + No, +} + +fn collect_fields<'tcx>( + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + offset: Size, + callback: &mut impl FnMut(Size, Ty<'tcx>, &FieldsBreadcrumbs) -> CollectFields, + fields_breadcrumbs: &mut FieldsBreadcrumbs, +) { + if callback(offset, ty, &fields_breadcrumbs) == CollectFields::No { + return; + } + + match ty.kind() { + &ty::Adt(adt_def, substs) if adt_def.is_struct() => { + let Ok(layout) = cx.layout_of(ty) else { + return; + }; + tracing::debug!(?offset, ?layout, "collect_fields struct"); + for (field_idx, field_offset, field_ty) in + struct_fields(cx, layout.layout, adt_def, substs) + { + fields_breadcrumbs.push_with(field_idx, |fields_breadcrumbs| { + collect_fields( + cx, + field_ty, + offset + field_offset, + callback, + fields_breadcrumbs, + ) + }); + } + } + + &ty::Tuple(tys) => { + let Ok(layout) = cx.layout_of(ty) else { + return; + }; + tracing::debug!(?offset, ?layout, "collect_fields tuple"); + for (field_idx, field_offset, field_ty) in tuple_fields(layout.layout, tys) { + fields_breadcrumbs.push_with(field_idx, |fields_breadcrumbs| { + collect_fields( + cx, + field_ty, + offset + field_offset, + callback, + fields_breadcrumbs, + ) + }); + } + } + + &ty::Array(ty, len) + if let Some(len) = len.try_eval_target_usize(cx.tcx, cx.param_env) + && len != 0 => + { + tracing::debug!(?offset, "collect_fields array"); + fields_breadcrumbs.push_with(FieldIdx::from_u32(0), |fields_breadcrumbs| { + collect_fields(cx, ty, offset, callback, fields_breadcrumbs) + }); + } + + _ => {} + } +} + +trait Field { + fn start_offset(&self) -> Size; + fn end_offset(&self) -> Size; +} + +#[derive(Debug)] +enum CheckOverlapping { + Yes, + No, + /// Reported an error - stop the search completely (in order to not report more than one error). + ReportedError, +} + +/// `check_overlap_with` must be sorted by starting offset. +fn check_overlapping<'tcx>( + cx: &LateContext<'tcx>, + check_overlap_with: &[impl Field], + ty: Ty<'tcx>, + offset: Size, + on_overlapping: &mut impl FnMut(Ty<'tcx>, usize, &FieldsBreadcrumbs) -> CheckOverlapping, + fields_breadcrumbs: &mut FieldsBreadcrumbs, +) -> ControlFlow<()> { + let Ok(layout) = cx.layout_of(ty) else { + return ControlFlow::Continue(()); + }; + tracing::debug!(?offset, ?layout, "check_overlapping"); + + // Then, for any entry that we overlap with (there can be many as our size can be bigger than one, + // or because both are only partially overlapping) call the closure. + let mut idx = 0; + while idx < check_overlap_with.len() + && check_overlap_with[idx].start_offset() < offset + layout.size + { + if check_overlap_with[idx].end_offset() < offset + layout.size { + idx += 1; + continue; + } + + let result = on_overlapping(ty, idx, &fields_breadcrumbs); + tracing::debug!(?ty, ?idx, ?fields_breadcrumbs, ?result, "on_overlapping"); + match result { + CheckOverlapping::Yes => {} + CheckOverlapping::No => return ControlFlow::Continue(()), + CheckOverlapping::ReportedError => return ControlFlow::Break(()), + }; + idx += 1; + } + + // Finally, descend to every field and check there too. + match ty.kind() { + &ty::Adt(adt_def, substs) if adt_def.is_struct() => { + for (field_idx, field_offset, field_ty) in + struct_fields(cx, layout.layout, adt_def, substs) + { + fields_breadcrumbs.push_with(field_idx, |fields_breadcrumbs| { + check_overlapping( + cx, + check_overlap_with, + field_ty, + offset + field_offset, + on_overlapping, + fields_breadcrumbs, + ) + })?; + } + } + + &ty::Tuple(tys) => { + for (field_idx, field_offset, field_ty) in tuple_fields(layout.layout, tys) { + fields_breadcrumbs.push_with(field_idx, |fields_breadcrumbs| { + check_overlapping( + cx, + check_overlap_with, + field_ty, + offset + field_offset, + on_overlapping, + fields_breadcrumbs, + ) + })?; + } + } + + &ty::Array(ty, _) if layout.size != Size::ZERO => { + fields_breadcrumbs.push_with(FieldIdx::from_u32(0), |fields_breadcrumbs| { + check_overlapping( + cx, + check_overlap_with, + ty, + offset, + on_overlapping, + fields_breadcrumbs, + ) + })?; + } + + _ => {} + } + + ControlFlow::Continue(()) +} + +#[derive(Debug)] +struct Ref<'tcx> { + start_offset: Size, + end_offset: Size, + ty: Ty<'tcx>, + mutability: Mutability, + fields_breadcrumbs: FieldsBreadcrumbs, +} + +impl Field for Ref<'_> { + fn start_offset(&self) -> Size { + self.start_offset + } + + fn end_offset(&self) -> Size { + self.end_offset + } +} + +#[derive(Debug)] +struct UnsafeCellField { + start_offset: Size, + end_offset: Size, + fields_breadcrumbs: FieldsBreadcrumbs, +} + +impl Field for UnsafeCellField { + fn start_offset(&self) -> Size { + self.start_offset + } + + fn end_offset(&self) -> Size { + self.end_offset + } +} + +/// The parameters to `report_error` are: breadcrumbs to src's UnsafeCell, breadcrumbs to dst's UnsafeCell. +fn check_unsafe_cells<'tcx>( + cx: &LateContext<'tcx>, + dst_ref_ty: Ty<'tcx>, + src_ty: Ty<'tcx>, + mut report_error: impl FnMut(&FieldsBreadcrumbs, &FieldsBreadcrumbs), +) -> ControlFlow<()> { + let mut dst_unsafe_cells = Vec::new(); + collect_fields( + cx, + dst_ref_ty, + Size::ZERO, + &mut |offset, ty, fields_breadcrumbs| match ty.kind() { + &ty::Adt(adt_def, _) if adt_def.is_unsafe_cell() => { + let Ok(layout) = cx.layout_of(ty) else { + return CollectFields::Yes; + }; + if layout.is_zst() { + return CollectFields::No; + } + + let Ok(layout) = cx.layout_of(ty) else { + return CollectFields::Yes; + }; + dst_unsafe_cells.push(UnsafeCellField { + start_offset: offset, + end_offset: offset + layout.size, + fields_breadcrumbs: fields_breadcrumbs.clone(), + }); + CollectFields::No + } + _ => CollectFields::Yes, + }, + &mut FieldsBreadcrumbs::default(), + ); + dst_unsafe_cells.sort_unstable_by_key(|unsafe_cell| unsafe_cell.start_offset); + tracing::debug!(?dst_unsafe_cells, "collected UnsafeCells"); + + check_overlapping( + cx, + &dst_unsafe_cells, + src_ty, + Size::ZERO, + &mut |ty, idx, src_pointee_fields_breadcrumbs| { + // First, check that there are actually some bytes there, not just padding or ZST. + match ty.kind() { + // Transmuting `&UnsafeCell` to `&UnsafeCell` is fine, don't check inside. + ty::Adt(adt_def, _) if adt_def.is_unsafe_cell() => { + return CheckOverlapping::No; + } + + ty::Bool + | ty::Char + | ty::Int(..) + | ty::Uint(..) + | ty::Float(..) + | ty::RawPtr(..) + | ty::Ref(..) + | ty::FnPtr(..) => {} + // Enums have their discriminant. + ty::Adt(adt_def, _) if adt_def.is_enum() => {} + _ => return CheckOverlapping::Yes, + } + + report_error(src_pointee_fields_breadcrumbs, &dst_unsafe_cells[idx].fields_breadcrumbs); + + CheckOverlapping::ReportedError + }, + &mut FieldsBreadcrumbs::default(), + ) +} + +impl<'tcx> LateLintPass<'tcx> for MutableTransmutes { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + let Some((src, dst)) = get_transmute_from_to(cx, expr) else { return }; + let span = tracing::debug_span!("rustc_lint::mutable_transmutes::check_expr"); + let _guard = span.enter(); + tracing::debug!(?src, ?dst); + + let mut dst_refs = Vec::new(); + // First, collect references in the target type, so we can see for mutable references if they are transmuted + // from a shared reference, and for shared references if `UnsafeCell` inside them is transmuted from non-`UnsafeCell`. + // Instead of doing this in two passes, one for shared references (`UnsafeCell`) and another for mutable, + // we do it in one pass and keep track of what kind of reference it is. + collect_fields( + cx, + dst, + Size::ZERO, + &mut |offset, ty, fields_breadcrumbs| match ty.kind() { + &ty::Ref(_, ty, mutability) => { + dst_refs.push(Ref { + start_offset: offset, + end_offset: offset + cx.data_layout().pointer_size, + ty, + mutability, + fields_breadcrumbs: fields_breadcrumbs.clone(), + }); + CollectFields::No + } + _ => CollectFields::Yes, + }, + &mut FieldsBreadcrumbs::default(), + ); + dst_refs.sort_unstable_by_key(|ref_| ref_.start_offset); + tracing::debug!(?dst_refs, "collected refs"); + check_overlapping( + cx, + &dst_refs, + src, + Size::ZERO, + &mut |ty, idx, src_fields_breadcrumbs| { + let dst_ref = &dst_refs[idx]; + match dst_ref.mutability { + // For mutable references in the target type, we need to see if they were converted from shared references. + Mutability::Mut => match ty.kind() { + ty::Ref(_, _, Mutability::Not) => { + let mut from = src.to_string(); + src_fields_breadcrumbs.translate_for_diagnostics(cx, &mut from, src); + let mut to = dst.to_string(); + dst_ref.fields_breadcrumbs.translate_for_diagnostics(cx, &mut to, dst); + cx.emit_span_lint( + MUTABLE_TRANSMUTES, + expr.span, + BuiltinMutablesTransmutes { from, to }, + ); + CheckOverlapping::ReportedError + } + _ => CheckOverlapping::Yes, + }, + // For shared references, we need to see if they were transmuted from shared (not mutable!) references, + // and if there is an incorrectly transmuted `UnsafeCell` inside. + Mutability::Not => { + let &ty::Ref(_, src_ty, Mutability::Not) = ty.kind() else { + return CheckOverlapping::Yes; + }; + tracing::debug!(?src_ty, dst_ty = ?dst_ref.ty, "found shared ref"); + + let found_error = check_unsafe_cells( + cx, + dst_ref.ty, + src_ty, + |src_pointee_fields_breadcrumbs, dst_pointee_fields_breadcrumbs| { + let from = FieldsBreadcrumbs::unsafe_cell_breadcrumbs( + cx, + src_fields_breadcrumbs, + src, + src_pointee_fields_breadcrumbs, + src_ty, + ); + let to = FieldsBreadcrumbs::unsafe_cell_breadcrumbs( + cx, + &dst_ref.fields_breadcrumbs, + dst, + dst_pointee_fields_breadcrumbs, + dst_ref.ty, + ); + + cx.emit_span_lint( + UNSAFE_CELL_TRANSMUTES, + expr.span, + BuiltinUnsafeCellTransmutes { from, to }, + ); + }, + ); + match found_error { + ControlFlow::Continue(()) => CheckOverlapping::Yes, + ControlFlow::Break(()) => CheckOverlapping::ReportedError, + } + } + } + }, + &mut FieldsBreadcrumbs::default(), + ); + } +} + +fn get_transmute_from_to<'tcx>( + cx: &LateContext<'tcx>, + expr: &Expr<'_>, +) -> Option<(Ty<'tcx>, Ty<'tcx>)> { + let def = if let ExprKind::Path(ref qpath) = expr.kind { + cx.qpath_res(qpath, expr.hir_id) + } else { + return None; + }; + if let Res::Def(DefKind::Fn, did) = def { + if !def_id_is_transmute(cx, did) { + return None; + } + let sig = cx.typeck_results().node_type(expr.hir_id).fn_sig(cx.tcx); + let from = sig.inputs().skip_binder()[0]; + let to = sig.output().skip_binder(); + return Some((from, to)); + } + None +} + +fn def_id_is_transmute(cx: &LateContext<'_>, def_id: DefId) -> bool { + cx.tcx.is_intrinsic(def_id, sym::transmute) +} + +declare_lint! { + /// The `unsafe_cell_reference_casting` lint checks for casts of `&T` to `&UnsafeCell` + /// + /// ### Example + /// + /// ```rust,compile_fail + /// use std::cell::Cell; + /// + /// fn x(r: &i32) { + /// unsafe { + /// (*(r as *const i32 as *const Cell)).set(0); + /// } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Casting `&T` to `&UnsafeCell` is undefined behavior, + /// as it's a violation of Rust reference aliasing requirements. + UNSAFE_CELL_REFERENCE_CASTING, + Deny, + "casts of `&T` to `&UnsafeCell`" +} + +declare_lint_pass!(UnsafeCellReferenceCasting => [UNSAFE_CELL_REFERENCE_CASTING]); + +impl<'tcx> LateLintPass<'tcx> for UnsafeCellReferenceCasting { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + let ExprKind::AddrOf( + BorrowKind::Ref, + Mutability::Not, + Expr { kind: ExprKind::Unary(UnOp::Deref, e), .. }, + ) = expr.kind + else { + return; + }; + + let init = cx.expr_or_init(e); + + let Some((dst, src)) = is_type_cast(cx, init) else { + return; + }; + let orig_cast = if init.span != e.span { Some(init.span) } else { None }; + + check_unsafe_cells(cx, dst, src, |src_fields_breadcrumbs, dst_fields_breadcrumbs| { + let mut from = src.to_string(); + if !src_fields_breadcrumbs.0.is_empty() { + src_fields_breadcrumbs.translate_for_diagnostics(cx, &mut from, src); + } + let mut to = dst.to_string(); + if !dst_fields_breadcrumbs.0.is_empty() { + dst_fields_breadcrumbs.translate_for_diagnostics(cx, &mut to, dst); + } + + cx.emit_span_lint( + &UNSAFE_CELL_REFERENCE_CASTING, + expr.span, + UnsafeCellReferenceCastingDiag { from, to, orig_cast }, + ); + }); + } +} + +fn is_type_cast<'tcx>( + cx: &LateContext<'tcx>, + orig_expr: &'tcx Expr<'tcx>, +) -> Option<(Ty<'tcx>, Ty<'tcx>)> { + let mut e = orig_expr; + + let end_ty = cx.typeck_results().node_type(orig_expr.hir_id); + + let &ty::RawPtr(dst, _) = end_ty.kind() else { + return None; + }; + + loop { + e = e.peel_blocks(); + // as ... + e = if let ExprKind::Cast(expr, _) = e.kind { + expr + // .cast() + } else if let ExprKind::MethodCall(_, expr, [], _) = e.kind + && let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) + && matches!( + cx.tcx.get_diagnostic_name(def_id), + Some(sym::ptr_cast | sym::const_ptr_cast), + ) + { + expr + // ptr::from_ref() or mem::transmute<_, _>() + } else if let ExprKind::Call(path, [arg]) = e.kind + && let ExprKind::Path(ref qpath) = path.kind + && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() + && matches!( + cx.tcx.get_diagnostic_name(def_id), + Some(sym::ptr_from_ref | sym::transmute) + ) + { + arg + } else { + break; + }; + } + + let &ty::Ref(_, src, Mutability::Not) = cx.typeck_results().node_type(e.hir_id).kind() else { + return None; + }; + + Some((dst, src)) +} diff --git a/src/tools/miri/tests/fail/concurrency/read_only_atomic_cmpxchg.rs b/src/tools/miri/tests/fail/concurrency/read_only_atomic_cmpxchg.rs index 88c73d14ef72b..b6e4dbd62fb9d 100644 --- a/src/tools/miri/tests/fail/concurrency/read_only_atomic_cmpxchg.rs +++ b/src/tools/miri/tests/fail/concurrency/read_only_atomic_cmpxchg.rs @@ -1,5 +1,6 @@ // Should not rely on the aliasing model for its failure. //@compile-flags: -Zmiri-disable-stacked-borrows +#![allow(unsafe_cell_reference_casting)] use std::sync::atomic::{AtomicI32, Ordering}; diff --git a/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_acquire.rs b/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_acquire.rs index af0dc2d3fd64a..07e977577cb59 100644 --- a/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_acquire.rs +++ b/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_acquire.rs @@ -1,5 +1,6 @@ // Should not rely on the aliasing model for its failure. //@compile-flags: -Zmiri-disable-stacked-borrows +#![allow(unsafe_cell_reference_casting)] use std::sync::atomic::{AtomicI32, Ordering}; diff --git a/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_large.rs b/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_large.rs index 2d1cb04907cb7..d9ce1db942cf7 100644 --- a/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_large.rs +++ b/src/tools/miri/tests/fail/concurrency/read_only_atomic_load_large.rs @@ -2,6 +2,7 @@ //@compile-flags: -Zmiri-disable-stacked-borrows // Needs atomic accesses larger than the pointer size //@ignore-64bit +#![allow(unsafe_cell_reference_casting)] use std::sync::atomic::{AtomicI64, Ordering}; diff --git a/src/tools/miri/tests/pass/atomic-readonly-load.rs b/src/tools/miri/tests/pass/atomic-readonly-load.rs index 8f8086b3538c6..f9d0223c05d0a 100644 --- a/src/tools/miri/tests/pass/atomic-readonly-load.rs +++ b/src/tools/miri/tests/pass/atomic-readonly-load.rs @@ -1,5 +1,6 @@ // Stacked Borrows doesn't like this. //@compile-flags: -Zmiri-tree-borrows +#![allow(unsafe_cell_reference_casting)] use std::sync::atomic::*; diff --git a/src/tools/miri/tests/pass/tree_borrows/transmute-unsafecell.rs b/src/tools/miri/tests/pass/tree_borrows/transmute-unsafecell.rs index 1df0636e1e5d2..dab36024714f9 100644 --- a/src/tools/miri/tests/pass/tree_borrows/transmute-unsafecell.rs +++ b/src/tools/miri/tests/pass/tree_borrows/transmute-unsafecell.rs @@ -1,4 +1,5 @@ //@compile-flags: -Zmiri-tree-borrows +#![allow(unsafe_cell_transmutes)] //! Testing `mem::transmute` between types with and without interior mutability. //! All transmutations should work, as long as we don't do any actual accesses diff --git a/tests/ui/lint/force-warn/allowed-cli-deny-by-default-lint.stderr b/tests/ui/lint/force-warn/allowed-cli-deny-by-default-lint.stderr index 6a1fc76e18a18..768e57a968ee2 100644 --- a/tests/ui/lint/force-warn/allowed-cli-deny-by-default-lint.stderr +++ b/tests/ui/lint/force-warn/allowed-cli-deny-by-default-lint.stderr @@ -4,6 +4,7 @@ warning: transmuting &T to &mut T is undefined behavior, even if the reference i LL | let y = std::mem::transmute::<&i32, &mut i32>(&5); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | + = note: transmute from `&i32` to `&mut i32` = note: requested on the command line with `--force-warn mutable-transmutes` warning: 1 warning emitted diff --git a/tests/ui/lint/force-warn/allowed-deny-by-default-lint.stderr b/tests/ui/lint/force-warn/allowed-deny-by-default-lint.stderr index 9ef53d47eb931..e5b10122218c5 100644 --- a/tests/ui/lint/force-warn/allowed-deny-by-default-lint.stderr +++ b/tests/ui/lint/force-warn/allowed-deny-by-default-lint.stderr @@ -4,6 +4,7 @@ warning: transmuting &T to &mut T is undefined behavior, even if the reference i LL | let y = std::mem::transmute::<&i32, &mut i32>(&5); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | + = note: transmute from `&i32` to `&mut i32` = note: requested on the command line with `--force-warn mutable-transmutes` warning: 1 warning emitted diff --git a/tests/ui/lint/force-warn/deny-by-default-lint.stderr b/tests/ui/lint/force-warn/deny-by-default-lint.stderr index c644d0fe741ad..01508109939b1 100644 --- a/tests/ui/lint/force-warn/deny-by-default-lint.stderr +++ b/tests/ui/lint/force-warn/deny-by-default-lint.stderr @@ -4,6 +4,7 @@ warning: transmuting &T to &mut T is undefined behavior, even if the reference i LL | let y = std::mem::transmute::<&i32, &mut i32>(&5); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | + = note: transmute from `&i32` to `&mut i32` = note: requested on the command line with `--force-warn mutable-transmutes` warning: 1 warning emitted diff --git a/tests/ui/lint/mutable_transmutes/allow_mut_to_mut.rs b/tests/ui/lint/mutable_transmutes/allow_mut_to_mut.rs new file mode 100644 index 0000000000000..3fc2a942f0536 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/allow_mut_to_mut.rs @@ -0,0 +1,7 @@ +//@ check-pass + +use std::mem::transmute; + +fn main() { + let _a: &mut u8 = unsafe { transmute(&mut 0u8) }; +} diff --git a/tests/ui/lint/mutable_transmutes/allow_mut_to_unsafecell.rs b/tests/ui/lint/mutable_transmutes/allow_mut_to_unsafecell.rs new file mode 100644 index 0000000000000..6f62377aa89cc --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/allow_mut_to_unsafecell.rs @@ -0,0 +1,9 @@ +//@ check-pass + +use std::cell::UnsafeCell; +use std::mem::transmute; + +fn main() { + let _a: &mut UnsafeCell = unsafe { transmute(&mut 0u8) }; + let _a: &UnsafeCell = unsafe { transmute(&mut 0u8) }; +} diff --git a/tests/ui/lint/mutable_transmutes/allow_unsafecell_to_unsafecell.rs b/tests/ui/lint/mutable_transmutes/allow_unsafecell_to_unsafecell.rs new file mode 100644 index 0000000000000..36895d19a75dd --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/allow_unsafecell_to_unsafecell.rs @@ -0,0 +1,25 @@ +//@ check-pass + +use std::cell::UnsafeCell; +use std::mem::transmute; + +#[repr(C)] +struct A { + a: u32, + b: UnsafeCell, +} + +#[repr(C)] +struct B { + a: u32, + b: UnsafeCell, +} + +#[repr(transparent)] +struct AWrapper(A); + +fn main() { + let _a: &UnsafeCell = unsafe { transmute(&UnsafeCell::new(0u8)) }; + let _a: &B = unsafe { transmute(&A { a: 0, b: UnsafeCell::new(0) }) }; + let _a: &AWrapper = unsafe { transmute(&A { a: 0, b: UnsafeCell::new(0) }) }; +} diff --git a/tests/ui/lint/mutable_transmutes/array.rs b/tests/ui/lint/mutable_transmutes/array.rs new file mode 100644 index 0000000000000..5ae129e9c607a --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/array.rs @@ -0,0 +1,6 @@ +use std::mem::transmute; + +fn main() { + let _a: [&mut u8; 2] = unsafe { transmute([&1u8; 2]) }; + //~^ ERROR transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell +} diff --git a/tests/ui/lint/mutable_transmutes/array.stderr b/tests/ui/lint/mutable_transmutes/array.stderr new file mode 100644 index 0000000000000..2cdeb24fb70f7 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/array.stderr @@ -0,0 +1,11 @@ +error: transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell + --> $DIR/array.rs:4:37 + | +LL | let _a: [&mut u8; 2] = unsafe { transmute([&1u8; 2]) }; + | ^^^^^^^^^ + | + = note: transmute from `[&u8; 2][0]` to `[&mut u8; 2][0]` + = note: `#[deny(mutable_transmutes)]` on by default + +error: aborting due to 1 previous error + diff --git a/tests/ui/lint/mutable_transmutes/diag_includes_field_names.rs b/tests/ui/lint/mutable_transmutes/diag_includes_field_names.rs new file mode 100644 index 0000000000000..2ed4b198f9539 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/diag_includes_field_names.rs @@ -0,0 +1,45 @@ +use std::cell::UnsafeCell; +use std::mem::transmute; + +#[repr(transparent)] +struct A { + a: B, +} +#[repr(transparent)] +struct B { + b: C, +} +#[repr(transparent)] +struct C { + c: &'static D, +} +#[repr(transparent)] +struct D { + d: UnsafeCell, +} + +#[repr(transparent)] +struct E { + e: F, +} +#[repr(transparent)] +struct F { + f: &'static G, +} +#[repr(transparent)] +struct G { + g: H, +} +#[repr(transparent)] +struct H { + h: u8, +} + +fn main() { + let _: A = unsafe { transmute(&1u8) }; + //~^ ERROR transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + let _: A = unsafe { transmute(E { e: F { f: &G { g: H { h: 0 } } } }) }; + //~^ ERROR transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + let _: &'static UnsafeCell = unsafe { transmute(E { e: F { f: &G { g: H { h: 0 } } } }) }; + //~^ ERROR transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data +} diff --git a/tests/ui/lint/mutable_transmutes/diag_includes_field_names.stderr b/tests/ui/lint/mutable_transmutes/diag_includes_field_names.stderr new file mode 100644 index 0000000000000..6668f927f12fc --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/diag_includes_field_names.stderr @@ -0,0 +1,27 @@ +error: transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + --> $DIR/diag_includes_field_names.rs:39:25 + | +LL | let _: A = unsafe { transmute(&1u8) }; + | ^^^^^^^^^ + | + = note: transmute from `*&u8` to `(*A.a.b.c).d` + = note: `#[deny(unsafe_cell_transmutes)]` on by default + +error: transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + --> $DIR/diag_includes_field_names.rs:41:25 + | +LL | let _: A = unsafe { transmute(E { e: F { f: &G { g: H { h: 0 } } } }) }; + | ^^^^^^^^^ + | + = note: transmute from `(*E.e.f).g.h` to `(*A.a.b.c).d` + +error: transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + --> $DIR/diag_includes_field_names.rs:43:47 + | +LL | let _: &'static UnsafeCell = unsafe { transmute(E { e: F { f: &G { g: H { h: 0 } } } }) }; + | ^^^^^^^^^ + | + = note: transmute from `(*E.e.f).g.h` to `*&UnsafeCell` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/transmute/transmute-imut-to-mut.rs b/tests/ui/lint/mutable_transmutes/imm_to_mut.rs similarity index 100% rename from tests/ui/transmute/transmute-imut-to-mut.rs rename to tests/ui/lint/mutable_transmutes/imm_to_mut.rs diff --git a/tests/ui/transmute/transmute-imut-to-mut.stderr b/tests/ui/lint/mutable_transmutes/imm_to_mut.stderr similarity index 81% rename from tests/ui/transmute/transmute-imut-to-mut.stderr rename to tests/ui/lint/mutable_transmutes/imm_to_mut.stderr index d37050fa58af6..70af378dced2d 100644 --- a/tests/ui/transmute/transmute-imut-to-mut.stderr +++ b/tests/ui/lint/mutable_transmutes/imm_to_mut.stderr @@ -1,9 +1,10 @@ error: transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell - --> $DIR/transmute-imut-to-mut.rs:6:32 + --> $DIR/imm_to_mut.rs:6:32 | LL | let _a: &mut u8 = unsafe { transmute(&1u8) }; | ^^^^^^^^^ | + = note: transmute from `&u8` to `&mut u8` = note: `#[deny(mutable_transmutes)]` on by default error: aborting due to 1 previous error diff --git a/tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.rs b/tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.rs new file mode 100644 index 0000000000000..469d52d1dcd1d --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.rs @@ -0,0 +1,7 @@ +use std::cell::UnsafeCell; +use std::mem::transmute; + +fn main() { + let _a: &UnsafeCell = unsafe { transmute(&1u8) }; + //~^ ERROR transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data +} diff --git a/tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.stderr b/tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.stderr new file mode 100644 index 0000000000000..03026e7f6819c --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/imm_to_unsafecelll.stderr @@ -0,0 +1,11 @@ +error: transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + --> $DIR/imm_to_unsafecelll.rs:5:40 + | +LL | let _a: &UnsafeCell = unsafe { transmute(&1u8) }; + | ^^^^^^^^^ + | + = note: transmute from `*&u8` to `*&UnsafeCell` + = note: `#[deny(unsafe_cell_transmutes)]` on by default + +error: aborting due to 1 previous error + diff --git a/tests/ui/lint/mutable_transmutes/nested_field.rs b/tests/ui/lint/mutable_transmutes/nested_field.rs new file mode 100644 index 0000000000000..8bea859e5c2eb --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/nested_field.rs @@ -0,0 +1,22 @@ +use std::cell::UnsafeCell; +use std::mem::transmute; + +#[repr(C)] +struct Foo { + a: u32, + b: Bar, +} +#[repr(C)] +struct Bar(Baz); +#[repr(C)] +struct Baz(T); + +#[repr(C)] +struct Other(&'static u8, &'static u8); + +fn main() { + let _: Foo<&'static mut u8> = unsafe { transmute(Other(&1u8, &1u8)) }; + //~^ ERROR transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell + let _: Foo<&'static UnsafeCell> = unsafe { transmute(Other(&1u8, &1u8)) }; + //~^ ERROR transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data +} diff --git a/tests/ui/lint/mutable_transmutes/nested_field.stderr b/tests/ui/lint/mutable_transmutes/nested_field.stderr new file mode 100644 index 0000000000000..058d55a1bdc15 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/nested_field.stderr @@ -0,0 +1,20 @@ +error: transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell + --> $DIR/nested_field.rs:18:44 + | +LL | let _: Foo<&'static mut u8> = unsafe { transmute(Other(&1u8, &1u8)) }; + | ^^^^^^^^^ + | + = note: transmute from `Other.1` to `Foo<&mut u8>.b.0.0` + = note: `#[deny(mutable_transmutes)]` on by default + +error: transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + --> $DIR/nested_field.rs:20:52 + | +LL | let _: Foo<&'static UnsafeCell> = unsafe { transmute(Other(&1u8, &1u8)) }; + | ^^^^^^^^^ + | + = note: transmute from `*Other.1` to `*Foo<&UnsafeCell>.b.0.0` + = note: `#[deny(unsafe_cell_transmutes)]` on by default + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lint/mutable_transmutes/non_c_layout.rs b/tests/ui/lint/mutable_transmutes/non_c_layout.rs new file mode 100644 index 0000000000000..8f83e010b9246 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/non_c_layout.rs @@ -0,0 +1,23 @@ +//@ check-pass +//@ compile-flags: -Zrandomize-layout -Zlayout-seed=2464363 + +use std::cell::UnsafeCell; + +#[derive(Default)] +struct A { + a: u32, + b: u32, + c: u32, + d: u32, + e: UnsafeCell, + f: UnsafeCell, + g: UnsafeCell, + h: UnsafeCell, +} + +#[repr(transparent)] +struct B(A); + +fn main() { + let _b: &B = unsafe { std::mem::transmute(&A::default()) }; +} diff --git a/tests/ui/lint/mutable_transmutes/partially_overlapping.rs b/tests/ui/lint/mutable_transmutes/partially_overlapping.rs new file mode 100644 index 0000000000000..6dac827272b82 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/partially_overlapping.rs @@ -0,0 +1,18 @@ +// This test checks that transmuting part of `&T` to part of `&mut T` errors. +// I don't think this is necessary or common, but this is what the current code does. + +use std::cell::UnsafeCell; +use std::mem::transmute; + +#[repr(C, packed)] +struct Foo(u8, T); + +#[repr(C, packed)] +struct Bar(&'static u8, u8); + +fn main() { + let _: Foo<&'static mut u8> = unsafe { transmute(Bar(&1u8, 0)) }; + //~^ ERROR transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell + let _: Foo<&'static UnsafeCell> = unsafe { transmute(Bar(&1u8, 0)) }; + //~^ ERROR transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data +} diff --git a/tests/ui/lint/mutable_transmutes/partially_overlapping.stderr b/tests/ui/lint/mutable_transmutes/partially_overlapping.stderr new file mode 100644 index 0000000000000..c851fba472275 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/partially_overlapping.stderr @@ -0,0 +1,20 @@ +error: transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell + --> $DIR/partially_overlapping.rs:14:44 + | +LL | let _: Foo<&'static mut u8> = unsafe { transmute(Bar(&1u8, 0)) }; + | ^^^^^^^^^ + | + = note: transmute from `Bar.0` to `Foo<&mut u8>.1` + = note: `#[deny(mutable_transmutes)]` on by default + +error: transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + --> $DIR/partially_overlapping.rs:16:52 + | +LL | let _: Foo<&'static UnsafeCell> = unsafe { transmute(Bar(&1u8, 0)) }; + | ^^^^^^^^^ + | + = note: transmute from `*Bar.0` to `*Foo<&UnsafeCell>.1` + = note: `#[deny(unsafe_cell_transmutes)]` on by default + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lint/mutable_transmutes/report_only_first_error.rs b/tests/ui/lint/mutable_transmutes/report_only_first_error.rs new file mode 100644 index 0000000000000..4c70f3d406c36 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/report_only_first_error.rs @@ -0,0 +1,12 @@ +use std::cell::UnsafeCell; +use std::mem::transmute; + +#[repr(C)] +struct Foo(&'static u8, &'static u8); +#[repr(C)] +struct Bar(&'static UnsafeCell, &'static mut u8); + +fn main() { + let _a: Bar = unsafe { transmute(Foo(&0, &0)) }; + //~^ ERROR transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data +} diff --git a/tests/ui/lint/mutable_transmutes/report_only_first_error.stderr b/tests/ui/lint/mutable_transmutes/report_only_first_error.stderr new file mode 100644 index 0000000000000..742c631b53b91 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/report_only_first_error.stderr @@ -0,0 +1,11 @@ +error: transmuting &T to &UnsafeCell is undefined behavior, even if the reference is unused, consider using UnsafeCell on the original data + --> $DIR/report_only_first_error.rs:10:28 + | +LL | let _a: Bar = unsafe { transmute(Foo(&0, &0)) }; + | ^^^^^^^^^ + | + = note: transmute from `*Foo.0` to `*Bar.0` + = note: `#[deny(unsafe_cell_transmutes)]` on by default + +error: aborting due to 1 previous error + diff --git a/tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.rs b/tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.rs new file mode 100644 index 0000000000000..bf71dc402badb --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.rs @@ -0,0 +1,21 @@ +use std::cell::UnsafeCell; + +#[repr(C)] +struct A { + a: u64, + b: u32, + c: u32, +} + +#[repr(C)] +struct B { + a: u64, + b: UnsafeCell, + c: u32, +} + +fn main() { + let a = A { a: 0, b: 0, c: 0 }; + let _b = unsafe { &*(&a as *const A as *const B) }; + //~^ ERROR casting `&T` to `&UnsafeCell` is undefined behavior, even if the reference is unused, consider using an `UnsafeCell` on the original data +} diff --git a/tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.stderr b/tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.stderr new file mode 100644 index 0000000000000..8e1dcddcdba13 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/unsafe_cell_reference_casting.stderr @@ -0,0 +1,11 @@ +error: casting `&T` to `&UnsafeCell` is undefined behavior, even if the reference is unused, consider using an `UnsafeCell` on the original data + --> $DIR/unsafe_cell_reference_casting.rs:19:23 + | +LL | let _b = unsafe { &*(&a as *const A as *const B) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: cast from `A.b` to `B.b` + = note: `#[deny(unsafe_cell_reference_casting)]` on by default + +error: aborting due to 1 previous error + diff --git a/tests/ui/lint/mutable_transmutes/unsized.rs b/tests/ui/lint/mutable_transmutes/unsized.rs new file mode 100644 index 0000000000000..e69bb1a1d295a --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/unsized.rs @@ -0,0 +1,19 @@ +use std::cell::UnsafeCell; + +#[repr(C)] +struct A { + a: u32, + b: T, +} + +#[repr(C)] +struct B { + a: UnsafeCell, + b: [u32], +} + +fn main() { + let a = &A { a: 0, b: [0_u32, 0] } as &A<[u32]>; + let _b = unsafe { &*(a as *const A<[u32]> as *const B) }; + //~^ ERROR casting `&T` to `&UnsafeCell` is undefined behavior, even if the reference is unused, consider using an `UnsafeCell` on the original data +} diff --git a/tests/ui/lint/mutable_transmutes/unsized.stderr b/tests/ui/lint/mutable_transmutes/unsized.stderr new file mode 100644 index 0000000000000..3a8e4251cf796 --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/unsized.stderr @@ -0,0 +1,11 @@ +error: casting `&T` to `&UnsafeCell` is undefined behavior, even if the reference is unused, consider using an `UnsafeCell` on the original data + --> $DIR/unsized.rs:17:23 + | +LL | let _b = unsafe { &*(a as *const A<[u32]> as *const B) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: cast from `A<[u32]>.a` to `B.a` + = note: `#[deny(unsafe_cell_reference_casting)]` on by default + +error: aborting due to 1 previous error +