Auto merge of #9572 - Nilstrieb:as-ptr-cast-mut, r=dswij

Add `as_ptr_cast_mut` lint

This lint detects calls to a `&self`-taking `as_ptr` method, where the result is then immediately cast to a `*mut T`. Code like this is probably invalid, as that pointer will not have write permissions, and `*mut T` is usually used to write through.

Examples of broken code with this pattern:
https://miri.saethlin.dev/ub?crate=lol_alloc&version=0.1.3
https://miri.saethlin.dev/ub?crate=sophon-wasm&version=0.19.0
https://miri.saethlin.dev/ub?crate=polars-core&version=0.24.2
https://miri.saethlin.dev/ub?crate=ach-cell&version=0.1.17

changelog: Add [`as_ptr_cast_mut`]

CHANGELOG.md

@@ -3735,6 +3735,7 @@ Released 2018-09-13
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
 [`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects
 [`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
 [`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

clippy_lints/src/casts/as_ptr_cast_mut.rs

@@ -0,0 +1,38 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_opt;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_middle::{
+    mir::Mutability,
+    ty::{self, Ty, TypeAndMut},
+};
+
+use super::AS_PTR_CAST_MUT;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_to: Ty<'_>) {
+    if let ty::RawPtr(ptrty @ TypeAndMut { mutbl: Mutability::Mut, .. }) = cast_to.kind()
+        && let ty::RawPtr(TypeAndMut { mutbl: Mutability::Not, .. }) =
+            cx.typeck_results().node_type(cast_expr.hir_id).kind()
+        && let ExprKind::MethodCall(method_name, receiver, [], _) = cast_expr.peel_blocks().kind
+        && method_name.ident.name == rustc_span::sym::as_ptr
+        && let Some(as_ptr_did) = cx.typeck_results().type_dependent_def_id(cast_expr.peel_blocks().hir_id)
+        && let as_ptr_sig = cx.tcx.fn_sig(as_ptr_did)
+        && let Some(first_param_ty) = as_ptr_sig.skip_binder().inputs().iter().next()
+        && let ty::Ref(_, _, Mutability::Not) = first_param_ty.kind()
+        && let Some(recv) = snippet_opt(cx, receiver.span)
+    {
+        // `as_mut_ptr` might not exist
+        let applicability = Applicability::MaybeIncorrect;
+
+        span_lint_and_sugg(
+            cx,
+            AS_PTR_CAST_MUT,
+            expr.span,
+            &format!("casting the result of `as_ptr` to *{ptrty}"),
+            "replace with",
+            format!("{recv}.as_mut_ptr()"),
+            applicability
+        );
+    }
+}

clippy_lints/src/casts/mod.rs

@@ -1,3 +1,4 @@
+mod as_ptr_cast_mut;
 mod as_underscore;
 mod borrow_as_ptr;
 mod cast_abs_to_unsigned;
@@ -596,6 +597,32 @@ declare_clippy_lint! {
     "casting a slice created from a pointer and length to a slice pointer"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for the result of a `&self`-taking `as_ptr` being cast to a mutable pointer
+    ///
+    /// ### Why is this bad?
+    /// Since `as_ptr` takes a `&self`, the pointer won't have write permissions unless interior
+    /// mutability is used, making it unlikely that having it as a mutable pointer is correct.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let string = String::with_capacity(1);
+    /// let ptr = string.as_ptr() as *mut u8;
+    /// unsafe { ptr.write(4) }; // UNDEFINED BEHAVIOUR
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let mut string = String::with_capacity(1);
+    /// let ptr = string.as_mut_ptr();
+    /// unsafe { ptr.write(4) };
+    /// ```
+    #[clippy::version = "1.66.0"]
+    pub AS_PTR_CAST_MUT,
+    nursery,
+    "casting the result of the `&self`-taking `as_ptr` to a mutabe pointer"
+}
+
 pub struct Casts {
     msrv: Option<RustcVersion>,
 }
@@ -627,7 +654,8 @@ impl_lint_pass!(Casts => [
     CAST_ABS_TO_UNSIGNED,
     AS_UNDERSCORE,
     BORROW_AS_PTR,
-    CAST_SLICE_FROM_RAW_PARTS
+    CAST_SLICE_FROM_RAW_PARTS,
+    AS_PTR_CAST_MUT,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Casts {
@@ -653,6 +681,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
                 return;
             }
             cast_slice_from_raw_parts::check(cx, expr, cast_expr, cast_to, self.msrv);
+            as_ptr_cast_mut::check(cx, expr, cast_expr, cast_to);
             fn_to_numeric_cast_any::check(cx, expr, cast_expr, cast_from, cast_to);
             fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to);
             fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to);

clippy_lints/src/lib.register_lints.rs

@@ -66,6 +66,7 @@ store.register_lints(&[
     cargo::NEGATIVE_FEATURE_NAMES,
     cargo::REDUNDANT_FEATURE_NAMES,
     cargo::WILDCARD_DEPENDENCIES,
+    casts::AS_PTR_CAST_MUT,
     casts::AS_UNDERSCORE,
     casts::BORROW_AS_PTR,
     casts::CAST_ABS_TO_UNSIGNED,

clippy_lints/src/lib.register_nursery.rs

@@ -4,6 +4,7 @@
 
 store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
     LintId::of(attrs::EMPTY_LINE_AFTER_OUTER_ATTR),
+    LintId::of(casts::AS_PTR_CAST_MUT),
     LintId::of(cognitive_complexity::COGNITIVE_COMPLEXITY),
     LintId::of(copies::BRANCHES_SHARING_CODE),
     LintId::of(derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ),

src/docs.rs

@@ -28,6 +28,7 @@ docs! {
     "approx_constant",
     "arithmetic_side_effects",
     "as_conversions",
+    "as_ptr_cast_mut",
     "as_underscore",
     "assertions_on_constants",
     "assertions_on_result_states",

src/docs/as_ptr_cast_mut.txt

@@ -0,0 +1,19 @@
+### What it does
+Checks for the result of a `&self`-taking `as_ptr` being cast to a mutable pointer
+
+### Why is this bad?
+Since `as_ptr` takes a `&self`, the pointer won't have write permissions unless interior
+mutability is used, making it unlikely that having it as a mutable pointer is correct.
+
+### Example
+```
+let string = String::with_capacity(1);
+let ptr = string.as_ptr() as *mut u8;
+unsafe { ptr.write(4) }; // UNDEFINED BEHAVIOUR
+```
+Use instead:
+```
+let mut string = String::with_capacity(1);
+let ptr = string.as_mut_ptr();
+unsafe { ptr.write(4) };
+```

tests/ui/as_ptr_cast_mut.rs

@@ -0,0 +1,37 @@
+#![allow(unused)]
+#![warn(clippy::as_ptr_cast_mut)]
+#![allow(clippy::wrong_self_convention)]
+
+struct MutPtrWrapper(Vec<u8>);
+impl MutPtrWrapper {
+    fn as_ptr(&mut self) -> *const u8 {
+        self.0.as_mut_ptr() as *const u8
+    }
+}
+
+struct Covariant<T>(*const T);
+impl<T> Covariant<T> {
+    fn as_ptr(self) -> *const T {
+        self.0
+    }
+}
+
+fn main() {
+    let mut string = String::new();
+    let _ = string.as_ptr() as *mut u8;
+    let _: *mut i8 = string.as_ptr() as *mut _;
+    let _ = string.as_ptr() as *const i8;
+    let _ = string.as_mut_ptr();
+    let _ = string.as_mut_ptr() as *mut u8;
+    let _ = string.as_mut_ptr() as *const u8;
+
+    let nn = std::ptr::NonNull::new(4 as *mut u8).unwrap();
+    let _ = nn.as_ptr() as *mut u8;
+
+    let mut wrap = MutPtrWrapper(Vec::new());
+    let _ = wrap.as_ptr() as *mut u8;
+
+    let mut local = 4;
+    let ref_with_write_perm = Covariant(std::ptr::addr_of_mut!(local) as *const _);
+    let _ = ref_with_write_perm.as_ptr() as *mut u8;
+}

tests/ui/as_ptr_cast_mut.stderr

@@ -0,0 +1,16 @@
+error: casting the result of `as_ptr` to *mut u8
+  --> $DIR/as_ptr_cast_mut.rs:21:13
+   |
+LL |     let _ = string.as_ptr() as *mut u8;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `string.as_mut_ptr()`
+   |
+   = note: `-D clippy::as-ptr-cast-mut` implied by `-D warnings`
+
+error: casting the result of `as_ptr` to *mut i8
+  --> $DIR/as_ptr_cast_mut.rs:22:22
+   |
+LL |     let _: *mut i8 = string.as_ptr() as *mut _;
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `string.as_mut_ptr()`
+
+error: aborting due to 2 previous errors
+