From 3403b3e7175936f694403767b93803df3e606fc0 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Tue, 1 Feb 2022 14:53:12 -0500 Subject: [PATCH] Add lint `transumte_undefined_repr` --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_correctness.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.rs | 1 + clippy_lints/src/transmute/mod.rs | 31 +- .../src/transmute/transmute_undefined_repr.rs | 291 ++++++++++++++++++ tests/ui/crashes/ice-4968.rs | 1 + tests/ui/transmute_undefined_repr.rs | 44 +++ tests/ui/transmute_undefined_repr.stderr | 44 +++ 10 files changed, 415 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/transmute/transmute_undefined_repr.rs create mode 100644 tests/ui/transmute_undefined_repr.rs create mode 100644 tests/ui/transmute_undefined_repr.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 9548366daf9..7fb31f6d2b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3304,6 +3304,7 @@ Released 2018-09-13 [`transmute_num_to_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_num_to_bytes [`transmute_ptr_to_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr [`transmute_ptr_to_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref +[`transmute_undefined_repr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_undefined_repr [`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts [`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 4721b7f2b47..d93e34e76b4 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -277,6 +277,7 @@ LintId::of(transmute::TRANSMUTE_INT_TO_FLOAT), LintId::of(transmute::TRANSMUTE_NUM_TO_BYTES), LintId::of(transmute::TRANSMUTE_PTR_TO_REF), + LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), LintId::of(transmuting_null::TRANSMUTING_NULL), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 4217fd3a3ea..d013daa8e08 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -58,6 +58,7 @@ LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT), LintId::of(swap::ALMOST_SWAPPED), LintId::of(to_string_in_display::TO_STRING_IN_DISPLAY), + LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), LintId::of(transmuting_null::TRANSMUTING_NULL), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index e5e1c052c15..a80320a578f 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -473,6 +473,7 @@ transmute::TRANSMUTE_NUM_TO_BYTES, transmute::TRANSMUTE_PTR_TO_PTR, transmute::TRANSMUTE_PTR_TO_REF, + transmute::TRANSMUTE_UNDEFINED_REPR, transmute::UNSOUND_COLLECTION_TRANSMUTE, transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9d42185dc0e..0b07726519e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -5,6 +5,7 @@ #![feature(control_flow_enum)] #![feature(drain_filter)] #![feature(iter_intersperse)] +#![feature(let_chains)] #![feature(let_else)] #![feature(once_cell)] #![feature(rustc_private)] diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index c370941dd9c..4c320deecc2 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -7,6 +7,7 @@ mod transmute_ptr_to_ptr; mod transmute_ptr_to_ref; mod transmute_ref_to_ref; +mod transmute_undefined_repr; mod transmutes_expressible_as_ptr_casts; mod unsound_collection_transmute; mod useless_transmute; @@ -355,6 +356,30 @@ "transmute between collections of layout-incompatible types" } +declare_clippy_lint! { + /// ### What it does + /// Checks for transmutes either to or from a type which does not have a defined representation. + /// + /// ### Why is this bad? + /// The results of such a transmute are not defined. + /// + /// ### Example + /// ```rust + /// struct Foo(u32, T); + /// let _ = unsafe { core::mem::transmute::, Foo>(Foo(0u32, 0u32)) }; + /// ``` + /// Use instead: + /// ```rust + /// #[repr(C)] + /// struct Foo(u32, T); + /// let _ = unsafe { core::mem::transmute::, Foo>(Foo(0u32, 0u32)) }; + /// ``` + #[clippy::version = "1.60.0"] + pub TRANSMUTE_UNDEFINED_REPR, + correctness, + "transmute to or from a type with an undefined representation" +} + declare_lint_pass!(Transmute => [ CROSSPOINTER_TRANSMUTE, TRANSMUTE_PTR_TO_REF, @@ -369,6 +394,7 @@ TRANSMUTE_NUM_TO_BYTES, UNSOUND_COLLECTION_TRANSMUTE, TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, + TRANSMUTE_UNDEFINED_REPR, ]); impl<'tcx> LateLintPass<'tcx> for Transmute { @@ -402,7 +428,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { | transmute_int_to_float::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_float_to_int::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg, const_context) - | unsound_collection_transmute::check(cx, e, from_ty, to_ty); + | ( + unsound_collection_transmute::check(cx, e, from_ty, to_ty) + || transmute_undefined_repr::check(cx, e, from_ty, to_ty) + ); if !linted { transmutes_expressible_as_ptr_casts::check(cx, e, from_ty, to_ty, arg); diff --git a/clippy_lints/src/transmute/transmute_undefined_repr.rs b/clippy_lints/src/transmute/transmute_undefined_repr.rs new file mode 100644 index 00000000000..4b7c493278f --- /dev/null +++ b/clippy_lints/src/transmute/transmute_undefined_repr.rs @@ -0,0 +1,291 @@ +use super::TRANSMUTE_UNDEFINED_REPR; +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::subst::{GenericArg, Subst}; +use rustc_middle::ty::{self, Ty, TyS, TypeAndMut}; +use rustc_span::Span; + +#[allow(clippy::too_many_lines)] +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + from_ty_orig: Ty<'tcx>, + to_ty_orig: Ty<'tcx>, +) -> bool { + let mut from_ty = cx.tcx.erase_regions(from_ty_orig); + let mut to_ty = cx.tcx.erase_regions(to_ty_orig); + + while !TyS::same_type(from_ty, to_ty) { + match reduce_refs(cx, e.span, from_ty, to_ty) { + ReducedTys::FromFatPtr { unsized_ty, .. } => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute from `{}` which has an undefined layout", from_ty_orig), + |diag| { + if !TyS::same_type(from_ty_orig.peel_refs(), unsized_ty) { + diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); + } + }, + ); + return true; + }, + ReducedTys::ToFatPtr { unsized_ty, .. } => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute to `{}` which has an undefined layout", to_ty_orig), + |diag| { + if !TyS::same_type(to_ty_orig.peel_refs(), unsized_ty) { + diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); + } + }, + ); + return true; + }, + ReducedTys::ToPtr { + from_ty: from_sub_ty, + to_ty: to_sub_ty, + } => match reduce_ty(cx, from_sub_ty) { + ReducedTy::UnorderedFields(from_ty) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute from `{}` which has an undefined layout", from_ty_orig), + |diag| { + if !TyS::same_type(from_ty_orig.peel_refs(), from_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); + } + }, + ); + return true; + }, + ReducedTy::Ref(from_sub_ty) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + _ => break, + }, + ReducedTys::FromPtr { + from_ty: from_sub_ty, + to_ty: to_sub_ty, + } => match reduce_ty(cx, to_sub_ty) { + ReducedTy::UnorderedFields(to_ty) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute to `{}` which has an undefined layout", to_ty_orig), + |diag| { + if !TyS::same_type(to_ty_orig.peel_refs(), to_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); + } + }, + ); + return true; + }, + ReducedTy::Ref(to_sub_ty) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + _ => break, + }, + ReducedTys::Other { + from_ty: from_sub_ty, + to_ty: to_sub_ty, + } => match (reduce_ty(cx, from_sub_ty), reduce_ty(cx, to_sub_ty)) { + (ReducedTy::IntArray, _) | (_, ReducedTy::IntArray) => return false, + (ReducedTy::UnorderedFields(from_ty), ReducedTy::UnorderedFields(to_ty)) + if !TyS::same_type(from_ty, to_ty) => + { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!( + "transmute from `{}` to `{}`, both of which have an undefined layout", + from_ty_orig, to_ty_orig + ), + |diag| { + if let (Some(from_def), Some(to_def)) = (from_ty.ty_adt_def(), to_ty.ty_adt_def()) + && from_def == to_def + { + diag.note(&format!( + "two instances of the same generic type (`{}`) may have different layouts", + cx.tcx.item_name(from_def.did) + )); + } else { + if !TyS::same_type(from_ty_orig.peel_refs(), from_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); + } + if !TyS::same_type(to_ty_orig.peel_refs(), to_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); + } + } + }, + ); + return true; + }, + ( + ReducedTy::UnorderedFields(from_ty), + ReducedTy::Other(_) | ReducedTy::OrderedFields(_) | ReducedTy::Ref(_), + ) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute from `{}` which has an undefined layout", from_ty_orig), + |diag| { + if !TyS::same_type(from_ty_orig.peel_refs(), from_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", from_ty)); + } + }, + ); + return true; + }, + ( + ReducedTy::Other(_) | ReducedTy::OrderedFields(_) | ReducedTy::Ref(_), + ReducedTy::UnorderedFields(to_ty), + ) => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute into `{}` which has an undefined layout", to_ty_orig), + |diag| { + if !TyS::same_type(to_ty_orig.peel_refs(), to_ty) { + diag.note(&format!("the contained type `{}` has an undefined layout", to_ty)); + } + }, + ); + return true; + }, + (ReducedTy::Ref(from_sub_ty), ReducedTy::Ref(to_sub_ty)) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + ( + ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_), + ReducedTy::OrderedFields(_) | ReducedTy::Ref(_) | ReducedTy::Other(_), + ) + | (ReducedTy::UnorderedFields(_), ReducedTy::UnorderedFields(_)) => break, + }, + } + } + + false +} + +enum ReducedTys<'tcx> { + FromFatPtr { unsized_ty: Ty<'tcx> }, + ToFatPtr { unsized_ty: Ty<'tcx> }, + ToPtr { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, + FromPtr { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, + Other { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, +} + +fn reduce_refs<'tcx>( + cx: &LateContext<'tcx>, + span: Span, + mut from_ty: Ty<'tcx>, + mut to_ty: Ty<'tcx>, +) -> ReducedTys<'tcx> { + loop { + return match (from_ty.kind(), to_ty.kind()) { + ( + ty::Ref(_, from_sub_ty, _) | ty::RawPtr(TypeAndMut { ty: from_sub_ty, .. }), + ty::Ref(_, to_sub_ty, _) | ty::RawPtr(TypeAndMut { ty: to_sub_ty, .. }), + ) => { + from_ty = from_sub_ty; + to_ty = to_sub_ty; + continue; + }, + (ty::Ref(_, unsized_ty, _) | ty::RawPtr(TypeAndMut { ty: unsized_ty, .. }), _) + if !unsized_ty.is_sized(cx.tcx.at(span), cx.param_env) => + { + ReducedTys::FromFatPtr { unsized_ty } + }, + (_, ty::Ref(_, unsized_ty, _) | ty::RawPtr(TypeAndMut { ty: unsized_ty, .. })) + if !unsized_ty.is_sized(cx.tcx.at(span), cx.param_env) => + { + ReducedTys::ToFatPtr { unsized_ty } + }, + (ty::Ref(_, from_ty, _) | ty::RawPtr(TypeAndMut { ty: from_ty, .. }), _) => { + ReducedTys::FromPtr { from_ty, to_ty } + }, + (_, ty::Ref(_, to_ty, _) | ty::RawPtr(TypeAndMut { ty: to_ty, .. })) => { + ReducedTys::ToPtr { from_ty, to_ty } + }, + _ => ReducedTys::Other { from_ty, to_ty }, + }; + } +} + +enum ReducedTy<'tcx> { + OrderedFields(Ty<'tcx>), + UnorderedFields(Ty<'tcx>), + Ref(Ty<'tcx>), + Other(Ty<'tcx>), + IntArray, +} + +fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> { + loop { + ty = cx.tcx.try_normalize_erasing_regions(cx.param_env, ty).unwrap_or(ty); + return match *ty.kind() { + ty::Array(sub_ty, _) if matches!(sub_ty.kind(), ty::Int(_) | ty::Uint(_)) => ReducedTy::IntArray, + ty::Array(sub_ty, _) | ty::Slice(sub_ty) => { + ty = sub_ty; + continue; + }, + ty::Tuple(args) => { + let mut iter = args.iter().map(GenericArg::expect_ty); + let Some(sized_ty) = iter.find(|ty| !is_zero_sized_ty(cx, ty)) else { + return ReducedTy::OrderedFields(ty); + }; + if iter.all(|ty| is_zero_sized_ty(cx, ty)) { + ty = sized_ty; + continue; + } + ReducedTy::UnorderedFields(ty) + }, + ty::Adt(def, substs) if def.is_struct() => { + if def.repr.inhibit_struct_field_reordering_opt() { + return ReducedTy::OrderedFields(ty); + } + let mut iter = def + .non_enum_variant() + .fields + .iter() + .map(|f| cx.tcx.type_of(f.did).subst(cx.tcx, substs)); + let Some(sized_ty) = iter.find(|ty| !is_zero_sized_ty(cx, ty)) else { + return ReducedTy::OrderedFields(ty); + }; + if iter.all(|ty| is_zero_sized_ty(cx, ty)) { + ty = sized_ty; + continue; + } + ReducedTy::UnorderedFields(ty) + }, + ty::Ref(..) | ty::RawPtr(_) => ReducedTy::Ref(ty), + _ => ReducedTy::Other(ty), + }; + } +} + +fn is_zero_sized_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + if let Ok(ty) = cx.tcx.try_normalize_erasing_regions(cx.param_env, ty) + && let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty)) + { + layout.layout.size.bytes() == 0 + } else { + false + } +} diff --git a/tests/ui/crashes/ice-4968.rs b/tests/ui/crashes/ice-4968.rs index 3822f174598..e0510d942c2 100644 --- a/tests/ui/crashes/ice-4968.rs +++ b/tests/ui/crashes/ice-4968.rs @@ -3,6 +3,7 @@ // Test for https://github.com/rust-lang/rust-clippy/issues/4968 #![warn(clippy::unsound_collection_transmute)] +#![allow(clippy::transmute_undefined_repr)] trait Trait { type Assoc; diff --git a/tests/ui/transmute_undefined_repr.rs b/tests/ui/transmute_undefined_repr.rs new file mode 100644 index 00000000000..71539940fbf --- /dev/null +++ b/tests/ui/transmute_undefined_repr.rs @@ -0,0 +1,44 @@ +#![warn(clippy::transmute_undefined_repr)] +#![allow(clippy::unit_arg)] + +fn value() -> T { + unimplemented!() +} + +struct Empty; +struct Ty(T); +struct Ty2(T, U); + +#[repr(C)] +struct Ty2C(T, U); + +fn main() { + unsafe { + let _: () = core::mem::transmute(value::()); + let _: Empty = core::mem::transmute(value::<()>()); + + let _: Ty = core::mem::transmute(value::()); + let _: Ty = core::mem::transmute(value::()); + + let _: Ty2C = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + let _: Ty2 = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + + let _: Ty2 = core::mem::transmute(value::>>()); // Ok, Ty2 types are the same + let _: Ty> = core::mem::transmute(value::>()); // Ok, Ty2 types are the same + + let _: Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + let _: Ty> = core::mem::transmute(value::>()); // Lint, different Ty2 instances + + let _: Ty<&()> = core::mem::transmute(value::<&()>()); + let _: &() = core::mem::transmute(value::>()); + + let _: &Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + let _: Ty<&Ty2> = core::mem::transmute(value::<&Ty2>()); // Lint, different Ty2 instances + + let _: Ty = core::mem::transmute(value::<&Ty2>()); // Ok, pointer to usize conversion + let _: &Ty2 = core::mem::transmute(value::>()); // Ok, pointer to usize conversion + + let _: Ty<[u8; 8]> = core::mem::transmute(value::>()); // Ok, transmute to byte array + let _: Ty2 = core::mem::transmute(value::>()); // Ok, transmute from byte array + } +} diff --git a/tests/ui/transmute_undefined_repr.stderr b/tests/ui/transmute_undefined_repr.stderr new file mode 100644 index 00000000000..040c63c7afa --- /dev/null +++ b/tests/ui/transmute_undefined_repr.stderr @@ -0,0 +1,44 @@ +error: transmute from `Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:23:33 + | +LL | let _: Ty2C = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::transmute-undefined-repr` implied by `-D warnings` + +error: transmute into `Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:24:32 + | +LL | let _: Ty2 = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: transmute from `Ty>` to `Ty2`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:29:32 + | +LL | let _: Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Ty2`) may have different layouts + +error: transmute from `Ty2` to `Ty>`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:30:36 + | +LL | let _: Ty> = core::mem::transmute(value::>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Ty2`) may have different layouts + +error: transmute to `&Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:35:33 + | +LL | let _: &Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: transmute from `&Ty2` which has an undefined layout + --> $DIR/transmute_undefined_repr.rs:36:37 + | +LL | let _: Ty<&Ty2> = core::mem::transmute(value::<&Ty2>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + -- 2.44.0