Skip to content
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

add from_raw_with_void_ptr lint #9690

Merged
merged 1 commit into from
Oct 23, 2022
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
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())
roynrishingha marked this conversation as resolved.
Show resolved Hide resolved
&& 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