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 new lint as_underscore_ptr to check for as *{const,mut} _ #10567

Closed
Closed
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 @@ -4392,6 +4392,7 @@ Released 2018-09-13
[`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
[`as_ptr_cast_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut
[`as_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore
[`as_underscore_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore_ptr
[`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
[`assertions_on_result_states`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_result_states
[`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
Expand Down
58 changes: 58 additions & 0 deletions clippy_lints/src/casts/as_underscore_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::ty::is_ptr_like;
use rustc_errors::Applicability;
use rustc_hir::{Expr, MutTy, Ty, TyKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, TypeAndMut};

use super::AS_UNDERSCORE_PTR;

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, mut target_ty: &Ty<'_>) {
if let TyKind::Ptr(MutTy { mutbl, .. }) = target_ty.kind {
// get this before stripping the pointers, so the suggestion suggests replacing the whole type
let ty_span = target_ty.span;

// strip all pointers from the type
while let TyKind::Ptr(MutTy { ty: new_ty, .. }) = target_ty.kind {
target_ty = new_ty;
}

if matches!(target_ty.kind, TyKind::Infer) {
let mutbl_str = match mutbl {
rustc_ast::Mutability::Not => "const",
rustc_ast::Mutability::Mut => "mut",
};
span_lint_and_then(
cx,
AS_UNDERSCORE_PTR,
expr.span,
format!("using `as *{mutbl_str} _` conversion").as_str(),
|diag| {
let ty_resolved = cx.typeck_results().expr_ty(expr);
if let ty::Error(_) = ty_resolved.kind() {
diag.help("consider giving the type explicitly");
} else {
// strip the first pointer of the resolved type of the cast, to test if the pointed to type
// is also a pointer-like. This might be a logic error, so bring extra notice to it.
if let ty::RawPtr(TypeAndMut { ty: pointee_ty, .. }) = ty_resolved.kind() {
if is_ptr_like(cx.tcx, *pointee_ty).is_some() {
diag.note("the pointed to type is still a pointer-like type");
}
} else {
unreachable!("The target type of a cast for `as_underscore_ptr` is a pointer");
}

diag.span_suggestion(
ty_span,
"consider giving the type explicitly",
ty_resolved,
Applicability::MachineApplicable,
);
}
},
);
}
} else {
// not a pointer
}
}
49 changes: 49 additions & 0 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod as_ptr_cast_mut;
mod as_underscore;
mod as_underscore_ptr;
mod borrow_as_ptr;
mod cast_abs_to_unsigned;
mod cast_enum_constructor;
Expand Down Expand Up @@ -555,6 +556,52 @@ declare_clippy_lint! {
"detects `as _` conversion"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of `as *const _` or `as *mut _` where the pointed to type is inferred.
///
/// ### Why is this bad?
/// When converting to a pointer, it can be dangerous to not specify the type. Type inference can
/// be affected by many things, types may not always be obvious, and when working with pointers, while
/// the pointed to type doesn't technically matter until an access, you should always know what types
/// you intend to work with. When using multiple chained `as` casts, it can be especially dangerous,
/// because `*const T as *const U` **always compiles** (for Sized types T and U).
///
/// ### Example
/// ```rust
/// // intent: turn a `&u64` into a `*const u8` that points to the bytes of the u64 (⚠️does not work⚠️)
/// fn owo(x: &u64) -> *const u8 {
/// // ⚠️ `&x` is a `&&u64`, so this turns a double pointer into a single pointer
/// // ⚠️ This pointer is a dangling pointer to a local
/// &x as *const _ as *const u8
/// }
/// ```
/// Use instead:
/// ```rust
/// // intent: turn a `&u64` into a `*const u8` that points to the bytes of the u64 (⚠️does not work⚠️)
/// fn owo(x: &u64) -> *const u8 {
/// // ⚠️ `&x` is a `&&u64`, so this turns a double pointer into a single pointer
/// // ⚠️ This pointer is a dangling pointer to a local
/// &x as *const &u64 as *const u8
/// }
/// ```
/// While this is still buggy, the bug is more visible here: you have a `*const &u64`. To actually fix the
/// underlying issue, use:
/// ```rust
/// // turn a `&u64` into a `*const u8` that points to the bytes of the u64
/// fn owo(x: &u64) -> *const u8 {
/// x as *const u64 as *const u8
/// // ^ now `x` instead of `&x`
/// }
/// ```
/// This lint cannot suggest this final code because it's a more broad `restriction` lint that forces the
/// user to be more specific, and this transformation is not always valid.
#[clippy::version = "1.70.0"]
pub AS_UNDERSCORE_PTR,
restriction,
"detects `as *{const,mut} _ conversions"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of `&expr as *const T` or
Expand Down Expand Up @@ -693,6 +740,7 @@ impl_lint_pass!(Casts => [
CAST_ENUM_CONSTRUCTOR,
CAST_ABS_TO_UNSIGNED,
AS_UNDERSCORE,
AS_UNDERSCORE_PTR,
BORROW_AS_PTR,
CAST_SLICE_FROM_RAW_PARTS,
AS_PTR_CAST_MUT,
Expand Down Expand Up @@ -741,6 +789,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
}

as_underscore::check(cx, expr, cast_to_hir);
as_underscore_ptr::check(cx, expr, cast_to_hir);

if self.msrv.meets(msrvs::BORROW_AS_PTR) {
borrow_as_ptr::check(cx, expr, cast_expr, cast_to_hir);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::cargo::WILDCARD_DEPENDENCIES_INFO,
crate::casts::AS_PTR_CAST_MUT_INFO,
crate::casts::AS_UNDERSCORE_INFO,
crate::casts::AS_UNDERSCORE_PTR_INFO,
crate::casts::BORROW_AS_PTR_INFO,
crate::casts::CAST_ABS_TO_UNSIGNED_INFO,
crate::casts::CAST_ENUM_CONSTRUCTOR_INFO,
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hir::hir_id::HirIdMap;
use rustc_hir::{Body, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Node, PatKind, TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::subst::{EarlyBinder, GenericArgKind, SubstsRef};
use rustc_middle::ty::{self, ConstKind};
use rustc_middle::ty::{self, ConstKind, List};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{kw, Ident};
use rustc_span::Span;
Expand Down Expand Up @@ -249,7 +249,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
{
(
trait_item_id,
FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const _ as usize),
FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const List<_> as usize),
usize::from(sig.decl.implicit_self.has_implicit_self()),
)
} else {
Expand Down Expand Up @@ -390,6 +390,6 @@ fn has_matching_substs(kind: FnKind, substs: SubstsRef<'_>) -> bool {
GenericArgKind::Const(c) => matches!(c.kind(), ConstKind::Param(c) if c.index as usize == idx),
}),
#[allow(trivial_casts)]
FnKind::ImplTraitFn(expected_substs) => substs as *const _ as usize == expected_substs,
FnKind::ImplTraitFn(expected_substs) => substs as *const List<_> as usize == expected_substs,
}
}
25 changes: 25 additions & 0 deletions clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1164,3 +1164,28 @@ pub fn is_interior_mut_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
_ => false,
}
}

/// If a type is a pointer or something that is known to act like a pointer, returns
/// the wrapped type by recursively unpacking the type. Otherwise returns None.
/// "pointer-like" types are:
/// &T
/// &mut T
/// *const T
/// *mut T
/// Box<T>
/// Rc<T>
/// Arc<T>
pub fn is_ptr_like<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
match ty.kind() {
ty::Ref(_, ty, _) | ty::RawPtr(ty::TypeAndMut { ty, .. }) => is_ptr_like(tcx, *ty).or(Some(*ty)),
ty::Adt(def, generics)
if def.is_box()
|| tcx.is_diagnostic_item(sym::Rc, def.did())
|| tcx.is_diagnostic_item(sym::Arc, def.did()) =>
{
let ty = generics.type_at(0);
is_ptr_like(tcx, ty).or(Some(ty))
},
_ => None,
}
}
66 changes: 66 additions & 0 deletions tests/ui/as_underscore_ptr.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//@run-rustfix

#![allow(unused)]
#![deny(clippy::as_underscore_ptr)]

use std::rc::Rc;

// intent: turn a `&u64` into a `*const u8` that points to the bytes of the u64 (⚠️does not work⚠️)
fn owo(x: &u64) -> *const u8 {
// ⚠️ `&x` is a `&&u64`, so this turns a double pointer into a single pointer
// ⚠️ This pointer is a dangling pointer to a local
&x as *const &u64 as *const u8
}

// note: this test is the same basic idea as above, but uses a `&self` which can be even more
// misleading. In fact this is the case that was found in *real code* (that didn't work)
// that inspired this lint. Make sure that it lints with `&self`!
struct UwU;
impl UwU {
// like above, this creates a double pointer, and then returns a dangling single pointer
// intent: turn a `&UwU` into a `*const u8` that points to the same data
fn as_ptr(&self) -> *const u8 {
// ⚠️ `&self` is a `&&UwU`, so this turns a double pointer into a single pointer
// ⚠️ This pointer is a dangling pointer to a local
&self as *const &UwU as *const u8
}
}

fn use_ptr(_: *const ()) {}

fn main() {
let _: *const u8 = 1 as *const u8;
use_ptr(1 as *const ());

let _: *mut u8 = 1 as *mut u8;

// Pointer-to-pointer note tests
// If a _ resolves to a type that is itself a pointer, it's likely a mistake
// Show a note for all of these cases

// const ptr to ref
let r = &&1;
let _ = r as *const &i32;

// const ptr to mut ref
let r = &&mut 1;
let _ = r as *const &mut i32;

// mut ptr to ref
let r = &mut &1;
let _ = r as *mut &i32;

// mut ptr to mut ref
let r = &mut &mut 1;
let _ = r as *mut &mut i32;

// ptr to Box
let b = Box::new(1);
let r = &b;
let _ = r as *const std::boxed::Box<i32>;

// ptr to Rc
let rc = Rc::new(1);
let r = &rc;
let _ = r as *const std::rc::Rc<i32>;
}
66 changes: 66 additions & 0 deletions tests/ui/as_underscore_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//@run-rustfix

#![allow(unused)]
#![deny(clippy::as_underscore_ptr)]

use std::rc::Rc;

// intent: turn a `&u64` into a `*const u8` that points to the bytes of the u64 (⚠️does not work⚠️)
fn owo(x: &u64) -> *const u8 {
// ⚠️ `&x` is a `&&u64`, so this turns a double pointer into a single pointer
// ⚠️ This pointer is a dangling pointer to a local
&x as *const _ as *const u8
}

// note: this test is the same basic idea as above, but uses a `&self` which can be even more
// misleading. In fact this is the case that was found in *real code* (that didn't work)
// that inspired this lint. Make sure that it lints with `&self`!
struct UwU;
impl UwU {
// like above, this creates a double pointer, and then returns a dangling single pointer
// intent: turn a `&UwU` into a `*const u8` that points to the same data
fn as_ptr(&self) -> *const u8 {
// ⚠️ `&self` is a `&&UwU`, so this turns a double pointer into a single pointer
// ⚠️ This pointer is a dangling pointer to a local
&self as *const _ as *const u8
}
}

fn use_ptr(_: *const ()) {}

fn main() {
let _: *const u8 = 1 as *const _;
use_ptr(1 as *const _);

let _: *mut u8 = 1 as *mut _;

// Pointer-to-pointer note tests
// If a _ resolves to a type that is itself a pointer, it's likely a mistake
// Show a note for all of these cases

// const ptr to ref
let r = &&1;
let _ = r as *const _;

// const ptr to mut ref
let r = &&mut 1;
let _ = r as *const _;

// mut ptr to ref
let r = &mut &1;
let _ = r as *mut _;

// mut ptr to mut ref
let r = &mut &mut 1;
let _ = r as *mut _;

// ptr to Box
let b = Box::new(1);
let r = &b;
let _ = r as *const _;

// ptr to Rc
let rc = Rc::new(1);
let r = &rc;
let _ = r as *const _;
}
Loading