Skip to content

Replace misaligned_transmute lint #2661

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

Merged
merged 2 commits into from
Apr 11, 2018
Merged
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
11 changes: 11 additions & 0 deletions clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
10 changes: 8 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -635,18 +639,19 @@ 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,
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,
Expand Down Expand Up @@ -785,12 +790,12 @@ 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,
transmute::TRANSMUTE_INT_TO_FLOAT,
transmute::TRANSMUTE_PTR_TO_REF,
transmute::TRANSMUTE_PTR_TO_PTR,
transmute::USELESS_TRANSMUTE,
types::BORROWED_BOX,
types::CAST_LOSSLESS,
Expand Down Expand Up @@ -848,6 +853,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,
Expand Down
63 changes: 43 additions & 20 deletions clippy_lints/src/transmute.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -169,21 +168,31 @@ declare_clippy_lint! {
"transmutes from an integer to a float"
}

/// **What it does:** Checks for 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?** This might result in undefined behavior.
/// **Why is this bad?** Transmutes are dangerous, and these can instead be
/// written as casts.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// // u32 is 32-bit aligned; u8 is 8-bit aligned
/// let _: u32 = unsafe { std::mem::transmute([0u8; 4]) };
/// 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 MISALIGNED_TRANSMUTE,
pub TRANSMUTE_PTR_TO_PTR,
complexity,
"transmutes to a potentially less-aligned type"
"transmutes from a pointer to a reference type"
}

pub struct Transmute;
Expand All @@ -193,13 +202,13 @@ impl LintPass for Transmute {
lint_array!(
CROSSPOINTER_TRANSMUTE,
TRANSMUTE_PTR_TO_REF,
TRANSMUTE_PTR_TO_PTR,
USELESS_TRANSMUTE,
WRONG_TRANSMUTE,
TRANSMUTE_INT_TO_CHAR,
TRANSMUTE_BYTES_TO_STR,
TRANSMUTE_INT_TO_BOOL,
TRANSMUTE_INT_TO_FLOAT,
MISALIGNED_TRANSMUTE
)
}
}
Expand All @@ -220,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,
Expand Down Expand Up @@ -363,9 +360,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,
Expand Down
37 changes: 36 additions & 1 deletion clippy_lints/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -871,7 +890,8 @@ impl LintPass for CastPass {
CAST_POSSIBLE_TRUNCATION,
CAST_POSSIBLE_WRAP,
CAST_LOSSLESS,
UNNECESSARY_CAST
UNNECESSARY_CAST,
CAST_PTR_ALIGNMENT
)
}
}
Expand Down Expand Up @@ -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)
);
}
}
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions clippy_lints/src/utils/sugg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,20 @@ impl<'a> Sugg<'a> {
make_unop("*", self)
}

/// Convenience method to create the `&*<expr>` 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 *<expr>` 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 `<lhs>..<rhs>` or `<lhs>...<rhs>`
/// suggestion.
pub fn range(self, end: &Self, limit: ast::RangeLimits) -> Sugg<'static> {
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/cast_alignment.rs
Original file line number Diff line number Diff line change
@@ -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;
}
16 changes: 16 additions & 0 deletions tests/ui/cast_alignment.stderr
Original file line number Diff line number Diff line change
@@ -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

2 changes: 2 additions & 0 deletions tests/ui/deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@

#[warn(unstable_as_mut_slice)]

#[warn(misaligned_transmute)]

fn main() {}
8 changes: 7 additions & 1 deletion tests/ui/deprecated.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

24 changes: 18 additions & 6 deletions tests/ui/transmute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn my_vec() -> MyVec<i32> {
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);
Expand Down Expand Up @@ -140,11 +140,23 @@ 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;
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() { }
30 changes: 24 additions & 6 deletions tests/ui/transmute.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +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
error: transmute from a pointer to a pointer
--> $DIR/transmute.rs:149:29
|
145 | let _: u32 = unsafe { std::mem::transmute([0u8; 4]) }; // err
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
149 | let _: *const f32 = std::mem::transmute(ptr);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr as *const f32`
|
= note: `-D misaligned-transmute` implied by `-D warnings`
= note: `-D transmute-ptr-to-ptr` implied by `-D warnings`

error: aborting due to 33 previous errors
error: transmute from a pointer to a pointer
--> $DIR/transmute.rs:150:27
|
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:152:23
|
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:153:27
|
153 | let _: &mut f32 = std::mem::transmute(&mut 1u32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut *(&mut 1u32 as *mut u32 as *mut f32)`

error: aborting due to 36 previous errors