Implement a lint for implicit autoref of raw pointer dereference

compiler/rustc_lint/src/implicit_unsafe_autorefs.rs

@@ -0,0 +1,83 @@
+use crate::{LateContext, LateLintPass, LintContext};
+
+use rustc_errors::Applicability;
+use rustc_hir::{self as hir, Expr, ExprKind, UnOp};
+use rustc_middle::ty::adjustment::{Adjust, AutoBorrow};
+
+declare_lint! {
+    /// The `implicit_unsafe_autorefs` lint checks for implicitly taken references to dereferences of raw pointers.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
+    ///     addr_of_mut!((*ptr)[..16])
+    ///     //                 ^^^^^^ this calls `IndexMut::index_mut(&mut ..., ..16)`,
+    ///     //                        implicitly creating a reference
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// When working with raw pointers it's usually undesirable to create references,
+    /// since they inflict a lot of safety requirement. Unfortunately, it's possible
+    /// to take a reference to a dereferece of a raw pointer implitly, which inflicts
+    /// the usual reference requirements without you even knowing that.
+    /// 
+    /// If you are sure, you can soundly take a reference, then you can take it explicitly:
+    /// ```rust
+    /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
+    ///     addr_of_mut!((&mut *ptr)[..16])
+    /// }
+    /// ```
+    /// 
+    /// Otherwise try to find an alternative way to achive your goals that work only with
+    /// raw pointers:
+    /// ```rust
+    /// #![feature(slice_ptr_get)]
+    /// 
+    /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] {
+    ///     ptr.get_unchecked_mut(..16)
+    /// }
+    /// ```
+    pub IMPLICIT_UNSAFE_AUTOREFS,
+    Deny,
+    "implicit reference to a dereference of a raw pointer"
+}
+
+declare_lint_pass!(ImplicitUnsafeAutorefs => [IMPLICIT_UNSAFE_AUTOREFS]);
+
+impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        let typeck = cx.typeck_results();
+        let adjustments_table = typeck.adjustments();
+
+        if let Some(adjustments) = adjustments_table.get(expr.hir_id) 
+        && let [adjustment] = &**adjustments
+        // An auto-borrow
+        && let Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) = adjustment.kind
+        // ... of a deref
+        && let ExprKind::Unary(UnOp::Deref, dereferenced) = expr.kind
+        // ... of a raw pointer
+        && typeck.expr_ty(dereferenced).is_unsafe_ptr()
+        {
+            let mutbl = hir::Mutability::prefix_str(&mutbl.into());
+
+            let msg = "implicit auto-ref creates a reference to a dereference of a raw pointer";
+            cx.struct_span_lint(IMPLICIT_UNSAFE_AUTOREFS, expr.span, msg, |lint| {
+                lint
+                    .note("creating a reference inflicts a lot of safety requirements")
+                    .multipart_suggestion(
+                        "if this reference is intentional, make it explicit", 
+                        vec![
+                            (expr.span.shrink_to_lo(), format!("(&{mutbl}")),
+                            (expr.span.shrink_to_hi(), ")".to_owned())
+                        ], 
+                        Applicability::MaybeIncorrect
+                    )
+            })
+        }
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -54,6 +54,7 @@ mod errors;
 mod expect;
 mod for_loops_over_fallibles;
 pub mod hidden_unicode_codepoints;
+mod implicit_unsafe_autorefs;
 mod internal;
 mod late;
 mod let_underscore;
@@ -89,6 +90,7 @@ use builtin::*;
 use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
 use for_loops_over_fallibles::*;
 use hidden_unicode_codepoints::*;
+use implicit_unsafe_autorefs::*;
 use internal::*;
 use let_underscore::*;
 use methods::*;
@@ -191,6 +193,7 @@ macro_rules! late_lint_mod_passes {
             $args,
             [
                 ForLoopsOverFallibles: ForLoopsOverFallibles,
+                ImplicitUnsafeAutorefs: ImplicitUnsafeAutorefs,
                 HardwiredLints: HardwiredLints,
                 ImproperCTypesDeclarations: ImproperCTypesDeclarations,
                 ImproperCTypesDefinitions: ImproperCTypesDefinitions,

library/alloc/src/collections/btree/node.rs

@@ -1699,7 +1699,7 @@ unsafe fn slice_insert<T>(slice: &mut [MaybeUninit<T>], idx: usize, val: T) {
         if len > idx + 1 {
             ptr::copy(slice_ptr.add(idx), slice_ptr.add(idx + 1), len - idx - 1);
         }
-        (*slice_ptr.add(idx)).write(val);
+        slice_ptr.add(idx).cast::<T>().write(val);
     }
 }
 
@@ -1713,7 +1713,7 @@ unsafe fn slice_remove<T>(slice: &mut [MaybeUninit<T>], idx: usize) -> T {
         let len = slice.len();
         debug_assert!(idx < len);
         let slice_ptr = slice.as_mut_ptr();
-        let ret = (*slice_ptr.add(idx)).assume_init_read();
+        let ret = slice_ptr.add(idx).cast::<T>().read();
         ptr::copy(slice_ptr.add(idx + 1), slice_ptr.add(idx), len - idx - 1);
         ret
     }

library/alloc/src/string.rs

@@ -2909,7 +2909,7 @@ impl Drop for Drain<'_> {
         unsafe {
             // Use Vec::drain. "Reaffirm" the bounds checks to avoid
             // panic code being inserted again.
-            let self_vec = (*self.string).as_mut_vec();
+            let self_vec = (&mut *self.string).as_mut_vec();
             if self.start <= self.end && self.end <= self_vec.len() {
                 self_vec.drain(self.start..self.end);
             }

library/alloc/src/vec/mod.rs

@@ -1942,7 +1942,7 @@ impl<T, A: Allocator> Vec<T, A> {
     #[cfg(not(no_global_oom_handling))]
     #[inline]
     unsafe fn append_elements(&mut self, other: *const [T]) {
-        let count = unsafe { (*other).len() };
+        let count = other.len();
         self.reserve(count);
         let len = self.len();
         unsafe { ptr::copy_nonoverlapping(other as *const T, self.as_mut_ptr().add(len), count) };

library/proc_macro/src/bridge/closure.rs

@@ -19,7 +19,7 @@ struct Env;
 impl<'a, A, R, F: FnMut(A) -> R> From<&'a mut F> for Closure<'a, A, R> {
     fn from(f: &'a mut F) -> Self {
         unsafe extern "C" fn call<A, R, F: FnMut(A) -> R>(env: *mut Env, arg: A) -> R {
-            (*(env as *mut _ as *mut F))(arg)
+            (&mut *(env as *mut _ as *mut F))(arg)
         }
         Closure { call: call::<A, R, F>, env: f as *mut _ as *mut Env, _marker: PhantomData }
     }

library/std/src/sync/mpsc/oneshot.rs

@@ -86,7 +86,7 @@ impl<T> Packet<T> {
                 NothingSent => {}
                 _ => panic!("sending on a oneshot that's already sent on "),
             }
-            assert!((*self.data.get()).is_none());
+            assert!((&*self.data.get()).is_none());
             ptr::write(self.data.get(), Some(t));
             ptr::write(self.upgrade.get(), SendUsed);
 
@@ -289,7 +289,7 @@ impl<T> Packet<T> {
             // We then need to check to see if there was an upgrade requested,
             // and if so, the upgraded port needs to have its selection aborted.
             DISCONNECTED => unsafe {
-                if (*self.data.get()).is_some() {
+                if (&*self.data.get()).is_some() {
                     Ok(true)
                 } else {
                     match ptr::replace(self.upgrade.get(), SendUsed) {

library/std/src/sys/unix/stack_overflow.rs

@@ -81,7 +81,7 @@ mod imp {
         _data: *mut libc::c_void,
     ) {
         let guard = thread_info::stack_guard().unwrap_or(0..0);
-        let addr = (*info).si_addr() as usize;
+        let addr = (&*info).si_addr() as usize;
 
         // If the faulting address is within the guard page, then we print a
         // message saying so and abort.

library/std/src/thread/local.rs

@@ -799,7 +799,7 @@ mod lazy {
             // the inner cell nor mutable reference to the Option<T> inside said
             // cell. This make it safe to hand a reference, though the lifetime
             // of 'static is itself unsafe, making the get method unsafe.
-            unsafe { (*self.inner.get()).as_ref() }
+            unsafe { (&*self.inner.get()).as_ref() }
         }
 
         /// The caller must ensure that no reference is active: this method
@@ -853,7 +853,7 @@ mod lazy {
         #[allow(unused)]
         pub unsafe fn take(&mut self) -> Option<T> {
             // SAFETY: See doc comment for this method.
-            unsafe { (*self.inner.get()).take() }
+            unsafe { (&mut *self.inner.get()).take() }
         }
     }
 }

src/test/ui/lint/implicit_unsafe_autorefs.fixed

@@ -0,0 +1,14 @@
+// run-rustfix
+use std::ptr::{addr_of, addr_of_mut};
+
+unsafe fn _test_mut(ptr: *mut [u8]) -> *mut [u8] {
+    addr_of_mut!((&mut (*ptr))[..16])
+    //~^ error: implicit auto-ref creates a reference to a dereference of a raw pointer
+}
+
+unsafe fn _test_const(ptr: *const [u8]) -> *const [u8] {
+    addr_of!((&(*ptr))[..16])
+    //~^ error: implicit auto-ref creates a reference to a dereference of a raw pointer
+}
+
+fn main() {}

src/test/ui/lint/implicit_unsafe_autorefs.rs

@@ -0,0 +1,14 @@
+// run-rustfix
+use std::ptr::{addr_of, addr_of_mut};
+
+unsafe fn _test_mut(ptr: *mut [u8]) -> *mut [u8] {
+    addr_of_mut!((*ptr)[..16])
+    //~^ error: implicit auto-ref creates a reference to a dereference of a raw pointer
+}
+
+unsafe fn _test_const(ptr: *const [u8]) -> *const [u8] {
+    addr_of!((*ptr)[..16])
+    //~^ error: implicit auto-ref creates a reference to a dereference of a raw pointer
+}
+
+fn main() {}

src/test/ui/lint/implicit_unsafe_autorefs.stderr

@@ -0,0 +1,27 @@
+error: implicit auto-ref creates a reference to a dereference of a raw pointer
+  --> $DIR/implicit_unsafe_autoref.rs:5:18
+   |
+LL |     addr_of_mut!((*ptr)[..16])
+   |                  ^^^^^^
+   |
+   = note: creating a reference inflicts a lot of safety requirements
+   = note: `#[deny(implicit_unsafe_autorefs)]` on by default
+help: if this reference is intentional, make it explicit
+   |
+LL |     addr_of_mut!((&mut (*ptr))[..16])
+   |                  +++++       +
+
+error: implicit auto-ref creates a reference to a dereference of a raw pointer
+  --> $DIR/implicit_unsafe_autoref.rs:10:14
+   |
+LL |     addr_of!((*ptr)[..16])
+   |              ^^^^^^
+   |
+   = note: creating a reference inflicts a lot of safety requirements
+help: if this reference is intentional, make it explicit
+   |
+LL |     addr_of!((&(*ptr))[..16])
+   |              ++      +
+
+error: aborting due to 2 previous errors
+