diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d7bda27e4fc..cb5a4d4a5778 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3903,6 +3903,7 @@ Released 2018-09-13 [`format_push_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#format_push_string [`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect [`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into +[`from_raw_with_void_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_raw_with_void_ptr [`from_str_radix_10`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_str_radix_10 [`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send [`get_first`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_first diff --git a/clippy_lints/src/from_raw_with_void_ptr.rs b/clippy_lints/src/from_raw_with_void_ptr.rs new file mode 100644 index 000000000000..63c3c5f601df --- /dev/null +++ b/clippy_lints/src/from_raw_with_void_ptr.rs @@ -0,0 +1,57 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::path_def_id; +use clippy_utils::ty::is_c_void; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::RawPtr; +use rustc_middle::ty::TypeAndMut; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Checks if we're passing a `c_void` raw pointer to `Box::from_raw(_)` + /// + /// ### Why is this bad? + /// However, it is easy to run into the pitfall of calling from_raw with the c_void pointer. + /// Note that the definition of, say, Box::from_raw is: + /// + /// `pub unsafe fn from_raw(raw: *mut T) -> Box` + /// + /// meaning that if you pass a *mut c_void you will get a Box. + /// Per the safety requirements in the documentation, for this to be safe, + /// c_void would need to have the same memory layout as the original type, which is often not the case. + /// + /// ### Example + /// ```rust + /// # use std::ffi::c_void; + /// let ptr = Box::into_raw(Box::new(42usize)) as *mut c_void; + /// let _ = unsafe { Box::from_raw(ptr) }; + /// ``` + /// Use instead: + /// ```rust + /// # use std::ffi::c_void; + /// # let ptr = Box::into_raw(Box::new(42usize)) as *mut c_void; + /// let _ = unsafe { Box::from_raw(ptr as *mut usize) }; + /// ``` + /// + #[clippy::version = "1.66.0"] + pub FROM_RAW_WITH_VOID_PTR, + suspicious, + "creating a `Box` from a raw void pointer" +} +declare_lint_pass!(FromRawWithVoidPtr => [FROM_RAW_WITH_VOID_PTR]); + +impl LateLintPass<'_> for FromRawWithVoidPtr { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if let ExprKind::Call(box_from_raw, [arg]) = expr.kind + && let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_from_raw.kind + && seg.ident.name == sym!(from_raw) + // FIXME: This lint is also applicable to other types, like `Rc`, `Arc` and `Weak`. + && path_def_id(cx, ty).map_or(false, |id| Some(id) == cx.tcx.lang_items().owned_box()) + && let arg_kind = cx.typeck_results().expr_ty(arg).kind() + && let RawPtr(TypeAndMut { ty, .. }) = arg_kind + && is_c_void(cx, *ty) { + span_lint_and_help(cx, FROM_RAW_WITH_VOID_PTR, expr.span, "creating a `Box` from a raw void pointer", Some(arg.span), "cast this to a pointer of the actual type"); + } + } +} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index f5ad52ba1892..34fded26cfcf 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -81,6 +81,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING), LintId::of(formatting::SUSPICIOUS_UNARY_OP_FORMATTING), LintId::of(from_over_into::FROM_OVER_INTO), + LintId::of(from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR), LintId::of(from_str_radix_10::FROM_STR_RADIX_10), LintId::of(functions::DOUBLE_MUST_USE), LintId::of(functions::MUST_USE_UNIT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 44d3fec8bd24..6eaf17709a88 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -173,6 +173,7 @@ store.register_lints(&[ formatting::SUSPICIOUS_ELSE_FORMATTING, formatting::SUSPICIOUS_UNARY_OP_FORMATTING, from_over_into::FROM_OVER_INTO, + from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR, from_str_radix_10::FROM_STR_RADIX_10, functions::DOUBLE_MUST_USE, functions::MUST_USE_CANDIDATE, diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index b70c4bb73e57..61af098c59a0 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -21,6 +21,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING), LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING), LintId::of(formatting::SUSPICIOUS_UNARY_OP_FORMATTING), + LintId::of(from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR), LintId::of(loops::EMPTY_LOOP), LintId::of(loops::MUT_RANGE_BOUND), LintId::of(methods::NO_EFFECT_REPLACE), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8723eb7943cf..28248d01b643 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -230,6 +230,7 @@ mod format_impl; mod format_push_string; mod formatting; mod from_over_into; +mod from_raw_with_void_ptr; mod from_str_radix_10; mod functions; mod future_not_send; @@ -918,6 +919,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(implicit_saturating_add::ImplicitSaturatingAdd)); store.register_early_pass(|| Box::new(partial_pub_fields::PartialPubFields)); store.register_late_pass(|_| Box::new(missing_trait_methods::MissingTraitMethods)); + store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/src/docs.rs b/src/docs.rs index e9aa6e880970..d9c34a6c0498 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -179,6 +179,7 @@ docs! { "format_push_string", "from_iter_instead_of_collect", "from_over_into", + "from_raw_with_void_ptr", "from_str_radix_10", "future_not_send", "get_first", diff --git a/src/docs/from_raw_with_void_ptr.txt b/src/docs/from_raw_with_void_ptr.txt new file mode 100644 index 000000000000..755fff23955d --- /dev/null +++ b/src/docs/from_raw_with_void_ptr.txt @@ -0,0 +1,22 @@ +### What it does +Checks if we're passing a `c_void` raw pointer to `Box::from_raw(_)` + +### Why is this bad? +However, it is easy to run into the pitfall of calling from_raw with the c_void pointer. +Note that the definition of, say, Box::from_raw is: + +`pub unsafe fn from_raw(raw: *mut T) -> Box` + +meaning that if you pass a *mut c_void you will get a Box. +Per the safety requirements in the documentation, for this to be safe, +c_void would need to have the same memory layout as the original type, which is often not the case. + +### Example +``` +let ptr = Box::into_raw(Box::new(42usize)) as *mut c_void; +let _ = unsafe { Box::from_raw(ptr) }; +``` +Use instead: +``` +let _ = unsafe { Box::from_raw(ptr as *mut usize) }; +``` diff --git a/tests/ui/from_raw_with_void_ptr.rs b/tests/ui/from_raw_with_void_ptr.rs new file mode 100644 index 000000000000..de5a8d60092b --- /dev/null +++ b/tests/ui/from_raw_with_void_ptr.rs @@ -0,0 +1,16 @@ +#![warn(clippy::from_raw_with_void_ptr)] + +use std::ffi::c_void; + +fn main() { + // must lint + let ptr = Box::into_raw(Box::new(42usize)) as *mut c_void; + let _ = unsafe { Box::from_raw(ptr) }; + + // shouldn't be linted + let _ = unsafe { Box::from_raw(ptr as *mut usize) }; + + // shouldn't be linted + let should_not_lint_ptr = Box::into_raw(Box::new(12u8)) as *mut u8; + let _ = unsafe { Box::from_raw(should_not_lint_ptr as *mut u8) }; +} diff --git a/tests/ui/from_raw_with_void_ptr.stderr b/tests/ui/from_raw_with_void_ptr.stderr new file mode 100644 index 000000000000..9e8e3e445465 --- /dev/null +++ b/tests/ui/from_raw_with_void_ptr.stderr @@ -0,0 +1,15 @@ +error: creating a `Box` from a raw void pointer + --> $DIR/from_raw_with_void_ptr.rs:8:22 + | +LL | let _ = unsafe { Box::from_raw(ptr) }; + | ^^^^^^^^^^^^^^^^^^ + | +help: cast this to a pointer of the actual type + --> $DIR/from_raw_with_void_ptr.rs:8:36 + | +LL | let _ = unsafe { Box::from_raw(ptr) }; + | ^^^ + = note: `-D clippy::from-raw-with-void-ptr` implied by `-D warnings` + +error: aborting due to previous error +