Auto merge of #9690 - royrustdev:boxed_void, r=flip1995

add `from_raw_with_void_ptr` lint

This PR `fixes #9679`

- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`

---

changelog:  [`from_raw_with_void_ptr`]: added new lint

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

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<T>`
+    ///
+    /// meaning that if you pass a *mut c_void you will get a Box<c_void>.
+    /// 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");
+        }
+    }
+}

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),

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,

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),

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`
 }
 

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",

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<T>`
+
+meaning that if you pass a *mut c_void you will get a Box<c_void>.
+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) };
+```

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) };
+}

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
+