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
+