Skip to content

Commit

Permalink
lint for casting raw pointers to slices with different element sizes
Browse files Browse the repository at this point in the history
  • Loading branch information
asquared31415 authored and J-ZhengLi committed Mar 21, 2022
1 parent 271eaa4 commit b64ecc0
Show file tree
Hide file tree
Showing 12 changed files with 267 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3077,6 +3077,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
Expand Down
117 changes: 117 additions & 0 deletions clippy_lints/src/casts/cast_slice_different_sizes.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
48 changes: 48 additions & 0 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
}
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_correctness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,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,
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/cast_slice_different_sizes.rs
Original file line number Diff line number Diff line change
@@ -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];
}
52 changes: 52 additions & 0 deletions tests/ui/cast_slice_different_sizes.stderr
Original file line number Diff line number Diff line change
@@ -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

4 changes: 2 additions & 2 deletions tests/ui/transmutes_expressible_as_ptr_casts.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/transmutes_expressible_as_ptr_casts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/transmutes_expressible_as_ptr_casts.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b64ecc0

Please sign in to comment.