diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/casts/cast_slice_different_sizes.rs | 117 | ||||
| -rw-r--r-- | clippy_lints/src/casts/mod.rs | 48 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_all.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_correctness.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_utils/src/msrvs.rs | 2 | ||||
| -rw-r--r-- | tests/ui/cast_slice_different_sizes.rs | 39 | ||||
| -rw-r--r-- | tests/ui/cast_slice_different_sizes.stderr | 52 | ||||
| -rw-r--r-- | tests/ui/transmutes_expressible_as_ptr_casts.fixed | 4 | ||||
| -rw-r--r-- | tests/ui/transmutes_expressible_as_ptr_casts.rs | 4 | ||||
| -rw-r--r-- | tests/ui/transmutes_expressible_as_ptr_casts.stderr | 4 |
12 files changed, 267 insertions, 7 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f18a00a9dd..c024dbd9a0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3076,6 +3076,7 @@ Released 2018-09-13 [`cast_ptr_alignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment [`cast_ref_to_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ref_to_mut [`cast_sign_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_sign_loss +[`cast_slice_different_sizes`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_slice_different_sizes [`char_lit_as_u8`]: https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp [`chars_next_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_next_cmp diff --git a/clippy_lints/src/casts/cast_slice_different_sizes.rs b/clippy_lints/src/casts/cast_slice_different_sizes.rs new file mode 100644 index 00000000000..3608c1654d5 --- /dev/null +++ b/clippy_lints/src/casts/cast_slice_different_sizes.rs @@ -0,0 +1,117 @@ +use clippy_utils::{diagnostics::span_lint_and_then, meets_msrv, msrvs, source::snippet_opt}; +use if_chain::if_chain; +use rustc_ast::Mutability; +use rustc_hir::{Expr, ExprKind, Node}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, layout::LayoutOf, Ty, TypeAndMut}; +use rustc_semver::RustcVersion; + +use super::CAST_SLICE_DIFFERENT_SIZES; + +fn is_child_of_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let map = cx.tcx.hir(); + if_chain! { + if let Some(parent_id) = map.find_parent_node(expr.hir_id); + if let Some(parent) = map.find(parent_id); + then { + let expr = match parent { + Node::Block(block) => { + if let Some(parent_expr) = block.expr { + parent_expr + } else { + return false; + } + }, + Node::Expr(expr) => expr, + _ => return false, + }; + + matches!(expr.kind, ExprKind::Cast(..)) + } else { + false + } + } +} + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Option<RustcVersion>) { + // suggestion is invalid if `ptr::slice_from_raw_parts` does not exist + if !meets_msrv(msrv.as_ref(), &msrvs::PTR_SLICE_RAW_PARTS) { + return; + } + + // if this cast is the child of another cast expression then don't emit something for it, the full + // chain will be analyzed + if is_child_of_cast(cx, expr) { + return; + } + + if let Some((from_slice_ty, to_slice_ty)) = expr_cast_chain_tys(cx, expr) { + if let (Ok(from_layout), Ok(to_layout)) = (cx.layout_of(from_slice_ty.ty), cx.layout_of(to_slice_ty.ty)) { + let from_size = from_layout.size.bytes(); + let to_size = to_layout.size.bytes(); + if from_size != to_size && from_size != 0 && to_size != 0 { + span_lint_and_then( + cx, + CAST_SLICE_DIFFERENT_SIZES, + expr.span, + &format!( + "casting between raw pointers to `[{}]` (element size {}) and `[{}]` (element size {}) does not adjust the count", + from_slice_ty, from_size, to_slice_ty, to_size, + ), + |diag| { + let cast_expr = match expr.kind { + ExprKind::Cast(cast_expr, ..) => cast_expr, + _ => unreachable!("expr should be a cast as checked by expr_cast_chain_tys"), + }; + let ptr_snippet = snippet_opt(cx, cast_expr.span).unwrap(); + + let (mutbl_fn_str, mutbl_ptr_str) = match to_slice_ty.mutbl { + Mutability::Mut => ("_mut", "mut"), + Mutability::Not => ("", "const"), + }; + let sugg = format!( + "core::ptr::slice_from_raw_parts{mutbl_fn_str}({ptr_snippet} as *{mutbl_ptr_str} {to_slice_ty}, ..)" + ); + + diag.span_suggestion( + expr.span, + &format!("replace with `ptr::slice_from_raw_parts{mutbl_fn_str}`"), + sugg, + rustc_errors::Applicability::HasPlaceholders, + ); + }, + ); + } + } + } +} + +/// Returns the type T of the pointed to *const [T] or *mut [T] and the mutability of the slice if +/// the type is one of those slices +fn get_raw_slice_ty_mut(ty: Ty<'_>) -> Option<TypeAndMut<'_>> { + match ty.kind() { + ty::RawPtr(TypeAndMut { ty: slice_ty, mutbl }) => match slice_ty.kind() { + ty::Slice(ty) => Some(TypeAndMut { ty: *ty, mutbl: *mutbl }), + _ => None, + }, + _ => None, + } +} + +/// Returns the pair (original ptr T, final ptr U) if the expression is composed of casts +/// Returns None if the expr is not a Cast +fn expr_cast_chain_tys<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<(TypeAndMut<'tcx>, TypeAndMut<'tcx>)> { + if let ExprKind::Cast(cast_expr, _cast_to_hir_ty) = expr.peel_blocks().kind { + let cast_to = cx.typeck_results().expr_ty(expr); + let to_slice_ty = get_raw_slice_ty_mut(cast_to)?; + if let Some((inner_from_ty, _inner_to_ty)) = expr_cast_chain_tys(cx, cast_expr) { + Some((inner_from_ty, to_slice_ty)) + } else { + let cast_from = cx.typeck_results().expr_ty(cast_expr); + let from_slice_ty = get_raw_slice_ty_mut(cast_from)?; + Some((from_slice_ty, to_slice_ty)) + } + } else { + None + } +} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index f2077c569c0..6a0eabd089d 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -5,6 +5,7 @@ mod cast_precision_loss; mod cast_ptr_alignment; mod cast_ref_to_mut; mod cast_sign_loss; +mod cast_slice_different_sizes; mod char_lit_as_u8; mod fn_to_numeric_cast; mod fn_to_numeric_cast_any; @@ -409,6 +410,50 @@ declare_clippy_lint! { "casts from an enum type to an integral type which will truncate the value" } +declare_clippy_lint! { + /// Checks for `as` casts between raw pointers to slices with differently sized elements. + /// + /// ### Why is this bad? + /// The produced raw pointer to a slice does not update its length metadata. The produced + /// pointer will point to a different number of bytes than the original pointer because the + /// length metadata of a raw slice pointer is in elements rather than bytes. + /// Producing a slice reference from the raw pointer will either create a slice with + /// less data (which can be surprising) or create a slice with more data and cause Undefined Behavior. + /// + /// ### Example + /// // Missing data + /// ```rust + /// let a = [1_i32, 2, 3, 4]; + /// let p = &a as *const [i32] as *const [u8]; + /// unsafe { + /// println!("{:?}", &*p); + /// } + /// ``` + /// // Undefined Behavior (note: also potential alignment issues) + /// ```rust + /// let a = [1_u8, 2, 3, 4]; + /// let p = &a as *const [u8] as *const [u32]; + /// unsafe { + /// println!("{:?}", &*p); + /// } + /// ``` + /// Instead use `ptr::slice_from_raw_parts` to construct a slice from a data pointer and the correct length + /// ```rust + /// let a = [1_i32, 2, 3, 4]; + /// let old_ptr = &a as *const [i32]; + /// // The data pointer is cast to a pointer to the target `u8` not `[u8]` + /// // The length comes from the known length of 4 i32s times the 4 bytes per i32 + /// let new_ptr = core::ptr::slice_from_raw_parts(old_ptr as *const u8, 16); + /// unsafe { + /// println!("{:?}", &*new_ptr); + /// } + /// ``` + #[clippy::version = "1.60.0"] + pub CAST_SLICE_DIFFERENT_SIZES, + correctness, + "casting using `as` between raw pointers to slices of types with different sizes" +} + pub struct Casts { msrv: Option<RustcVersion>, } @@ -428,6 +473,7 @@ impl_lint_pass!(Casts => [ CAST_LOSSLESS, CAST_REF_TO_MUT, CAST_PTR_ALIGNMENT, + CAST_SLICE_DIFFERENT_SIZES, UNNECESSARY_CAST, FN_TO_NUMERIC_CAST_ANY, FN_TO_NUMERIC_CAST, @@ -478,6 +524,8 @@ impl<'tcx> LateLintPass<'tcx> for Casts { cast_ref_to_mut::check(cx, expr); cast_ptr_alignment::check(cx, expr); char_lit_as_u8::check(cx, expr); + ptr_as_ptr::check(cx, expr, &self.msrv); + cast_slice_different_sizes::check(cx, expr, &self.msrv); } extract_msrv_attr!(LateContext); diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index f388ae7ce60..f69c4e98719 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -25,6 +25,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(booleans::NONMINIMAL_BOOL), LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(casts::CAST_REF_TO_MUT), + LintId::of(casts::CAST_SLICE_DIFFERENT_SIZES), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::FN_TO_NUMERIC_CAST), LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index d7bf91eb692..df63f84463d 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -13,6 +13,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(bit_mask::INEFFECTIVE_BIT_MASK), LintId::of(booleans::LOGIC_BUG), LintId::of(casts::CAST_REF_TO_MUT), + LintId::of(casts::CAST_SLICE_DIFFERENT_SIZES), LintId::of(copies::IFS_SAME_COND), LintId::of(copies::IF_SAME_THEN_ELSE), LintId::of(derive::DERIVE_HASH_XOR_EQ), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index d248c2a41ee..89629b41525 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -77,6 +77,7 @@ store.register_lints(&[ casts::CAST_PTR_ALIGNMENT, casts::CAST_REF_TO_MUT, casts::CAST_SIGN_LOSS, + casts::CAST_SLICE_DIFFERENT_SIZES, casts::CHAR_LIT_AS_U8, casts::FN_TO_NUMERIC_CAST, casts::FN_TO_NUMERIC_CAST_ANY, diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index a5b409ad96b..fce93153d96 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -20,7 +20,7 @@ msrv_aliases! { 1,46,0 { CONST_IF_MATCH } 1,45,0 { STR_STRIP_PREFIX } 1,43,0 { LOG2_10, LOG10_2 } - 1,42,0 { MATCHES_MACRO, SLICE_PATTERNS } + 1,42,0 { MATCHES_MACRO, SLICE_PATTERNS, PTR_SLICE_RAW_PARTS } 1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE } 1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF } 1,38,0 { POINTER_CAST } diff --git a/tests/ui/cast_slice_different_sizes.rs b/tests/ui/cast_slice_different_sizes.rs new file mode 100644 index 00000000000..cfe1cca2eba --- /dev/null +++ b/tests/ui/cast_slice_different_sizes.rs @@ -0,0 +1,39 @@ +fn main() { + let x: [i32; 3] = [1_i32, 2, 3]; + let r_x = &x; + // Check casting through multiple bindings + // Because it's separate, it does not check the cast back to something of the same size + let a = r_x as *const [i32]; + let b = a as *const [u8]; + let c = b as *const [u32]; + + // loses data + let loss = r_x as *const [i32] as *const [u8]; + + // Cast back to same size but different type loses no data, just type conversion + // This is weird code but there's no reason for this lint specifically to fire *twice* on it + let restore = r_x as *const [i32] as *const [u8] as *const [u32]; + + // Check casting through blocks is detected + let loss_block_1 = { r_x as *const [i32] } as *const [u8]; + let loss_block_2 = { + let _ = (); + r_x as *const [i32] + } as *const [u8]; + + // Check that resores of the same size are detected through blocks + let restore_block_1 = { r_x as *const [i32] } as *const [u8] as *const [u32]; + let restore_block_2 = { ({ r_x as *const [i32] }) as *const [u8] } as *const [u32]; + let restore_block_3 = { + let _ = (); + ({ + let _ = (); + r_x as *const [i32] + }) as *const [u8] + } as *const [u32]; + + // Check that the result of a long chain of casts is detected + let long_chain_loss = r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const [u8]; + let long_chain_restore = + r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const [u8] as *const [u32]; +} diff --git a/tests/ui/cast_slice_different_sizes.stderr b/tests/ui/cast_slice_different_sizes.stderr new file mode 100644 index 00000000000..a37cec7cb3b --- /dev/null +++ b/tests/ui/cast_slice_different_sizes.stderr @@ -0,0 +1,52 @@ +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:7:13 + | +LL | let b = a as *const [u8]; + | ^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(a as *const u8, ..)` + | + = note: `#[deny(clippy::cast_slice_different_sizes)]` on by default + +error: casting between raw pointers to `[u8]` (element size 1) and `[u32]` (element size 4) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:8:13 + | +LL | let c = b as *const [u32]; + | ^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(b as *const u32, ..)` + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:11:16 + | +LL | let loss = r_x as *const [i32] as *const [u8]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(r_x as *const [i32] as *const u8, ..)` + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:18:24 + | +LL | let loss_block_1 = { r_x as *const [i32] } as *const [u8]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts({ r_x as *const [i32] } as *const u8, ..)` + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:19:24 + | +LL | let loss_block_2 = { + | ________________________^ +LL | | let _ = (); +LL | | r_x as *const [i32] +LL | | } as *const [u8]; + | |____________________^ + | +help: replace with `ptr::slice_from_raw_parts` + | +LL ~ let loss_block_2 = core::ptr::slice_from_raw_parts({ +LL + let _ = (); +LL + r_x as *const [i32] +LL ~ } as *const u8, ..); + | + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:36:27 + | +LL | let long_chain_loss = r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const [u8]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const u8, ..)` + +error: aborting due to 6 previous errors + diff --git a/tests/ui/transmutes_expressible_as_ptr_casts.fixed b/tests/ui/transmutes_expressible_as_ptr_casts.fixed index 9ae0ed0b13f..b425cdd6cbf 100644 --- a/tests/ui/transmutes_expressible_as_ptr_casts.fixed +++ b/tests/ui/transmutes_expressible_as_ptr_casts.fixed @@ -25,8 +25,8 @@ fn main() { let slice_ptr = &[0, 1, 2, 3] as *const [i32]; // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast - let _ptr_to_unsized_transmute = unsafe { slice_ptr as *const [u16] }; - let _ptr_to_unsized = slice_ptr as *const [u16]; + let _ptr_to_unsized_transmute = unsafe { slice_ptr as *const [u32] }; + let _ptr_to_unsized = slice_ptr as *const [u32]; // TODO: We could try testing vtable casts here too, but maybe // we should wait until std::raw::TraitObject is stabilized? diff --git a/tests/ui/transmutes_expressible_as_ptr_casts.rs b/tests/ui/transmutes_expressible_as_ptr_casts.rs index 985cf9a075d..8fd57c59652 100644 --- a/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -25,8 +25,8 @@ fn main() { let slice_ptr = &[0, 1, 2, 3] as *const [i32]; // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast - let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u16]>(slice_ptr) }; - let _ptr_to_unsized = slice_ptr as *const [u16]; + let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u32]>(slice_ptr) }; + let _ptr_to_unsized = slice_ptr as *const [u32]; // TODO: We could try testing vtable casts here too, but maybe // we should wait until std::raw::TraitObject is stabilized? diff --git a/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/tests/ui/transmutes_expressible_as_ptr_casts.stderr index e8496a325d6..d9b64a0ed7b 100644 --- a/tests/ui/transmutes_expressible_as_ptr_casts.stderr +++ b/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -17,8 +17,8 @@ LL | let _ptr_i8_transmute = unsafe { transmute::<*const i32, *const i8>(ptr error: transmute from a pointer to a pointer --> $DIR/transmutes_expressible_as_ptr_casts.rs:28:46 | -LL | let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u16]>(slice_ptr) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u16]` +LL | let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u32]>(slice_ptr) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u32]` error: transmute from `*const i32` to `usize` which could be expressed as a pointer cast instead --> $DIR/transmutes_expressible_as_ptr_casts.rs:34:50 |