add new lint as_underscore_ptr to check for as *{const,mut} _

Author: asquared31415

CHANGELOG.md

@@ -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

clippy_lints/src/casts/as_underscore_ptr.rs

@@ -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
+    }
+}

clippy_lints/src/casts/mod.rs

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

clippy_lints/src/declared_lints.rs

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

clippy_lints/src/only_used_in_recursion.rs

@@ -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;
@@ -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 {
@@ -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,
     }
 }

clippy_utils/src/ty.rs

@@ -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,
+    }
+}

tests/ui/as_underscore_ptr.fixed

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

tests/ui/as_underscore_ptr.rs

@@ -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 _;
+}