-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add transmute_slice_to_larger_element_type lint
#10312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
2d3b145
9288d7f
43eeb82
222596c
3bf3cdf
856a69c
7c9fe29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,36 @@ declare_clippy_lint! { | |
| "transmute results in a null function pointer, which is undefined behavior" | ||
| } | ||
|
|
||
| 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 | ||
| /// 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::<Vec<_>>().to_slice(); | ||
| /// ``` | ||
| /// | ||
| /// or, alternatively: | ||
| /// | ||
| /// ```rust | ||
| /// let i32_slice: &[i32] = std::slice::align_to::<i32>(i8_slice).1; | ||
|
||
| /// ``` | ||
| #[clippy::version = "1.69.0"] | ||
| pub TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE, | ||
| correctness, | ||
| "transmute leads out-of-bounds read, which is undefined behavior" | ||
| } | ||
|
|
||
| pub struct Transmute { | ||
| msrv: Msrv, | ||
| } | ||
|
|
@@ -458,6 +489,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 +535,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) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
KisaragiEffective marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 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::<Vec<_>>()\ | ||
| .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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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::<Vec<_>>().to_slice() }; | ||
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| LL | let i32_slice: &[i32] = unsafe { std::slice::align_to::<i32>(i8_slice).1 }; | ||
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| error: aborting due to previous error | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried this suggestion? I don't think this will do what you want: this attempts to transmute
&i8toi32.Also, isn't the vector you create dropped at the end of the statement? (and don't you mean
.as_slice()?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this code does not compile because of the temporary
Vecbeing dropped, and thetransmuteon each element is wrong as well. A "proper" suggestion might be to go throughcore::slice::from_raw_parts, but also because of alignment requirements, I don't think it can be an automated suggestion. Perhaps "considercore::slice::from_raw_parts(_mut)instead" would suffice?