Skip to content

Commit

Permalink
add boxed_void lint
Browse files Browse the repository at this point in the history
  • Loading branch information
roynrishingha committed Oct 23, 2022
1 parent 967f172 commit b083a39
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 57 additions & 0 deletions clippy_lints/src/from_raw_with_void_ptr.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
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 @@ -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),
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 @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_suspicious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
1 change: 1 addition & 0 deletions src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
22 changes: 22 additions & 0 deletions src/docs/from_raw_with_void_ptr.txt
Original file line number Diff line number Diff line change
@@ -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) };
```
16 changes: 16 additions & 0 deletions tests/ui/from_raw_with_void_ptr.rs
Original file line number Diff line number Diff line change
@@ -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) };
}
15 changes: 15 additions & 0 deletions tests/ui/from_raw_with_void_ptr.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit b083a39

Please sign in to comment.