From 053455f3760ece3411e239e438074ce1d74fb8db Mon Sep 17 00:00:00 2001 From: Orion Gonzalez Date: Fri, 29 Nov 2024 19:48:16 +0100 Subject: [PATCH] Check for changes to interior mutability in `mem::transmute`. --- compiler/rustc_lint/messages.ftl | 4 + compiler/rustc_lint/src/builtin.rs | 90 ++++++++++++------- compiler/rustc_lint/src/lints.rs | 7 ++ compiler/rustc_lint/src/passes.rs | 1 - tests/ui/consts/const-eval/issue-55541.rs | 1 + tests/ui/consts/const-eval/issue-55541.stderr | 27 ++++++ .../interior-mutability-issue-111229.rs | 16 ++++ .../interior-mutability-issue-111229.stderr | 11 +++ .../transmute-interior-mutability.rs | 19 ++++ .../transmute-interior-mutability.stderr | 27 ++++++ 10 files changed, 171 insertions(+), 32 deletions(-) create mode 100644 tests/ui/consts/const-eval/issue-55541.stderr create mode 100644 tests/ui/transmute/interior-mutability-issue-111229.rs create mode 100644 tests/ui/transmute/interior-mutability-issue-111229.stderr create mode 100644 tests/ui/transmute/transmute-interior-mutability.rs create mode 100644 tests/ui/transmute/transmute-interior-mutability.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 9df0c50868cb1..615532b269f4b 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -142,6 +142,10 @@ lint_builtin_special_module_name_used_lib = found module declaration for lib.rs lint_builtin_special_module_name_used_main = found module declaration for main.rs .note = a binary crate cannot be used as library +lint_builtin_transmute_to_interior_mutability = + transmuting from a type without interior mutability to a type with interior mutability is undefined behaviour if you modify the content + .help = `{$output_ty}` has interior mutability + lint_builtin_trivial_bounds = {$predicate_kind_name} bound {$predicate} does not depend on any type or lifetime parameters lint_builtin_type_alias_bounds_enable_feat_help = add `#![feature(lazy_type_alias)]` to the crate attributes to enable the desired semantics diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index e130cfc1d736d..75a42def8786c 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -56,10 +56,10 @@ use crate::lints::{ BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents, BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed, - BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller, - BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, - BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, - BuiltinWhileTrue, InvalidAsmLabel, + BuiltinTransmuteInteriorMutability, BuiltinTrivialBounds, BuiltinTypeAliasBounds, + BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, + BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, + BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel, }; use crate::nonstandard_style::{MethodLateContext, method_context}; use crate::{ @@ -1103,41 +1103,68 @@ declare_lint! { "transmuting &T to &mut T is undefined behavior, even if the reference is unused" } +declare_lint! { + /// The `transmute_to_interior_mutability` lint catches transmuting from `&T` or `*const T` to another type + /// with interior mutability because changing its value is [undefined behavior]. + /// + /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html + /// + /// ### Example + /// + /// ```rust,compile_fail + /// unsafe { + /// let x = std::mem::transmute::<&i32, UnsafeCell>(&5); + /// let y = std::mem::transmute::<&i32, AtomicI32>(&5); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// It's probably a mistake to transmute a type without interior mutability to a type with it. + TRANSMUTE_TO_INTERIOR_MUTABILITY, + Warn, + "transmuting from a type without interior mutability to a type with interior mutability is undefined behaviour if you modify the content" +} + +declare_lint_pass!(TransmuteToInteriorMutability => [TRANSMUTE_TO_INTERIOR_MUTABILITY]); 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); - } + // Is our expr mem::transmute? + let hir::ExprKind::Path(ref qpath) = expr.kind else { return }; + let def: Res = cx.qpath_res(qpath, expr.hir_id); + let Res::Def(DefKind::Fn, def_id) = def else { return }; + if !cx.tcx.is_intrinsic(def_id, sym::transmute) { + return; } + let sig = cx.typeck_results().node_type(expr.hir_id).fn_sig(cx.tcx); + let input = sig.inputs().skip_binder()[0]; + let output = sig.output().skip_binder(); - 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; + // For both checks we only care if the input is an immutable ref + let &ty::Ref(_, input_deref, Mutability::Not) = input.kind() else { return }; + match output.kind() { + // If the output is a mutable reference that's bad + &ty::Ref(_, _, Mutability::Mut) => { + cx.emit_span_lint(MUTABLE_TRANSMUTES, expr.span, BuiltinMutablesTransmutes); + } + &ty::Ref(_, output_deref, Mutability::Not) => { + // If the input type doesn't have interior mutability but the output type does: + if input_deref.is_freeze(cx.tcx, cx.typing_env()) + && !output_deref.is_freeze(cx.tcx, cx.typing_env()) + { + cx.emit_span_lint( + TRANSMUTE_TO_INTERIOR_MUTABILITY, + expr.span, + BuiltinTransmuteInteriorMutability { output_ty: output_deref }, + ); } - 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) + // The user is transmuting a reference into something else, warn...? + _ => (), } } } @@ -1596,6 +1623,7 @@ declare_lint_pass!( NO_MANGLE_CONST_ITEMS, NO_MANGLE_GENERIC_ITEMS, MUTABLE_TRANSMUTES, + TRANSMUTE_TO_INTERIOR_MUTABILITY, UNSTABLE_FEATURES, UNREACHABLE_PUB, TYPE_ALIAS_BOUNDS, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 3a854796f6735..8c2258469c1c6 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -228,6 +228,13 @@ pub(crate) struct BuiltinConstNoMangle { #[diag(lint_builtin_mutable_transmutes)] pub(crate) struct BuiltinMutablesTransmutes; +#[derive(LintDiagnostic)] +#[diag(lint_builtin_transmute_to_interior_mutability)] +#[help] +pub(crate) struct BuiltinTransmuteInteriorMutability<'tcx> { + pub output_ty: Ty<'tcx>, +} + #[derive(LintDiagnostic)] #[diag(lint_builtin_unstable_features)] pub(crate) struct BuiltinUnstableFeatures; diff --git a/compiler/rustc_lint/src/passes.rs b/compiler/rustc_lint/src/passes.rs index 9d84d36e779e8..f1fb4c448bd91 100644 --- a/compiler/rustc_lint/src/passes.rs +++ b/compiler/rustc_lint/src/passes.rs @@ -57,7 +57,6 @@ macro_rules! late_lint_methods { // // FIXME: eliminate the duplication with `Visitor`. But this also // contains a few lint-specific methods with no equivalent in `Visitor`. - macro_rules! declare_late_lint_pass { ([], [$($(#[$attr:meta])* fn $name:ident($($param:ident: $arg:ty),*);)*]) => ( pub trait LateLintPass<'tcx>: LintPass { diff --git a/tests/ui/consts/const-eval/issue-55541.rs b/tests/ui/consts/const-eval/issue-55541.rs index 21bc89c2dda22..953c49a7bb299 100644 --- a/tests/ui/consts/const-eval/issue-55541.rs +++ b/tests/ui/consts/const-eval/issue-55541.rs @@ -3,6 +3,7 @@ // Test that we can handle newtypes wrapping extern types #![feature(extern_types)] +#![allow(transmute_to_interior_mutability)] use std::marker::PhantomData; diff --git a/tests/ui/consts/const-eval/issue-55541.stderr b/tests/ui/consts/const-eval/issue-55541.stderr new file mode 100644 index 0000000000000..f068e08060fed --- /dev/null +++ b/tests/ui/consts/const-eval/issue-55541.stderr @@ -0,0 +1,27 @@ +warning: unknown lint: `transmute_to_interior_mutability` + --> $DIR/issue-55541.rs:6:10 + | +LL | #![allow(transmute_to_interior_mutability)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unknown_lints)]` on by default + +warning: transmuting from a type without interior mutability to a type with interior mutability is undefined behaviour if you modify the content + --> $DIR/issue-55541.rs:19:3 + | +LL | std::mem::transmute(&MAGIC_FFI_STATIC) + | ^^^^^^^^^^^^^^^^^^^ + | + = help: `Wrapper` has interior mutability + = note: `#[warn(transmute_to_interior_mutability)]` on by default + +warning: transmuting from a type without interior mutability to a type with interior mutability is undefined behaviour if you modify the content + --> $DIR/issue-55541.rs:25:3 + | +LL | std::mem::transmute(&MAGIC_FFI_STATIC) + | ^^^^^^^^^^^^^^^^^^^ + | + = help: `Wrapper2` has interior mutability + +warning: 3 warnings emitted + diff --git a/tests/ui/transmute/interior-mutability-issue-111229.rs b/tests/ui/transmute/interior-mutability-issue-111229.rs new file mode 100644 index 0000000000000..27ec2012d2c04 --- /dev/null +++ b/tests/ui/transmute/interior-mutability-issue-111229.rs @@ -0,0 +1,16 @@ +//@ run-pass +// https://github.com/rust-lang/rust/issues/111229 + +pub struct Foo(std::cell::UnsafeCell); +pub struct Bar([u8; 0]); + +pub fn foo(f: &Bar) { + unsafe { + let f = std::mem::transmute::<&Bar, &Foo>(f); + //~^ WARNING transmuting from a type without interior mutability to a type with interior mutability + //~| HELP `Foo` has interior mutability + *(f.0.get()) += 1; + } +} + +fn main() {} diff --git a/tests/ui/transmute/interior-mutability-issue-111229.stderr b/tests/ui/transmute/interior-mutability-issue-111229.stderr new file mode 100644 index 0000000000000..f939af0ba7259 --- /dev/null +++ b/tests/ui/transmute/interior-mutability-issue-111229.stderr @@ -0,0 +1,11 @@ +warning: transmuting from a type without interior mutability to a type with interior mutability is undefined behaviour if you modify the content + --> $DIR/interior-mutability-issue-111229.rs:9:17 + | +LL | let f = std::mem::transmute::<&Bar, &Foo>(f); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `Foo` has interior mutability + = note: `#[warn(transmute_to_interior_mutability)]` on by default + +warning: 1 warning emitted + diff --git a/tests/ui/transmute/transmute-interior-mutability.rs b/tests/ui/transmute/transmute-interior-mutability.rs new file mode 100644 index 0000000000000..4b19a7013cd90 --- /dev/null +++ b/tests/ui/transmute/transmute-interior-mutability.rs @@ -0,0 +1,19 @@ +#![allow(unused)] +use std::{cell::UnsafeCell, mem, sync::atomic::AtomicI32}; + +fn main() { + unsafe { + mem::transmute::<&i32, &UnsafeCell>(&42); + //~^ WARNING transmuting from a type without interior mutability to a type with interior mutability + //~| HELP `UnsafeCell` has interior mutability + // It's an error to transmute to a type containing unsafe cell + mem::transmute::<&i32, &AtomicI32>(&42); + //~^ WARNING transmuting from a type without interior mutability to a type with interior mutability + //~| HELP `AtomicI32` has interior mutability + // mutable_transmutes triggers before + + // This one is here because & -> &mut is worse, to assert that this one triggers. + mem::transmute::<&i32, &mut UnsafeCell>(&42); + //~^ 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/transmute/transmute-interior-mutability.stderr b/tests/ui/transmute/transmute-interior-mutability.stderr new file mode 100644 index 0000000000000..6ac4d93aea805 --- /dev/null +++ b/tests/ui/transmute/transmute-interior-mutability.stderr @@ -0,0 +1,27 @@ +warning: transmuting from a type without interior mutability to a type with interior mutability is undefined behaviour if you modify the content + --> $DIR/transmute-interior-mutability.rs:6:9 + | +LL | mem::transmute::<&i32, &UnsafeCell>(&42); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `UnsafeCell` has interior mutability + = note: `#[warn(transmute_to_interior_mutability)]` on by default + +warning: transmuting from a type without interior mutability to a type with interior mutability is undefined behaviour if you modify the content + --> $DIR/transmute-interior-mutability.rs:10:9 + | +LL | mem::transmute::<&i32, &AtomicI32>(&42); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `AtomicI32` has interior mutability + +error: transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell + --> $DIR/transmute-interior-mutability.rs:16:9 + | +LL | mem::transmute::<&i32, &mut UnsafeCell>(&42); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[deny(mutable_transmutes)]` on by default + +error: aborting due to 1 previous error; 2 warnings emitted +