diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 2db610d640c29..07b07ee427732 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -95,6 +95,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 @@ -140,6 +141,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_impl = implementation of an `unsafe` trait lint_builtin_unsafe_trait = declaration of an `unsafe` trait diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 8932200c5b7b1..83fb5fce78049 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -29,11 +29,10 @@ use crate::{ BuiltinDerefNullptr, BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents, - BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, - BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, - BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasGenericBounds, - BuiltinTypeAliasGenericBoundsSuggestion, BuiltinTypeAliasWhereClause, - BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, + BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinNoMangleGeneric, + BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, + BuiltinTypeAliasGenericBounds, BuiltinTypeAliasGenericBoundsSuggestion, + BuiltinTypeAliasWhereClause, BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, BuiltinWhileTrue, SuggestChangingAssocTypes, @@ -1165,72 +1164,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_spanned_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) && cx.tcx.item_name(def_id) == sym::transmute - } - } -} - declare_lint! { /// The `unstable_features` is deprecated and should no longer be used. UNSTABLE_FEATURES, @@ -1641,7 +1574,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 8e95a71e11f1e..73efb6e055a7b 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -74,6 +74,7 @@ mod lints; mod map_unit_fn; mod methods; mod multiple_supertrait_upcastable; +mod mutable_transmutes; mod non_ascii_idents; mod non_fmt_panic; mod nonstandard_style; @@ -112,6 +113,7 @@ use let_underscore::*; use map_unit_fn::*; use methods::*; use multiple_supertrait_upcastable::*; +use mutable_transmutes::MutableTransmutes; use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; use nonstandard_style::*; diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 9fda53c25334a..59dbd61b1dd69 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -230,7 +230,19 @@ pub struct BuiltinConstNoMangle { #[derive(LintDiagnostic)] #[diag(lint_builtin_mutable_transmutes)] -pub struct BuiltinMutablesTransmutes; +#[note] +pub struct BuiltinMutablesTransmutes { + pub from: String, + pub to: String, +} + +#[derive(LintDiagnostic)] +#[diag(lint_builtin_unsafe_cell_transmutes)] +#[note] +pub struct BuiltinUnsafeCellTransmutes { + 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..ef3c2a296791f --- /dev/null +++ b/compiler/rustc_lint/src/mutable_transmutes.rs @@ -0,0 +1,587 @@ +//! 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 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 crate::{ + lints::{BuiltinMutablesTransmutes, BuiltinUnsafeCellTransmutes}, + LateContext, LateLintPass, LintContext, +}; +use rustc_hir as hir; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; +use rustc_index::IndexSlice; +use rustc_middle::ty::layout::LayoutOf; +use rustc_middle::ty::{self, AdtDef, GenericArgsRef, List, Ty}; +use rustc_span::symbol::sym; +use rustc_target::abi::{FieldIdx, FieldsShape, HasDataLayout, Layout, Size}; +use rustc_type_ir::Mutability; + +use std::fmt::Write; +use std::ops::ControlFlow; + +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! { + /// 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 + 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>, + ) { + let mut breadcrumbs = self.0.as_slice(); + while let &[current_breadcrumb, ref rest_breadcrumbs @ ..] = breadcrumbs { + breadcrumbs = rest_breadcrumbs; + + 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()); + + ty = field_ty; + } + + &ty::Tuple(tys) => { + let field_ty = tys[current_breadcrumb.as_usize()]; + write!(output, ".{}", current_breadcrumb.as_u32()).unwrap(); + + ty = field_ty; + } + + &ty::Array(element_ty, _) => { + output.push_str("[0]"); + + ty = 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 { + CheckFields, + DoNotCheckFields, +} + +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::DoNotCheckFields { + return; + } + + match ty.kind() { + &ty::Adt(adt_def, substs) if adt_def.is_struct() => { + let Ok(layout) = cx.layout_of(ty) else { + return; + }; + 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; + }; + 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 => + { + 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; +} + +enum CheckOverlapping { + CheckFields, + DoNotCheckFields, + /// 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], + mut first_maybe_overlapping: usize, + ty: Ty<'tcx>, + offset: Size, + on_overlapping: &mut dyn FnMut(Ty<'tcx>, usize, &FieldsBreadcrumbs) -> CheckOverlapping, + fields_breadcrumbs: &mut FieldsBreadcrumbs, +) -> ControlFlow<()> { + let Ok(layout) = cx.layout_of(ty) else { + return ControlFlow::Continue(()); + }; + + // First, advance the cursor to the first potentially overlapping entry, + // in order to speed up lookup for fields. + while first_maybe_overlapping < check_overlap_with.len() + && check_overlap_with[first_maybe_overlapping].end_offset() < offset + { + first_maybe_overlapping += 1; + } + + if first_maybe_overlapping >= check_overlap_with.len() { + return ControlFlow::Continue(()); + } + + // 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 = first_maybe_overlapping; + while idx < check_overlap_with.len() + && check_overlap_with[idx].start_offset() < offset + layout.size + { + match on_overlapping(ty, idx, &fields_breadcrumbs) { + CheckOverlapping::CheckFields => {} + CheckOverlapping::DoNotCheckFields => 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, + first_maybe_overlapping, + 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, + first_maybe_overlapping, + 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, + first_maybe_overlapping, + 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 + } +} + +impl<'tcx> LateLintPass<'tcx> for MutableTransmutes { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) { + let Some((src, dst)) = get_transmute_from_to(cx, expr) else { return }; + + 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::DoNotCheckFields + } + _ => CollectFields::CheckFields, + }, + &mut FieldsBreadcrumbs::default(), + ); + dst_refs.sort_unstable_by_key(|ref_| ref_.start_offset); + check_overlapping( + cx, + &dst_refs, + 0, + 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_spanned_lint( + MUTABLE_TRANSMUTES, + expr.span, + BuiltinMutablesTransmutes { from, to }, + ); + CheckOverlapping::ReportedError + } + _ => CheckOverlapping::CheckFields, + }, + // 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::CheckFields; + }; + + 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::CheckFields; + }; + if layout.is_zst() { + return CollectFields::DoNotCheckFields; + } + + let Ok(layout) = cx.layout_of(ty) else { + return CollectFields::CheckFields; + }; + dst_unsafe_cells.push(UnsafeCellField { + start_offset: offset, + end_offset: offset + layout.size, + fields_breadcrumbs: fields_breadcrumbs.clone(), + }); + CollectFields::DoNotCheckFields + } + _ => CollectFields::CheckFields, + }, + &mut FieldsBreadcrumbs::default(), + ); + dst_unsafe_cells + .sort_unstable_by_key(|unsafe_cell| unsafe_cell.start_offset); + + let found_error = check_overlapping( + cx, + &dst_unsafe_cells, + 0, + 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::DoNotCheckFields; + } + + 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::CheckFields, + } + + 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_unsafe_cells[idx].fields_breadcrumbs, + dst_ref.ty, + ); + + cx.emit_spanned_lint( + UNSAFE_CELL_TRANSMUTES, + expr.span, + BuiltinUnsafeCellTransmutes { from, to }, + ); + + CheckOverlapping::ReportedError + }, + &mut FieldsBreadcrumbs::default(), + ); + match found_error { + ControlFlow::Continue(()) => CheckOverlapping::CheckFields, + ControlFlow::Break(()) => CheckOverlapping::ReportedError, + } + } + } + }, + &mut FieldsBreadcrumbs::default(), + ); + } +} + +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) && cx.tcx.item_name(def_id) == sym::transmute +} 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..37947cdc24560 --- /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..bb384b1432428 --- /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..a93761307418f --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/allow_unsafecell_to_unsafecell.rs @@ -0,0 +1,8 @@ +// check-pass + +use std::cell::UnsafeCell; +use std::mem::transmute; + +fn main() { + let _a: &UnsafeCell = unsafe { transmute(&UnsafeCell::new(0u8)) }; +} 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_unsafecell.rs b/tests/ui/lint/mutable_transmutes/imm_to_unsafecell.rs new file mode 100644 index 0000000000000..469d52d1dcd1d --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/imm_to_unsafecell.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_unsafecell.stderr b/tests/ui/lint/mutable_transmutes/imm_to_unsafecell.stderr new file mode 100644 index 0000000000000..dd9bb6a58fcdf --- /dev/null +++ b/tests/ui/lint/mutable_transmutes/imm_to_unsafecell.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_unsafecell.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/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 +