From 2d3b1452c1cb3f886780c088decca37cdc589087 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 10 Feb 2023 17:52:57 +0900 Subject: [PATCH 1/7] Add `transmute_slice_to_larger_element_type` lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/transmute/mod.rs | 22 ++++++ .../transmute_slice_to_larger_element_type.rs | 71 +++++++++++++++++++ .../transmute_slice_to_larger_element_type.rs | 9 +++ ...nsmute_slice_to_larger_element_type.stderr | 17 +++++ 6 files changed, 121 insertions(+) create mode 100644 clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs create mode 100644 tests/ui/transmute_slice_to_larger_element_type.rs create mode 100644 tests/ui/transmute_slice_to_larger_element_type.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 659e8aebcd57..409457d28c0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4794,6 +4794,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_slice_to_larger_element_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_slice_to_larger_element_type [`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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 457a25826e79..1e8dc5cc76ed 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -577,6 +577,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::transmute::TRANSMUTE_NUM_TO_BYTES_INFO, crate::transmute::TRANSMUTE_PTR_TO_PTR_INFO, crate::transmute::TRANSMUTE_PTR_TO_REF_INFO, + crate::transmute::TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE_INFO, crate::transmute::TRANSMUTE_UNDEFINED_REPR_INFO, crate::transmute::TRANSMUTING_NULL_INFO, crate::transmute::UNSOUND_COLLECTION_TRANSMUTE_INFO, diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index c0d290b5adc4..dd4d743297a2 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -8,6 +8,7 @@ mod transmute_num_to_bytes; mod transmute_ptr_to_ptr; mod transmute_ptr_to_ref; mod transmute_ref_to_ref; +mod transmute_slice_to_larger_element_type; mod transmute_undefined_repr; mod transmutes_expressible_as_ptr_casts; mod transmuting_null; @@ -438,6 +439,25 @@ declare_clippy_lint! { "transmute results in a null function pointer, which is undefined behavior" } +declare_clippy_lint! { + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "1.69.0"] + pub TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE, + correctness, + "default lint description" +} + pub struct Transmute { msrv: Msrv, } @@ -458,6 +478,7 @@ impl_lint_pass!(Transmute => [ TRANSMUTE_UNDEFINED_REPR, TRANSMUTING_NULL, TRANSMUTE_NULL_TO_FN, + TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE, ]); impl Transmute { #[must_use] @@ -503,6 +524,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { | 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) + | transmute_slice_to_larger_element_type::check(cx, e, from_ty, to_ty, arg) | ( unsound_collection_transmute::check(cx, e, from_ty, to_ty) || transmute_undefined_repr::check(cx, e, from_ty, to_ty) diff --git a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs new file mode 100644 index 000000000000..7394c35ab535 --- /dev/null +++ b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs @@ -0,0 +1,71 @@ +use super::TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::reindent_multiline; +use clippy_utils::sugg; +use clippy_utils::ty::approx_ty_size; +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, Ty}; +use std::borrow::Cow; + +// TODO: Adjust the parameters as necessary +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + call_to_transmute: &'tcx Expr<'_>, + from_ty: Ty<'tcx>, + to_ty: Ty<'tcx>, + transmute_arg: &'tcx Expr<'_>, +) -> bool { + if let (ty::Ref(_, ty_from, _), ty::Ref(_, ty_to, _)) = (&from_ty.kind(), &to_ty.kind()) { + if let (&ty::Slice(ty_elem_from), &ty::Slice(ty_elem_to)) = (&ty_from.kind(), &ty_to.kind()) { + let ty_eleme_from_size = approx_ty_size(cx, *ty_elem_from); + let ty_elem_to_size = approx_ty_size(cx, *ty_elem_to); + if ty_eleme_from_size < ty_elem_to_size { + // this is UB!! + span_lint_and_then( + cx, + TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE, + call_to_transmute.span, + &format!("transmute from `&[{ty_elem_from}]` to `&[{ty_elem_to}]` results in undefined behavior"), + |diag| { + let transmute_arg = sugg::Sugg::hir(cx, transmute_arg, ".."); + // TODO: In this case, outer unsafe block is not needed anymore. It should be removed in + // suggestion. + let sugg_reallocate = format!( + "{transmute_arg}\ + .iter()\ + .map(|item| unsafe {{ std::mem::transmute(item) }})\ + .collect::>()\ + .to_slice()" + ); + let sugg_reallocate = Cow::from(sugg_reallocate); + let sugg_align_to = format!("std::slice::align_to::<{ty_elem_to}>({transmute_arg}).1"); + let sugg_align_to = Cow::from(sugg_align_to); + diag.note("this transmute leads out-of-bounds read"); + diag.span_suggestions( + call_to_transmute.span, + "try", + [ + reindent_multiline(sugg_reallocate, true, None).to_string(), + // TODO: this suggestion does not check if there's prefix and postfix. + // NOTE: this is not what user want to do if ty_elem_to is ZST; however, + // this lint will not fire in such case anyway (ZSTs cannot be larger than any type). + reindent_multiline(sugg_align_to, true, None).to_string(), + ], + Applicability::Unspecified, + ); + }, + ); + + true + } else { + false + } + } else { + false + } + } else { + false + } +} diff --git a/tests/ui/transmute_slice_to_larger_element_type.rs b/tests/ui/transmute_slice_to_larger_element_type.rs new file mode 100644 index 000000000000..0c63c12dca94 --- /dev/null +++ b/tests/ui/transmute_slice_to_larger_element_type.rs @@ -0,0 +1,9 @@ +#![allow(unused)] +#![warn(clippy::transmute_slice_to_larger_element_type)] + +fn i8_slice_to_i32_slice() { + let i8_slice: &[i8] = &[1i8, 2, 3, 4]; + let i32_slice: &[i32] = unsafe { std::mem::transmute(i8_slice) }; +} + +fn main() {} diff --git a/tests/ui/transmute_slice_to_larger_element_type.stderr b/tests/ui/transmute_slice_to_larger_element_type.stderr new file mode 100644 index 000000000000..51a1841e3988 --- /dev/null +++ b/tests/ui/transmute_slice_to_larger_element_type.stderr @@ -0,0 +1,17 @@ +error: transmute from `&[i8]` to `&[i32]` results in undefined behavior + --> $DIR/transmute_slice_to_larger_element_type.rs:6:38 + | +LL | let i32_slice: &[i32] = unsafe { std::mem::transmute(i8_slice) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this transmute leads out-of-bounds read + = note: `-D clippy::transmute-slice-to-larger-element-type` implied by `-D warnings` +help: try + | +LL | let i32_slice: &[i32] = unsafe { i8_slice.iter().map(|item| unsafe { std::mem::transmute(item) }).collect::>().to_slice() }; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | let i32_slice: &[i32] = unsafe { std::slice::align_to::(i8_slice).1 }; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to previous error + From 9288d7f854f5bf7bea90cc37b98715fa00495a19 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 10 Feb 2023 17:46:34 +0900 Subject: [PATCH 2/7] add documentation --- clippy_lints/src/transmute/mod.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index dd4d743297a2..0676b56130cb 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -441,21 +441,32 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for slice creation which has larger element before transmute. /// /// ### Why is this bad? + /// Creating those slices leads to out-of-bounds read, considered Undefined Behavior. /// /// ### Example /// ```rust - /// // example code where clippy issues a warning + /// let i8_slice: &[i8] = &[1i8, 2, 3, 4]; + // let i32_slice: &[i32] = unsafe { std::mem::transmute(i8_slice) }; /// ``` + /// /// Use instead: + /// + /// ```rust + /// let i32_slice: &[i32] = i8_slice.iter().map(|item| unsafe { std::mem::transmute(item) }).collect::>().to_slice(); + /// ``` + /// + /// or, alternatively: + /// /// ```rust - /// // example code which does not raise clippy warning + /// let i32_slice: &[i32] = std::slice::align_to::(i8_slice).1; /// ``` #[clippy::version = "1.69.0"] pub TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE, correctness, - "default lint description" + "transmute leads out-of-bounds read, which is undefined behavior" } pub struct Transmute { From 43eeb82fcc0e0c2b5149f7ffb75d8406f2fe2d38 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 10 Feb 2023 18:11:51 +0900 Subject: [PATCH 3/7] fixes for align_to issue --- clippy_lints/src/transmute/mod.rs | 6 ++++-- .../src/transmute/transmute_slice_to_larger_element_type.rs | 2 +- tests/ui/transmute_slice_to_larger_element_type.stderr | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index 0676b56130cb..9d55080c6322 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -449,19 +449,21 @@ declare_clippy_lint! { /// ### Example /// ```rust /// let i8_slice: &[i8] = &[1i8, 2, 3, 4]; - // let i32_slice: &[i32] = unsafe { std::mem::transmute(i8_slice) }; + /// let i32_slice: &[i32] = unsafe { std::mem::transmute(i8_slice) }; /// ``` /// /// Use instead: /// /// ```rust + /// let i8_slice: &[i8] = &[1i8, 2, 3, 4]; /// let i32_slice: &[i32] = i8_slice.iter().map(|item| unsafe { std::mem::transmute(item) }).collect::>().to_slice(); /// ``` /// /// or, alternatively: /// /// ```rust - /// let i32_slice: &[i32] = std::slice::align_to::(i8_slice).1; + /// let i8_slice: &[i8] = &[1i8, 2, 3, 4]; + /// let i32_slice: &[i32] = unsafe { i8_slice.align_to::().1 }; /// ``` #[clippy::version = "1.69.0"] pub TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE, diff --git a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs index 7394c35ab535..07efcfef81c2 100644 --- a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs +++ b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs @@ -40,7 +40,7 @@ pub(super) fn check<'tcx>( .to_slice()" ); let sugg_reallocate = Cow::from(sugg_reallocate); - let sugg_align_to = format!("std::slice::align_to::<{ty_elem_to}>({transmute_arg}).1"); + let sugg_align_to = format!("({transmute_arg}).align_to::<{ty_elem_to}>().1"); let sugg_align_to = Cow::from(sugg_align_to); diag.note("this transmute leads out-of-bounds read"); diag.span_suggestions( diff --git a/tests/ui/transmute_slice_to_larger_element_type.stderr b/tests/ui/transmute_slice_to_larger_element_type.stderr index 51a1841e3988..378005ed95fe 100644 --- a/tests/ui/transmute_slice_to_larger_element_type.stderr +++ b/tests/ui/transmute_slice_to_larger_element_type.stderr @@ -10,7 +10,7 @@ help: try | LL | let i32_slice: &[i32] = unsafe { i8_slice.iter().map(|item| unsafe { std::mem::transmute(item) }).collect::>().to_slice() }; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -LL | let i32_slice: &[i32] = unsafe { std::slice::align_to::(i8_slice).1 }; +LL | let i32_slice: &[i32] = unsafe { i8_slice.align_to::().1 }; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error From 222596c971da25bd4ad0a23dd6936a5aefac551a Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 10 Feb 2023 18:18:18 +0900 Subject: [PATCH 4/7] fix diffed stderr --- tests/ui/transmute_slice_to_larger_element_type.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/transmute_slice_to_larger_element_type.stderr b/tests/ui/transmute_slice_to_larger_element_type.stderr index 378005ed95fe..5f3562f0bbe0 100644 --- a/tests/ui/transmute_slice_to_larger_element_type.stderr +++ b/tests/ui/transmute_slice_to_larger_element_type.stderr @@ -8,10 +8,10 @@ LL | let i32_slice: &[i32] = unsafe { std::mem::transmute(i8_slice) }; = note: `-D clippy::transmute-slice-to-larger-element-type` implied by `-D warnings` help: try | +LL | let i32_slice: &[i32] = unsafe { (i8_slice).align_to::().1 }; + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ LL | let i32_slice: &[i32] = unsafe { i8_slice.iter().map(|item| unsafe { std::mem::transmute(item) }).collect::>().to_slice() }; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -LL | let i32_slice: &[i32] = unsafe { i8_slice.align_to::().1 }; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error From 3bf3cdfdf3f5041592d9403b7acf583fad1a7018 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 10 Feb 2023 18:33:09 +0900 Subject: [PATCH 5/7] delete unnecessary TODO --- .../src/transmute/transmute_slice_to_larger_element_type.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs index 07efcfef81c2..a091904ae79a 100644 --- a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs +++ b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs @@ -9,7 +9,6 @@ use rustc_lint::LateContext; use rustc_middle::ty::{self, Ty}; use std::borrow::Cow; -// TODO: Adjust the parameters as necessary pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, call_to_transmute: &'tcx Expr<'_>, From 856a69cd18a7c0ff987a9fe5d983e4cb0ce8be29 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 10 Feb 2023 18:41:53 +0900 Subject: [PATCH 6/7] fix call for slice conversion --- .../src/transmute/transmute_slice_to_larger_element_type.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs index a091904ae79a..6f82f8e5df06 100644 --- a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs +++ b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs @@ -36,7 +36,7 @@ pub(super) fn check<'tcx>( .iter()\ .map(|item| unsafe {{ std::mem::transmute(item) }})\ .collect::>()\ - .to_slice()" + .as_slice()" ); let sugg_reallocate = Cow::from(sugg_reallocate); let sugg_align_to = format!("({transmute_arg}).align_to::<{ty_elem_to}>().1"); From 7c9fe29dc9cd38daa85fc1c206f65f19e2e9cf36 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 10 Feb 2023 18:42:03 +0900 Subject: [PATCH 7/7] add FIXME --- .../src/transmute/transmute_slice_to_larger_element_type.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs index 6f82f8e5df06..951f049febf4 100644 --- a/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs +++ b/clippy_lints/src/transmute/transmute_slice_to_larger_element_type.rs @@ -31,6 +31,7 @@ pub(super) fn check<'tcx>( let transmute_arg = sugg::Sugg::hir(cx, transmute_arg, ".."); // TODO: In this case, outer unsafe block is not needed anymore. It should be removed in // suggestion. + // FIXME: this do not compile, because temporal Vec dropped at end of outer unsafe block. let sugg_reallocate = format!( "{transmute_arg}\ .iter()\