From c6bc6823258b3ce1a675f9ed4e80ff3268c82e97 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Wed, 11 Apr 2018 02:17:59 -0700 Subject: [PATCH 1/2] Fix misaligned_transmute lint This is done by adding two new lints: cast_ptr_alignment and transmute_ptr_to_ptr. These will replace misaligned_transmute. --- clippy_lints/src/lib.rs | 4 +++ clippy_lints/src/transmute.rs | 54 ++++++++++++++++++++++++++++++++++ clippy_lints/src/types.rs | 37 ++++++++++++++++++++++- clippy_lints/src/utils/sugg.rs | 14 +++++++++ tests/ui/cast_alignment.rs | 19 ++++++++++++ tests/ui/cast_alignment.stderr | 16 ++++++++++ tests/ui/transmute.rs | 21 ++++++++++++- tests/ui/transmute.stderr | 28 +++++++++++++++++- 8 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 tests/ui/cast_alignment.rs create mode 100644 tests/ui/cast_alignment.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 187b1fcee82e..a9ebd4dfd3ee 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -641,12 +641,14 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::TRANSMUTE_INT_TO_CHAR, transmute::TRANSMUTE_INT_TO_FLOAT, transmute::TRANSMUTE_PTR_TO_REF, + transmute::TRANSMUTE_PTR_TO_PTR, transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::BORROWED_BOX, types::BOX_VEC, types::CAST_LOSSLESS, + types::CAST_PTR_ALIGNMENT, types::CHAR_LIT_AS_U8, types::IMPLICIT_HASHER, types::LET_UNIT_VALUE, @@ -791,6 +793,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::TRANSMUTE_INT_TO_CHAR, transmute::TRANSMUTE_INT_TO_FLOAT, transmute::TRANSMUTE_PTR_TO_REF, + transmute::TRANSMUTE_PTR_TO_PTR, transmute::USELESS_TRANSMUTE, types::BORROWED_BOX, types::CAST_LOSSLESS, @@ -848,6 +851,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { swap::ALMOST_SWAPPED, transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, + types::CAST_PTR_ALIGNMENT, types::UNIT_CMP, unicode::ZERO_WIDTH_SPACE, unused_io_amount::UNUSED_IO_AMOUNT, diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index 00d6c310c697..7b591a583282 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -186,6 +186,33 @@ declare_clippy_lint! { "transmutes to a potentially less-aligned type" } +/// **What it does:** Checks for transmutes from a pointer to a pointer, or +/// from a reference to a reference. +/// +/// **Why is this bad?** Transmutes are dangerous, and these can instead be +/// written as casts. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// let ptr = &1u32 as *const u32; +/// unsafe { +/// // pointer-to-pointer transmute +/// let _: *const f32 = std::mem::transmute(ptr); +/// // ref-ref transmute +/// let _: &f32 = std::mem::transmute(&1u32); +/// } +/// // These can be respectively written: +/// let _ = ptr as *const f32 +/// let _ = unsafe{ &*(&1u32 as *const u32 as *const f32) }; +/// ``` +declare_clippy_lint! { + pub TRANSMUTE_PTR_TO_PTR, + complexity, + "transmutes from a pointer to a reference type" +} + pub struct Transmute; impl LintPass for Transmute { @@ -193,6 +220,7 @@ impl LintPass for Transmute { lint_array!( CROSSPOINTER_TRANSMUTE, TRANSMUTE_PTR_TO_REF, + TRANSMUTE_PTR_TO_PTR, USELESS_TRANSMUTE, WRONG_TRANSMUTE, TRANSMUTE_INT_TO_CHAR, @@ -363,9 +391,35 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute { ); } ) + } else { + span_lint_and_then( + cx, + TRANSMUTE_PTR_TO_PTR, + e.span, + "transmute from a reference to a reference", + |db| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + let sugg_paren = arg.as_ty(cx.tcx.mk_ptr(*ref_from)).as_ty(cx.tcx.mk_ptr(*ref_to)); + let sugg = if ref_to.mutbl == Mutability::MutMutable { + sugg_paren.mut_addr_deref() + } else { + sugg_paren.addr_deref() + }; + db.span_suggestion(e.span, "try", sugg.to_string()); + }, + ) } } }, + (&ty::TyRawPtr(_), &ty::TyRawPtr(to_ty)) => span_lint_and_then( + cx, + TRANSMUTE_PTR_TO_PTR, + e.span, + "transmute from a pointer to a pointer", + |db| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + let sugg = arg.as_ty(cx.tcx.mk_ptr(to_ty)); + db.span_suggestion(e.span, "try", sugg.to_string()); + }, + ), (&ty::TyInt(ast::IntTy::I8), &ty::TyBool) | (&ty::TyUint(ast::UintTy::U8), &ty::TyBool) => { span_lint_and_then( cx, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 16a1142efb7a..9034badd5c5d 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -679,6 +679,25 @@ declare_clippy_lint! { "cast to the same type, e.g. `x as i32` where `x: i32`" } +/// **What it does:** Checks for casts from a more-strictly-aligned pointer to a +/// less-strictly-aligned pointer +/// +/// **Why is this bad?** Dereferencing the resulting pointer is undefined +/// behavior. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// let _ = (&1u8 as *const u8) as *const u16; +/// let _ = (&mut 1u8 as *mut u8) as *mut u16; +/// ``` +declare_clippy_lint! { + pub CAST_PTR_ALIGNMENT, + correctness, + "cast from a pointer to a less-strictly-aligned pointer" +} + /// Returns the size in bits of an integral type. /// Will return 0 if the type is not an int or uint variant fn int_ty_to_nbits(typ: Ty, tcx: TyCtxt) -> u64 { @@ -871,7 +890,8 @@ impl LintPass for CastPass { CAST_POSSIBLE_TRUNCATION, CAST_POSSIBLE_WRAP, CAST_LOSSLESS, - UNNECESSARY_CAST + UNNECESSARY_CAST, + CAST_PTR_ALIGNMENT ) } } @@ -955,6 +975,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { }, } } + if_chain!{ + if let ty::TyRawPtr(from_ptr_ty) = &cast_from.sty; + if let ty::TyRawPtr(to_ptr_ty) = &cast_to.sty; + if let Some(from_align) = cx.layout_of(from_ptr_ty.ty).ok().map(|a| a.align.abi()); + if let Some(to_align) = cx.layout_of(to_ptr_ty.ty).ok().map(|a| a.align.abi()); + if from_align < to_align; + then { + span_lint( + cx, + CAST_PTR_ALIGNMENT, + expr.span, + &format!("casting from `{}` to a less-strictly-aligned pointer (`{}`)", cast_from, cast_to) + ); + } + } } } } diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 4ca6e5cbf730..bd48774a5ca9 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -161,6 +161,20 @@ impl<'a> Sugg<'a> { make_unop("*", self) } + /// Convenience method to create the `&*` suggestion. Currently this + /// is needed because `sugg.deref().addr()` produces an unnecessary set of + /// parentheses around the deref. + pub fn addr_deref(self) -> Sugg<'static> { + make_unop("&*", self) + } + + /// Convenience method to create the `&mut *` suggestion. Currently + /// this is needed because `sugg.deref().mut_addr()` produces an unnecessary + /// set of parentheses around the deref. + pub fn mut_addr_deref(self) -> Sugg<'static> { + make_unop("&mut *", self) + } + /// Convenience method to create the `..` or `...` /// suggestion. pub fn range(self, end: &Self, limit: ast::RangeLimits) -> Sugg<'static> { diff --git a/tests/ui/cast_alignment.rs b/tests/ui/cast_alignment.rs new file mode 100644 index 000000000000..4985a90bdf56 --- /dev/null +++ b/tests/ui/cast_alignment.rs @@ -0,0 +1,19 @@ +//! Test casts for alignment issues + +#[warn(cast_ptr_alignment)] +#[allow(no_effect, unnecessary_operation, cast_lossless)] +fn main() { + /* These should be warned against */ + + // cast to more-strictly-aligned type + (&1u8 as *const u8) as *const u16; + (&mut 1u8 as *mut u8) as *mut u16; + + /* These should be okay */ + + // not a pointer type + 1u8 as u16; + // cast to less-strictly-aligned type + (&1u16 as *const u16) as *const u8; + (&mut 1u16 as *mut u16) as *mut u8; +} diff --git a/tests/ui/cast_alignment.stderr b/tests/ui/cast_alignment.stderr new file mode 100644 index 000000000000..d4fdb5becf9e --- /dev/null +++ b/tests/ui/cast_alignment.stderr @@ -0,0 +1,16 @@ +error: casting from `*const u8` to a less-strictly-aligned pointer (`*const u16`) + --> $DIR/cast_alignment.rs:9:5 + | +9 | (&1u8 as *const u8) as *const u16; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D cast-ptr-alignment` implied by `-D warnings` + +error: casting from `*mut u8` to a less-strictly-aligned pointer (`*mut u16`) + --> $DIR/cast_alignment.rs:10:5 + | +10 | (&mut 1u8 as *mut u8) as *mut u16; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/transmute.rs b/tests/ui/transmute.rs index 48b89c33ab91..3ff444b38656 100644 --- a/tests/ui/transmute.rs +++ b/tests/ui/transmute.rs @@ -16,7 +16,7 @@ fn my_vec() -> MyVec { vec![] } -#[allow(needless_lifetimes)] +#[allow(needless_lifetimes, transmute_ptr_to_ptr)] #[warn(useless_transmute)] unsafe fn _generic<'a, T, U: 'a>(t: &'a T) { let _: &'a T = core::intrinsics::transmute(t); @@ -147,4 +147,23 @@ fn misaligned_transmute() { let _: [u8; 4] = unsafe { std::mem::transmute(0u32) }; // ok (alignment-wise) } +#[warn(transmute_ptr_to_ptr)] +fn transmute_ptr_to_ptr() { + let ptr = &1u32 as *const u32; + let mut_ptr = &mut 1u32 as *mut u32; + unsafe { + // pointer-to-pointer transmutes; bad + let _: *const f32 = std::mem::transmute(ptr); + let _: *mut f32 = std::mem::transmute(mut_ptr); + // ref-ref transmutes; bad + let _: &f32 = std::mem::transmute(&1u32); + let _: &mut f32 = std::mem::transmute(&mut 1u32); + } + // These should be fine + let _ = ptr as *const f32; + let _ = mut_ptr as *mut f32; + let _ = unsafe { &*(&1u32 as *const u32 as *const f32) }; + let _ = unsafe { &mut *(&mut 1u32 as *mut u32 as *mut f32) }; +} + fn main() { } diff --git a/tests/ui/transmute.stderr b/tests/ui/transmute.stderr index 74bbf95d5258..f7090f0dca38 100644 --- a/tests/ui/transmute.stderr +++ b/tests/ui/transmute.stderr @@ -212,5 +212,31 @@ error: transmute from `[u8; 4]` to a less-aligned type (`u32`) | = note: `-D misaligned-transmute` implied by `-D warnings` -error: aborting due to 33 previous errors +error: transmute from a pointer to a pointer + --> $DIR/transmute.rs:156:29 + | +156 | let _: *const f32 = std::mem::transmute(ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr as *const f32` + | + = note: `-D transmute-ptr-to-ptr` implied by `-D warnings` + +error: transmute from a pointer to a pointer + --> $DIR/transmute.rs:157:27 + | +157 | let _: *mut f32 = std::mem::transmute(mut_ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `mut_ptr as *mut f32` + +error: transmute from a reference to a reference + --> $DIR/transmute.rs:159:23 + | +159 | let _: &f32 = std::mem::transmute(&1u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(&1u32 as *const u32 as *const f32)` + +error: transmute from a reference to a reference + --> $DIR/transmute.rs:160:27 + | +160 | let _: &mut f32 = std::mem::transmute(&mut 1u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut *(&mut 1u32 as *mut u32 as *mut f32)` + +error: aborting due to 37 previous errors From b77d74030b193c5d188d2191ece074b0898a3996 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Wed, 11 Apr 2018 02:50:04 -0700 Subject: [PATCH 2/2] Deprecate misaligned_transmute --- clippy_lints/src/deprecated_lints.rs | 11 ++++++++++ clippy_lints/src/lib.rs | 6 ++++-- clippy_lints/src/transmute.rs | 31 ---------------------------- tests/ui/deprecated.rs | 2 ++ tests/ui/deprecated.stderr | 8 ++++++- tests/ui/transmute.rs | 7 ------- tests/ui/transmute.stderr | 26 ++++++++--------------- 7 files changed, 33 insertions(+), 58 deletions(-) diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs index e51d7cc6d383..1edeb30560ce 100644 --- a/clippy_lints/src/deprecated_lints.rs +++ b/clippy_lints/src/deprecated_lints.rs @@ -71,3 +71,14 @@ declare_deprecated_lint! { pub STRING_TO_STRING, "using `string::to_string` is common even today and specialization will likely happen soon" } + +/// **What it does:** Nothing. This lint has been deprecated. +/// +/// **Deprecation reason:** This lint should never have applied to non-pointer types, as transmuting +/// between non-pointer types of differing alignment is well-defined behavior (it's semantically +/// equivalent to a memcpy). This lint has thus been refactored into two separate lints: +/// cast_ptr_alignment and transmute_ptr_to_ptr. +declare_deprecated_lint! { + pub MISALIGNED_TRANSMUTE, + "this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr" +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a9ebd4dfd3ee..190d65be86e9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -277,6 +277,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { "string_to_string", "using `string::to_string` is common even today and specialization will likely happen soon", ); + store.register_removed( + "misaligned_transmute", + "this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr", + ); // end deprecated lints, do not remove this comment, it’s used in `update_lints` reg.register_late_lint_pass(box serde_api::Serde); @@ -635,7 +639,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { swap::MANUAL_SWAP, temporary_assignment::TEMPORARY_ASSIGNMENT, transmute::CROSSPOINTER_TRANSMUTE, - transmute::MISALIGNED_TRANSMUTE, transmute::TRANSMUTE_BYTES_TO_STR, transmute::TRANSMUTE_INT_TO_BOOL, transmute::TRANSMUTE_INT_TO_CHAR, @@ -787,7 +790,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { swap::MANUAL_SWAP, temporary_assignment::TEMPORARY_ASSIGNMENT, transmute::CROSSPOINTER_TRANSMUTE, - transmute::MISALIGNED_TRANSMUTE, transmute::TRANSMUTE_BYTES_TO_STR, transmute::TRANSMUTE_INT_TO_BOOL, transmute::TRANSMUTE_INT_TO_CHAR, diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index 7b591a583282..68321e7bd5e3 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -1,7 +1,6 @@ use rustc::lint::*; use rustc::ty::{self, Ty}; use rustc::hir::*; -use rustc::ty::layout::LayoutOf; use std::borrow::Cow; use syntax::ast; use utils::{last_path_segment, match_def_path, paths, snippet, span_lint, span_lint_and_then}; @@ -169,23 +168,6 @@ declare_clippy_lint! { "transmutes from an integer to a float" } -/// **What it does:** Checks for transmutes to a potentially less-aligned type. -/// -/// **Why is this bad?** This might result in undefined behavior. -/// -/// **Known problems:** None. -/// -/// **Example:** -/// ```rust -/// // u32 is 32-bit aligned; u8 is 8-bit aligned -/// let _: u32 = unsafe { std::mem::transmute([0u8; 4]) }; -/// ``` -declare_clippy_lint! { - pub MISALIGNED_TRANSMUTE, - complexity, - "transmutes to a potentially less-aligned type" -} - /// **What it does:** Checks for transmutes from a pointer to a pointer, or /// from a reference to a reference. /// @@ -227,7 +209,6 @@ impl LintPass for Transmute { TRANSMUTE_BYTES_TO_STR, TRANSMUTE_INT_TO_BOOL, TRANSMUTE_INT_TO_FLOAT, - MISALIGNED_TRANSMUTE ) } } @@ -248,18 +229,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute { e.span, &format!("transmute from a type (`{}`) to itself", from_ty), ), - _ if cx.layout_of(from_ty).ok().map(|a| a.align.abi()) - < cx.layout_of(to_ty).ok().map(|a| a.align.abi()) - => span_lint( - cx, - MISALIGNED_TRANSMUTE, - e.span, - &format!( - "transmute from `{}` to a less-aligned type (`{}`)", - from_ty, - to_ty, - ) - ), (&ty::TyRef(_, rty), &ty::TyRawPtr(ptr_ty)) => span_lint_and_then( cx, USELESS_TRANSMUTE, diff --git a/tests/ui/deprecated.rs b/tests/ui/deprecated.rs index 0598e174e50a..f456c4172237 100644 --- a/tests/ui/deprecated.rs +++ b/tests/ui/deprecated.rs @@ -9,4 +9,6 @@ #[warn(unstable_as_mut_slice)] +#[warn(misaligned_transmute)] + fn main() {} diff --git a/tests/ui/deprecated.stderr b/tests/ui/deprecated.stderr index 7d5d594cfa1c..aa62ccbd0e52 100644 --- a/tests/ui/deprecated.stderr +++ b/tests/ui/deprecated.stderr @@ -24,5 +24,11 @@ error: lint unstable_as_mut_slice has been removed: `Vec::as_mut_slice` has been 10 | #[warn(unstable_as_mut_slice)] | ^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: lint misaligned_transmute has been removed: this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr + --> $DIR/deprecated.rs:12:8 + | +12 | #[warn(misaligned_transmute)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors diff --git a/tests/ui/transmute.rs b/tests/ui/transmute.rs index 3ff444b38656..7c5e3f03d13b 100644 --- a/tests/ui/transmute.rs +++ b/tests/ui/transmute.rs @@ -140,13 +140,6 @@ fn bytes_to_str(b: &[u8], mb: &mut [u8]) { let _: &mut str = unsafe { std::mem::transmute(mb) }; } -#[warn(misaligned_transmute)] -fn misaligned_transmute() { - let _: u32 = unsafe { std::mem::transmute([0u8; 4]) }; // err - let _: u32 = unsafe { std::mem::transmute(0f32) }; // ok (alignment-wise) - let _: [u8; 4] = unsafe { std::mem::transmute(0u32) }; // ok (alignment-wise) -} - #[warn(transmute_ptr_to_ptr)] fn transmute_ptr_to_ptr() { let ptr = &1u32 as *const u32; diff --git a/tests/ui/transmute.stderr b/tests/ui/transmute.stderr index f7090f0dca38..a343a8a9cb5b 100644 --- a/tests/ui/transmute.stderr +++ b/tests/ui/transmute.stderr @@ -204,39 +204,31 @@ error: transmute from a `&mut [u8]` to a `&mut str` 140 | let _: &mut str = unsafe { std::mem::transmute(mb) }; | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::str::from_utf8_mut(mb).unwrap()` -error: transmute from `[u8; 4]` to a less-aligned type (`u32`) - --> $DIR/transmute.rs:145:27 - | -145 | let _: u32 = unsafe { std::mem::transmute([0u8; 4]) }; // err - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `-D misaligned-transmute` implied by `-D warnings` - error: transmute from a pointer to a pointer - --> $DIR/transmute.rs:156:29 + --> $DIR/transmute.rs:149:29 | -156 | let _: *const f32 = std::mem::transmute(ptr); +149 | let _: *const f32 = std::mem::transmute(ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr as *const f32` | = note: `-D transmute-ptr-to-ptr` implied by `-D warnings` error: transmute from a pointer to a pointer - --> $DIR/transmute.rs:157:27 + --> $DIR/transmute.rs:150:27 | -157 | let _: *mut f32 = std::mem::transmute(mut_ptr); +150 | let _: *mut f32 = std::mem::transmute(mut_ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `mut_ptr as *mut f32` error: transmute from a reference to a reference - --> $DIR/transmute.rs:159:23 + --> $DIR/transmute.rs:152:23 | -159 | let _: &f32 = std::mem::transmute(&1u32); +152 | let _: &f32 = std::mem::transmute(&1u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(&1u32 as *const u32 as *const f32)` error: transmute from a reference to a reference - --> $DIR/transmute.rs:160:27 + --> $DIR/transmute.rs:153:27 | -160 | let _: &mut f32 = std::mem::transmute(&mut 1u32); +153 | let _: &mut f32 = std::mem::transmute(&mut 1u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut *(&mut 1u32 as *mut u32 as *mut f32)` -error: aborting due to 37 previous errors +error: aborting due to 36 previous errors