lint for casting raw pointers to slices with different element sizes

asquared31415 · asquared31415 · commit e2462858ead1 · 2022-02-18T05:17:07.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3075,6 +3075,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
@@ -0,0 +1,119 @@
+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 {}) to `[{}]` (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,
+                        );
+
+                        diag.help("the count in elements of raw pointers to slices is preserved across `as`-casts so the slice covers a different number of bytes");
+                    },
+                );
+            }
+        }
+    }
+}
+
+/// 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, 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
@@ -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;
@@ -390,6 +391,51 @@ declare_clippy_lint! {
     "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `as` casts between raw pointers to slices with elements with different sizes.
+    ///
+    /// ### 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>,
 }
@@ -409,6 +455,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,
@@ -456,6 +503,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
         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
@@ -22,6 +22,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(booleans::LOGIC_BUG),
     LintId::of(booleans::NONMINIMAL_BOOL),
     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
@@ -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
@@ -74,6 +74,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
@@ -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
@@ -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
@@ -0,0 +1,62 @@
+error: casting between raw pointers to `[i32]` (element size 4) to `[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
+   = help: the count in elements of raw pointers to slices is preserved across `as`-casts so the slice covers a different number of bytes
+
+error: casting between raw pointers to `[u8]` (element size 1) to `[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, ..)`
+   |
+   = help: the count in elements of raw pointers to slices is preserved across `as`-casts so the slice covers a different number of bytes
+
+error: casting between raw pointers to `[i32]` (element size 4) to `[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, ..)`
+   |
+   = help: the count in elements of raw pointers to slices is preserved across `as`-casts so the slice covers a different number of bytes
+
+error: casting between raw pointers to `[i32]` (element size 4) to `[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, ..)`
+   |
+   = help: the count in elements of raw pointers to slices is preserved across `as`-casts so the slice covers a different number of bytes
+
+error: casting between raw pointers to `[i32]` (element size 4) to `[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: the count in elements of raw pointers to slices is preserved across `as`-casts so the slice covers a different number of bytes
+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) to `[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, ..)`
+   |
+   = help: the count in elements of raw pointers to slices is preserved across `as`-casts so the slice covers a different number of bytes
+
+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
@@ -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
@@ -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
@@ -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
