Skip to content
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

Add transmute_slice_to_larger_element_type lint #10312

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 35 additions & 0 deletions clippy_lints/src/transmute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -438,6 +439,38 @@ 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 i8_slice: &[i8] = &[1i8, 2, 3, 4];
/// let i32_slice: &[i32] = i8_slice.iter().map(|item| unsafe { std::mem::transmute(item) }).collect::<Vec<_>>().to_slice();
Copy link
Contributor

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 &i8 to i32.

Also, isn't the vector you create dropped at the end of the statement? (and don't you mean .as_slice()?)

Copy link
Contributor

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 Vec being dropped, and the transmute on each element is wrong as well. A "proper" suggestion might be to go through core::slice::from_raw_parts, but also because of alignment requirements, I don't think it can be an automated suggestion. Perhaps "consider core::slice::from_raw_parts(_mut) instead" would suffice?

/// ```
///
/// or, alternatively:
///
/// ```rust
/// let i8_slice: &[i8] = &[1i8, 2, 3, 4];
/// let i32_slice: &[i32] = unsafe { i8_slice.align_to::<i32>().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,
}
Expand All @@ -458,6 +491,7 @@ impl_lint_pass!(Transmute => [
TRANSMUTE_UNDEFINED_REPR,
TRANSMUTING_NULL,
TRANSMUTE_NULL_TO_FN,
TRANSMUTE_SLICE_TO_LARGER_ELEMENT_TYPE,
]);
impl Transmute {
#[must_use]
Expand Down Expand Up @@ -503,6 +537,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)
Expand Down
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;

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.
// FIXME: this do not compile, because temporal Vec dropped at end of outer unsafe block.
let sugg_reallocate = format!(
"{transmute_arg}\
.iter()\
.map(|item| unsafe {{ std::mem::transmute(item) }})\
.collect::<Vec<_>>()\
.as_slice()"
);
let sugg_reallocate = Cow::from(sugg_reallocate);
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(
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use if_chain! to avoid this cascade fo else { false } (or if let … && let … && … since feature let_chains is enabled).

}
}
9 changes: 9 additions & 0 deletions tests/ui/transmute_slice_to_larger_element_type.rs
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() {}
17 changes: 17 additions & 0 deletions tests/ui/transmute_slice_to_larger_element_type.stderr
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).align_to::<i32>().1 };
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | let i32_slice: &[i32] = unsafe { i8_slice.iter().map(|item| unsafe { std::mem::transmute(item) }).collect::<Vec<_>>().to_slice() };
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to previous error