From 561dfaf000e944ae45a843d564c42d3ceb1104a4 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Sat, 29 Oct 2022 19:37:57 +0400 Subject: [PATCH 01/18] Implement a lint for implicit autoref of raw pointer dereference --- .../src/implicit_unsafe_autorefs.rs | 83 +++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 3 + .../ui/lint/implicit_unsafe_autorefs.fixed | 14 ++++ src/test/ui/lint/implicit_unsafe_autorefs.rs | 14 ++++ .../ui/lint/implicit_unsafe_autorefs.stderr | 27 ++++++ 5 files changed, 141 insertions(+) create mode 100644 compiler/rustc_lint/src/implicit_unsafe_autorefs.rs create mode 100644 src/test/ui/lint/implicit_unsafe_autorefs.fixed create mode 100644 src/test/ui/lint/implicit_unsafe_autorefs.rs create mode 100644 src/test/ui/lint/implicit_unsafe_autorefs.stderr diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs new file mode 100644 index 0000000000000..b84a3db2f3634 --- /dev/null +++ b/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 + ) + }) + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index fee6e080c4fc7..42bcf310d3c69 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/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, diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.fixed b/src/test/ui/lint/implicit_unsafe_autorefs.fixed new file mode 100644 index 0000000000000..818f299fc5b0a --- /dev/null +++ b/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() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.rs b/src/test/ui/lint/implicit_unsafe_autorefs.rs new file mode 100644 index 0000000000000..f96a45f8a485e --- /dev/null +++ b/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() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.stderr b/src/test/ui/lint/implicit_unsafe_autorefs.stderr new file mode 100644 index 0000000000000..0add9ce823e32 --- /dev/null +++ b/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 + From 493b5fd81cb0eacf2585cfbcd52c1bcf2ece58aa Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Sat, 29 Oct 2022 19:38:44 +0400 Subject: [PATCH 02/18] Make some auto-refs in std explicit --- library/alloc/src/collections/btree/node.rs | 4 ++-- library/alloc/src/string.rs | 2 +- library/alloc/src/vec/mod.rs | 2 +- library/proc_macro/src/bridge/closure.rs | 2 +- library/std/src/sync/mpsc/oneshot.rs | 4 ++-- library/std/src/sys/unix/stack_overflow.rs | 2 +- library/std/src/thread/local.rs | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index da766b67a328f..7325439c11a2f 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -1699,7 +1699,7 @@ unsafe fn slice_insert(slice: &mut [MaybeUninit], 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::().write(val); } } @@ -1713,7 +1713,7 @@ unsafe fn slice_remove(slice: &mut [MaybeUninit], 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::().read(); ptr::copy(slice_ptr.add(idx + 1), slice_ptr.add(idx), len - idx - 1); ret } diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index c436adf70067a..a6dec61957d2c 100644 --- a/library/alloc/src/string.rs +++ b/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); } diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index bbbdc3aa2a2d3..1c543d5517a33 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1942,7 +1942,7 @@ impl Vec { #[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) }; diff --git a/library/proc_macro/src/bridge/closure.rs b/library/proc_macro/src/bridge/closure.rs index d371ae3cea098..1255f35d417e4 100644 --- a/library/proc_macro/src/bridge/closure.rs +++ b/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 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::, env: f as *mut _ as *mut Env, _marker: PhantomData } } diff --git a/library/std/src/sync/mpsc/oneshot.rs b/library/std/src/sync/mpsc/oneshot.rs index 0e259b8aecb9a..625b6c0bf8640 100644 --- a/library/std/src/sync/mpsc/oneshot.rs +++ b/library/std/src/sync/mpsc/oneshot.rs @@ -86,7 +86,7 @@ impl Packet { 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 Packet { // 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) { diff --git a/library/std/src/sys/unix/stack_overflow.rs b/library/std/src/sys/unix/stack_overflow.rs index 75a5c0f927982..7a26755c5f805 100644 --- a/library/std/src/sys/unix/stack_overflow.rs +++ b/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. diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 5d267891bb0ed..4e6eae3e5aae5 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -799,7 +799,7 @@ mod lazy { // the inner cell nor mutable reference to the Option 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 { // SAFETY: See doc comment for this method. - unsafe { (*self.inner.get()).take() } + unsafe { (&mut *self.inner.get()).take() } } } } From e1c0b69bfdd0670cecc79c4d8efe3b190e9380f1 Mon Sep 17 00:00:00 2001 From: Waffle Maybe Date: Tue, 1 Nov 2022 11:55:47 +0400 Subject: [PATCH 03/18] Apply suggestions from code review Co-authored-by: klensy Co-authored-by: Ralf Jung --- compiler/rustc_lint/src/implicit_unsafe_autorefs.rs | 6 +++--- src/test/ui/lint/implicit_unsafe_autorefs.stderr | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index b84a3db2f3634..7213256f23213 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -23,7 +23,7 @@ declare_lint! { /// /// 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 + /// to take a reference to a dereference of a raw pointer implicitly, 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: @@ -68,9 +68,9 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { 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") + .note("creating a reference requires the pointer to be valid and imposes aliasing requirements") .multipart_suggestion( - "if this reference is intentional, make it explicit", + "try using a raw pointer method instead; or if this reference is intentional, make it explicit", vec![ (expr.span.shrink_to_lo(), format!("(&{mutbl}")), (expr.span.shrink_to_hi(), ")".to_owned()) diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.stderr b/src/test/ui/lint/implicit_unsafe_autorefs.stderr index 0add9ce823e32..d76a0fc329562 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.stderr +++ b/src/test/ui/lint/implicit_unsafe_autorefs.stderr @@ -1,24 +1,24 @@ error: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autoref.rs:5:18 + --> $DIR/implicit_unsafe_autorefs.rs:5:18 | LL | addr_of_mut!((*ptr)[..16]) | ^^^^^^ | - = note: creating a reference inflicts a lot of safety requirements + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements = note: `#[deny(implicit_unsafe_autorefs)]` on by default -help: if this reference is intentional, make it explicit +help: try using a raw pointer method instead; or 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 + --> $DIR/implicit_unsafe_autorefs.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 + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements +help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | addr_of!((&(*ptr))[..16]) | ++ + From 4a7791d5a79a7677d383afc1549d102dd988e265 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 12:03:22 +0000 Subject: [PATCH 04/18] `implicit_unsafe_autorefs`: make the lint warn by default --- compiler/rustc_lint/src/implicit_unsafe_autorefs.rs | 2 +- src/test/ui/lint/implicit_unsafe_autorefs.fixed | 5 +++-- src/test/ui/lint/implicit_unsafe_autorefs.rs | 5 +++-- src/test/ui/lint/implicit_unsafe_autorefs.stderr | 12 ++++++------ 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index 7213256f23213..06052b97fa8ee 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -43,7 +43,7 @@ declare_lint! { /// } /// ``` pub IMPLICIT_UNSAFE_AUTOREFS, - Deny, + Warn, "implicit reference to a dereference of a raw pointer" } diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.fixed b/src/test/ui/lint/implicit_unsafe_autorefs.fixed index 818f299fc5b0a..f24b79a452c9b 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.fixed +++ b/src/test/ui/lint/implicit_unsafe_autorefs.fixed @@ -1,14 +1,15 @@ +// check-pass // 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 + //~^ warn: 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 + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.rs b/src/test/ui/lint/implicit_unsafe_autorefs.rs index f96a45f8a485e..3e84901991647 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.rs +++ b/src/test/ui/lint/implicit_unsafe_autorefs.rs @@ -1,14 +1,15 @@ +// check-pass // 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 + //~^ warn: 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 + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.stderr b/src/test/ui/lint/implicit_unsafe_autorefs.stderr index d76a0fc329562..6790fb2cb543c 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.stderr +++ b/src/test/ui/lint/implicit_unsafe_autorefs.stderr @@ -1,18 +1,18 @@ -error: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:5:18 +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:6:18 | LL | addr_of_mut!((*ptr)[..16]) | ^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements - = note: `#[deny(implicit_unsafe_autorefs)]` on by default + = note: `#[warn(implicit_unsafe_autorefs)]` on by default help: try using a raw pointer method instead; or 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_autorefs.rs:10:14 +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:11:14 | LL | addr_of!((*ptr)[..16]) | ^^^^^^ @@ -23,5 +23,5 @@ help: try using a raw pointer method instead; or if this reference is intentiona LL | addr_of!((&(*ptr))[..16]) | ++ + -error: aborting due to 2 previous errors +warning: 2 warnings emitted From af4ade4cf60d64b9c8406bc32b21a2caef865fdd Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 12:00:36 +0000 Subject: [PATCH 05/18] `implicit_unsafe_autorefs`: lint field access too --- .../src/implicit_unsafe_autorefs.rs | 11 +++++-- .../ui/lint/implicit_unsafe_autorefs.fixed | 17 +++++++++-- src/test/ui/lint/implicit_unsafe_autorefs.rs | 17 +++++++++-- .../ui/lint/implicit_unsafe_autorefs.stderr | 30 +++++++++++++++++-- 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index 06052b97fa8ee..498197d045a1f 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -58,8 +58,8 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { && 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 place derived from a deref + && let ExprKind::Unary(UnOp::Deref, dereferenced) = skip_field_access(&expr.kind) // ... of a raw pointer && typeck.expr_ty(dereferenced).is_unsafe_ptr() { @@ -81,3 +81,10 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { } } } + +fn skip_field_access<'a>(mut expr: &'a ExprKind<'a>) -> &'a ExprKind<'a> { + while let ExprKind::Field(e, _) = expr { + expr = &e.kind; + } + expr +} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.fixed b/src/test/ui/lint/implicit_unsafe_autorefs.fixed index f24b79a452c9b..5f88f50fbca49 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.fixed +++ b/src/test/ui/lint/implicit_unsafe_autorefs.fixed @@ -1,15 +1,28 @@ // check-pass // run-rustfix +#![allow(dead_code)] use std::ptr::{addr_of, addr_of_mut}; -unsafe fn _test_mut(ptr: *mut [u8]) -> *mut [u8] { +unsafe fn test_mut(ptr: *mut [u8]) -> *mut [u8] { addr_of_mut!((&mut (*ptr))[..16]) //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } -unsafe fn _test_const(ptr: *const [u8]) -> *const [u8] { +unsafe fn test_const(ptr: *const [u8]) -> *const [u8] { addr_of!((&(*ptr))[..16]) //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } +struct Test { + field: [u8], +} + +unsafe fn test_field(ptr: *const Test) -> *const [u8] { + let l = (&(*ptr).field).len(); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer + + addr_of!((&(*ptr).field)[..l - 1]) + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.rs b/src/test/ui/lint/implicit_unsafe_autorefs.rs index 3e84901991647..7cee6d0fb69b3 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.rs +++ b/src/test/ui/lint/implicit_unsafe_autorefs.rs @@ -1,15 +1,28 @@ // check-pass // run-rustfix +#![allow(dead_code)] use std::ptr::{addr_of, addr_of_mut}; -unsafe fn _test_mut(ptr: *mut [u8]) -> *mut [u8] { +unsafe fn test_mut(ptr: *mut [u8]) -> *mut [u8] { addr_of_mut!((*ptr)[..16]) //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } -unsafe fn _test_const(ptr: *const [u8]) -> *const [u8] { +unsafe fn test_const(ptr: *const [u8]) -> *const [u8] { addr_of!((*ptr)[..16]) //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } +struct Test { + field: [u8], +} + +unsafe fn test_field(ptr: *const Test) -> *const [u8] { + let l = (*ptr).field.len(); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer + + addr_of!((*ptr).field[..l - 1]) + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.stderr b/src/test/ui/lint/implicit_unsafe_autorefs.stderr index 6790fb2cb543c..bdecf80f49a23 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.stderr +++ b/src/test/ui/lint/implicit_unsafe_autorefs.stderr @@ -1,5 +1,5 @@ warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:6:18 + --> $DIR/implicit_unsafe_autorefs.rs:7:18 | LL | addr_of_mut!((*ptr)[..16]) | ^^^^^^ @@ -12,7 +12,7 @@ LL | addr_of_mut!((&mut (*ptr))[..16]) | +++++ + warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:11:14 + --> $DIR/implicit_unsafe_autorefs.rs:12:14 | LL | addr_of!((*ptr)[..16]) | ^^^^^^ @@ -23,5 +23,29 @@ help: try using a raw pointer method instead; or if this reference is intentiona LL | addr_of!((&(*ptr))[..16]) | ++ + -warning: 2 warnings emitted +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:21:13 + | +LL | let l = (*ptr).field.len(); + | ^^^^^^^^^^^^ + | + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements +help: try using a raw pointer method instead; or if this reference is intentional, make it explicit + | +LL | let l = (&(*ptr).field).len(); + | ++ + + +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:24:14 + | +LL | addr_of!((*ptr).field[..l - 1]) + | ^^^^^^^^^^^^ + | + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements +help: try using a raw pointer method instead; or if this reference is intentional, make it explicit + | +LL | addr_of!((&(*ptr).field)[..l - 1]) + | ++ + + +warning: 4 warnings emitted From 45a6283ce15058357bc4da6bea8b9139927b2b26 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 14:23:41 +0000 Subject: [PATCH 06/18] `implicit_unsafe_autorefs`: more std fixes --- library/alloc/src/collections/btree/node.rs | 2 +- library/alloc/src/rc.rs | 4 +-- library/alloc/src/sync.rs | 2 +- library/std/src/sync/mpsc/mpsc_queue.rs | 12 ++++----- library/std/src/sync/mpsc/spsc_queue.rs | 27 ++++++++++----------- library/std/src/sync/mpsc/sync.rs | 2 +- library/std/src/thread/local.rs | 4 +-- 7 files changed, 26 insertions(+), 27 deletions(-) diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index 7325439c11a2f..2e8216dbd920b 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -544,7 +544,7 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> { fn set_parent_link(&mut self, parent: NonNull>, parent_idx: usize) { let leaf = Self::as_leaf_ptr(self); unsafe { (*leaf).parent = Some(parent) }; - unsafe { (*leaf).parent_idx.write(parent_idx as u16) }; + unsafe { (*leaf).parent_idx = MaybeUninit::new(parent_idx as u16) }; } } diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 006d813e5f9fa..6c9640a73d5d6 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -457,9 +457,9 @@ impl Rc { let inner = init_ptr.as_ptr(); ptr::write(ptr::addr_of_mut!((*inner).value), data); - let prev_value = (*inner).strong.get(); + let prev_value = (&(*inner).strong).get(); debug_assert_eq!(prev_value, 0, "No prior strong references should exist"); - (*inner).strong.set(1); + (&mut (*inner).strong).set(1); Rc::from_inner(init_ptr) }; diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 81cd770748854..f5fc60b4aa8c5 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -456,7 +456,7 @@ impl Arc { // // These side effects do not impact us in any way, and no other side effects are // possible with safe code alone. - let prev_value = (*inner).strong.fetch_add(1, Release); + let prev_value = (&(*inner).strong).fetch_add(1, Release); debug_assert_eq!(prev_value, 0, "No prior strong references should exist"); Arc::from_inner(init_ptr) diff --git a/library/std/src/sync/mpsc/mpsc_queue.rs b/library/std/src/sync/mpsc/mpsc_queue.rs index cdd64a5def506..ebdf2a9f0f936 100644 --- a/library/std/src/sync/mpsc/mpsc_queue.rs +++ b/library/std/src/sync/mpsc/mpsc_queue.rs @@ -70,7 +70,7 @@ impl Queue { unsafe { let n = Node::new(Some(t)); let prev = self.head.swap(n, Ordering::AcqRel); - (*prev).next.store(n, Ordering::Release); + (&(*prev).next).store(n, Ordering::Release); } } @@ -87,13 +87,13 @@ impl Queue { pub fn pop(&self) -> PopResult { unsafe { let tail = *self.tail.get(); - let next = (*tail).next.load(Ordering::Acquire); + let next = (&(*tail).next).load(Ordering::Acquire); if !next.is_null() { *self.tail.get() = next; - assert!((*tail).value.is_none()); - assert!((*next).value.is_some()); - let ret = (*next).value.take().unwrap(); + assert!((&(*tail).value).is_none()); + assert!((&(*next).value).is_some()); + let ret = (&mut (*next).value).take().unwrap(); let _: Box> = Box::from_raw(tail); return Data(ret); } @@ -108,7 +108,7 @@ impl Drop for Queue { unsafe { let mut cur = *self.tail.get(); while !cur.is_null() { - let next = (*cur).next.load(Ordering::Relaxed); + let next = (&(*cur).next).load(Ordering::Relaxed); let _: Box> = Box::from_raw(cur); cur = next; } diff --git a/library/std/src/sync/mpsc/spsc_queue.rs b/library/std/src/sync/mpsc/spsc_queue.rs index 7e745eb31de60..e5337f1b46a5e 100644 --- a/library/std/src/sync/mpsc/spsc_queue.rs +++ b/library/std/src/sync/mpsc/spsc_queue.rs @@ -102,7 +102,7 @@ impl Queue Self { let n1 = Node::new(); let n2 = Node::new(); - (*n1).next.store(n2, Ordering::Relaxed); + (&(*n1).next).store(n2, Ordering::Relaxed); Queue { consumer: CacheAligned::new(Consumer { tail: UnsafeCell::new(n2), @@ -127,10 +127,10 @@ impl Queue Queue Queue Queue Queue Queue Drop for Queue> = Box::from_raw(cur); cur = next; } diff --git a/library/std/src/sync/mpsc/sync.rs b/library/std/src/sync/mpsc/sync.rs index 733761671a041..8578b6033ea3e 100644 --- a/library/std/src/sync/mpsc/sync.rs +++ b/library/std/src/sync/mpsc/sync.rs @@ -489,7 +489,7 @@ impl Queue { } unsafe { (*node).next = ptr::null_mut(); - Some((*node).token.take().unwrap()) + Some((&mut (*node).token).take().unwrap()) } } } diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 4e6eae3e5aae5..bac491090b734 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -1029,8 +1029,8 @@ pub mod fast { // causes future calls to `get` to run `try_initialize_drop` again, // which will now fail, and return `None`. unsafe { - let value = (*ptr).inner.take(); - (*ptr).dtor_state.set(DtorState::RunningOrHasRun); + let value = (&mut (*ptr).inner).take(); + (&(*ptr).dtor_state).set(DtorState::RunningOrHasRun); drop(value); } } From 0efeae619907d2bfd8f784a7387da4bca8a6204d Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 15:48:26 +0000 Subject: [PATCH 07/18] `implicit_unsafe_autorefs`: compiler fixes --- compiler/rustc_arena/src/lib.rs | 7 ++++--- compiler/rustc_data_structures/src/owning_ref/mod.rs | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index 46dbbd83d1905..fb804650dd658 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -19,6 +19,7 @@ #![feature(pointer_byte_offsets)] #![feature(rustc_attrs)] #![cfg_attr(test, feature(test))] +#![feature(slice_ptr_len)] #![feature(strict_provenance)] #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] @@ -103,7 +104,7 @@ impl ArenaChunk { // A pointer as large as possible for zero-sized elements. ptr::invalid_mut(!0) } else { - self.start().add((*self.storage.as_ptr()).len()) + self.start().add(self.storage.as_ptr().len()) } } } @@ -287,7 +288,7 @@ impl TypedArena { // If the previous chunk's len is less than HUGE_PAGE // bytes, then this chunk will be least double the previous // chunk's size. - new_cap = (*last_chunk.storage.as_ptr()).len().min(HUGE_PAGE / elem_size / 2); + new_cap = last_chunk.storage.as_ptr().len().min(HUGE_PAGE / elem_size / 2); new_cap *= 2; } else { new_cap = PAGE / elem_size; @@ -395,7 +396,7 @@ impl DroplessArena { // If the previous chunk's len is less than HUGE_PAGE // bytes, then this chunk will be least double the previous // chunk's size. - new_cap = (*last_chunk.storage.as_ptr()).len().min(HUGE_PAGE / 2); + new_cap = last_chunk.storage.as_ptr().len().min(HUGE_PAGE / 2); new_cap *= 2; } else { new_cap = PAGE; diff --git a/compiler/rustc_data_structures/src/owning_ref/mod.rs b/compiler/rustc_data_structures/src/owning_ref/mod.rs index ed5e566184f12..896009434fb2c 100644 --- a/compiler/rustc_data_structures/src/owning_ref/mod.rs +++ b/compiler/rustc_data_structures/src/owning_ref/mod.rs @@ -1105,14 +1105,14 @@ use std::sync::{MutexGuard, RwLockReadGuard, RwLockWriteGuard}; impl ToHandle for RefCell { type Handle = Ref<'static, T>; unsafe fn to_handle(x: *const Self) -> Self::Handle { - (*x).borrow() + (&*x).borrow() } } impl ToHandleMut for RefCell { type HandleMut = RefMut<'static, T>; unsafe fn to_handle_mut(x: *const Self) -> Self::HandleMut { - (*x).borrow_mut() + (&*x).borrow_mut() } } From 459d6ad2c123764e3320ebe4f41eb4137473a3bb Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 17:33:49 +0000 Subject: [PATCH 08/18] strip trailing whitespace --- .../rustc_lint/src/implicit_unsafe_autorefs.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index 498197d045a1f..fc379d1bf3dbc 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -25,19 +25,19 @@ declare_lint! { /// since they inflict a lot of safety requirement. Unfortunately, it's possible /// to take a reference to a dereference of a raw pointer implicitly, 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) /// } @@ -54,7 +54,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { let typeck = cx.typeck_results(); let adjustments_table = typeck.adjustments(); - if let Some(adjustments) = adjustments_table.get(expr.hir_id) + if let Some(adjustments) = adjustments_table.get(expr.hir_id) && let [adjustment] = &**adjustments // An auto-borrow && let Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) = adjustment.kind @@ -64,17 +64,17 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { && 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 requires the pointer to be valid and imposes aliasing requirements") .multipart_suggestion( - "try using a raw pointer method instead; or if this reference is intentional, make it explicit", + "try using a raw pointer method instead; or 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 ) }) From ef427e6f63c253547a078199fb8784c1e379a62e Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 19:37:47 +0000 Subject: [PATCH 09/18] `implicit_unsafe_autorefs`: ui test fixes --- .../ui/borrowck/borrowck-unsafe-static-mutable-borrows.rs | 2 +- src/test/ui/dynamically-sized-types/dst-coerce-custom.rs | 6 +++--- src/test/ui/dynamically-sized-types/dst-raw.rs | 8 ++++---- .../ui/self/arbitrary_self_types_raw_pointer_struct.rs | 6 +++--- src/test/ui/union/issue-99375.rs | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/test/ui/borrowck/borrowck-unsafe-static-mutable-borrows.rs b/src/test/ui/borrowck/borrowck-unsafe-static-mutable-borrows.rs index adc7dfd541f48..c3840c142c5ec 100644 --- a/src/test/ui/borrowck/borrowck-unsafe-static-mutable-borrows.rs +++ b/src/test/ui/borrowck/borrowck-unsafe-static-mutable-borrows.rs @@ -13,7 +13,7 @@ impl Foo { fn main() { unsafe { let sfoo: *mut Foo = &mut SFOO; - let x = (*sfoo).x(); + let x = (&mut *sfoo).x(); (*sfoo).x[1] += 1; *x += 1; } diff --git a/src/test/ui/dynamically-sized-types/dst-coerce-custom.rs b/src/test/ui/dynamically-sized-types/dst-coerce-custom.rs index 24d83eb5343ec..07e5c503d6f73 100644 --- a/src/test/ui/dynamically-sized-types/dst-coerce-custom.rs +++ b/src/test/ui/dynamically-sized-types/dst-coerce-custom.rs @@ -3,14 +3,14 @@ #![feature(unsize, coerce_unsized)] -use std::ops::CoerceUnsized; use std::marker::Unsize; +use std::ops::CoerceUnsized; struct Bar { x: *const T, } -impl, U: ?Sized> CoerceUnsized> for Bar {} +impl, U: ?Sized> CoerceUnsized> for Bar {} trait Baz { fn get(&self) -> i32; @@ -38,6 +38,6 @@ fn main() { let a: Bar = Bar { x: &42 }; let b: Bar = a; unsafe { - assert_eq!((*b.x).get(), 42); + assert_eq!((&*b.x).get(), 42); } } diff --git a/src/test/ui/dynamically-sized-types/dst-raw.rs b/src/test/ui/dynamically-sized-types/dst-raw.rs index 0893b02e74e82..5fc024e8f2460 100644 --- a/src/test/ui/dynamically-sized-types/dst-raw.rs +++ b/src/test/ui/dynamically-sized-types/dst-raw.rs @@ -51,7 +51,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } @@ -60,7 +60,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } @@ -108,7 +108,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } @@ -116,7 +116,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } diff --git a/src/test/ui/self/arbitrary_self_types_raw_pointer_struct.rs b/src/test/ui/self/arbitrary_self_types_raw_pointer_struct.rs index 0eab7617f7a75..ad86e02317a40 100644 --- a/src/test/ui/self/arbitrary_self_types_raw_pointer_struct.rs +++ b/src/test/ui/self/arbitrary_self_types_raw_pointer_struct.rs @@ -7,7 +7,7 @@ struct Foo(String); impl Foo { unsafe fn foo(self: *const Self) -> *const str { - (*self).0.as_ref() + (&(*self).0).as_ref() } fn complicated_1(self: *const Rc) -> &'static str { @@ -15,7 +15,7 @@ impl Foo { } unsafe fn complicated_2(self: Rc<*const Self>) -> *const str { - (**self).0.as_ref() + (&(**self).0).as_ref() } } @@ -24,5 +24,5 @@ fn main() { assert_eq!("abc123", unsafe { &*(&foo as *const Foo).foo() }); assert_eq!("Foo::complicated_1", std::ptr::null::>().complicated_1()); let rc = Rc::new(&foo as *const Foo); - assert_eq!("abc123", unsafe { &*rc.complicated_2()}); + assert_eq!("abc123", unsafe { &*rc.complicated_2() }); } diff --git a/src/test/ui/union/issue-99375.rs b/src/test/ui/union/issue-99375.rs index 175018a7d71a7..fff84770e9134 100644 --- a/src/test/ui/union/issue-99375.rs +++ b/src/test/ui/union/issue-99375.rs @@ -15,7 +15,7 @@ where R: Copy, F: Fn() -> R, { - (*params).result.init = ((*params).function)(); + (*params).result.init = (&(*params).function)(); } fn main() {} From e53055bc6043746aa09212a1210d95200c9f1c6e Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 19:48:24 +0000 Subject: [PATCH 10/18] Apply suggestion from code review --- library/alloc/src/collections/btree/node.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index 2e8216dbd920b..fa1408581ae4d 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -1699,7 +1699,7 @@ unsafe fn slice_insert(slice: &mut [MaybeUninit], 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).cast::().write(val); + (&mut *slice_ptr.add(idx)).write(val); } } @@ -1713,7 +1713,7 @@ unsafe fn slice_remove(slice: &mut [MaybeUninit], idx: usize) -> T { let len = slice.len(); debug_assert!(idx < len); let slice_ptr = slice.as_mut_ptr(); - let ret = slice_ptr.add(idx).cast::().read(); + let ret = (&*slice_ptr.add(idx)).assume_init_read(); ptr::copy(slice_ptr.add(idx + 1), slice_ptr.add(idx), len - idx - 1); ret } From 6839707760f7eac8ec767198196959559150dea5 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 20:27:06 +0000 Subject: [PATCH 11/18] `implicit_unsafe_autorefs`: fix clippy --- src/tools/clippy/tests/ui/cast_ref_to_mut.rs | 4 +-- .../clippy/tests/ui/cast_ref_to_mut.stderr | 30 ++++++++++++++----- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/tools/clippy/tests/ui/cast_ref_to_mut.rs b/src/tools/clippy/tests/ui/cast_ref_to_mut.rs index c48a734ba32c2..bafbfbffb241e 100644 --- a/src/tools/clippy/tests/ui/cast_ref_to_mut.rs +++ b/src/tools/clippy/tests/ui/cast_ref_to_mut.rs @@ -15,9 +15,9 @@ fn main() { let num = &3i32; let mut_num = &mut 3i32; // Should be warned against - (*(a as *const _ as *mut String)).push_str(" world"); + (&mut *(a as *const _ as *mut String)).push_str(" world"); *(a as *const _ as *mut _) = String::from("Replaced"); - *(a as *const _ as *mut String) += " world"; + *&mut *(a as *const _ as *mut String) += " world"; // Shouldn't be warned against println!("{}", *(num as *const _ as *const i16)); println!("{}", *(mut_num as *mut _ as *mut i16)); diff --git a/src/tools/clippy/tests/ui/cast_ref_to_mut.stderr b/src/tools/clippy/tests/ui/cast_ref_to_mut.stderr index aacd99437d9fc..658d6af930ae9 100644 --- a/src/tools/clippy/tests/ui/cast_ref_to_mut.stderr +++ b/src/tools/clippy/tests/ui/cast_ref_to_mut.stderr @@ -1,11 +1,27 @@ +error: immediately dereferencing a reference + --> $DIR/cast_ref_to_mut.rs:20:9 + | +LL | *&mut *(a as *const _ as *mut String) += " world"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `*(a as *const _ as *mut String)` + | + = note: `-D clippy::deref-addrof` implied by `-D warnings` + error: casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell` - --> $DIR/cast_ref_to_mut.rs:18:9 + --> $DIR/cast_ref_to_mut.rs:18:15 | -LL | (*(a as *const _ as *mut String)).push_str(" world"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | (&mut *(a as *const _ as *mut String)).push_str(" world"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::cast-ref-to-mut` implied by `-D warnings` +error: this expression borrows a value the compiler would automatically borrow + --> $DIR/cast_ref_to_mut.rs:18:9 + | +LL | (&mut *(a as *const _ as *mut String)).push_str(" world"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: change this to: `(*(a as *const _ as *mut String))` + | + = note: `-D clippy::needless-borrow` implied by `-D warnings` + error: casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell` --> $DIR/cast_ref_to_mut.rs:19:9 | @@ -13,10 +29,10 @@ LL | *(a as *const _ as *mut _) = String::from("Replaced"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell` - --> $DIR/cast_ref_to_mut.rs:20:9 + --> $DIR/cast_ref_to_mut.rs:20:15 | -LL | *(a as *const _ as *mut String) += " world"; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | *&mut *(a as *const _ as *mut String) += " world"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: aborting due to 5 previous errors From 7b280e29323fdd9531db0bf71a285323064ba1a9 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 21:57:59 +0000 Subject: [PATCH 12/18] `implicit_unsafe_autorefs`: fix doctests --- compiler/rustc_lint/src/implicit_unsafe_autorefs.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index fc379d1bf3dbc..234d812b70f9e 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -10,6 +10,8 @@ declare_lint! { /// ### Example /// /// ```rust + /// use std::ptr::addr_of_mut; + /// /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] { /// addr_of_mut!((*ptr)[..16]) /// // ^^^^^^ this calls `IndexMut::index_mut(&mut ..., ..16)`, @@ -28,6 +30,7 @@ declare_lint! { /// /// If you are sure, you can soundly take a reference, then you can take it explicitly: /// ```rust + /// # use std::ptr::addr_of_mut; /// unsafe fn fun(ptr: *mut [u8]) -> *mut [u8] { /// addr_of_mut!((&mut *ptr)[..16]) /// } From dda0fef684815be5615323f096806b50b27e1b69 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 21:58:29 +0000 Subject: [PATCH 13/18] `implicit_unsafe_autorefs`: fix std tests --- library/alloc/src/sync/tests.rs | 20 +++++++------------- library/alloc/src/tests.rs | 4 ++-- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index 0fae8953aa2c7..d2ce34670a4f3 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -16,17 +16,11 @@ use std::thread; use crate::vec::Vec; -struct Canary(*mut atomic::AtomicUsize); +struct Canary<'a>(&'a atomic::AtomicUsize); -impl Drop for Canary { +impl Drop for Canary<'_> { fn drop(&mut self) { - unsafe { - match *self { - Canary(c) => { - (*c).fetch_add(1, SeqCst); - } - } - } + self.0.fetch_add(1, SeqCst); } } @@ -272,16 +266,16 @@ fn weak_self_cyclic() { #[test] fn drop_arc() { - let mut canary = atomic::AtomicUsize::new(0); - let x = Arc::new(Canary(&mut canary as *mut atomic::AtomicUsize)); + let canary = atomic::AtomicUsize::new(0); + let x = Arc::new(Canary(&canary)); drop(x); assert!(canary.load(Acquire) == 1); } #[test] fn drop_arc_weak() { - let mut canary = atomic::AtomicUsize::new(0); - let arc = Arc::new(Canary(&mut canary as *mut atomic::AtomicUsize)); + let canary = atomic::AtomicUsize::new(0); + let arc = Arc::new(Canary(&canary)); let arc_weak = Arc::downgrade(&arc); assert!(canary.load(Acquire) == 0); drop(arc); diff --git a/library/alloc/src/tests.rs b/library/alloc/src/tests.rs index 299ed156a5d27..8e55ab7ce561e 100644 --- a/library/alloc/src/tests.rs +++ b/library/alloc/src/tests.rs @@ -102,8 +102,8 @@ fn raw_trait() { let x: Box = Box::new(Bar(17)); let p = Box::into_raw(x); unsafe { - assert_eq!(17, (*p).get()); - (*p).set(19); + assert_eq!(17, (&*p).get()); + (&mut *p).set(19); let y: Box = Box::from_raw(p); assert_eq!(19, y.get()); } From 94ad6c837880d957629d4ebaf55f593995d04530 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 21:58:47 +0000 Subject: [PATCH 14/18] `implicit_unsafe_autorefs`: fix rust-analyzer bridge --- .../src/abis/abi_1_58/proc_macro/bridge/closure.rs | 2 +- .../src/abis/abi_1_63/proc_macro/bridge/closure.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_58/proc_macro/bridge/closure.rs b/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_58/proc_macro/bridge/closure.rs index 5be71cc3d7013..61ffea98871c7 100644 --- a/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_58/proc_macro/bridge/closure.rs +++ b/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_58/proc_macro/bridge/closure.rs @@ -11,7 +11,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 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::, env: unsafe { &mut *(f as *mut _ as *mut Env) } } } diff --git a/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_63/proc_macro/bridge/closure.rs b/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_63/proc_macro/bridge/closure.rs index d371ae3cea098..1255f35d417e4 100644 --- a/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_63/proc_macro/bridge/closure.rs +++ b/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_1_63/proc_macro/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 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::, env: f as *mut _ as *mut Env, _marker: PhantomData } } From c380f3e887a3235c76c6723774476005188b97e1 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 2 Nov 2022 23:04:59 +0000 Subject: [PATCH 15/18] `implicit_unsafe_autorefs`: fix miri tests too --- src/tools/miri/tests/fail/box-cell-alias.rs | 2 +- src/tools/miri/tests/fail/box-cell-alias.stderr | 4 ++-- .../miri/tests/fail/weak_memory/racing_mixed_size_read.rs | 2 +- .../tests/fail/weak_memory/racing_mixed_size_read.stderr | 4 ++-- src/tools/miri/tests/pass/dst-raw.rs | 8 ++++---- .../tests/pass/stacked-borrows/interior_mutability.rs | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/tools/miri/tests/fail/box-cell-alias.rs b/src/tools/miri/tests/fail/box-cell-alias.rs index 1a4a3b97ea3ff..5a73dd5719952 100644 --- a/src/tools/miri/tests/fail/box-cell-alias.rs +++ b/src/tools/miri/tests/fail/box-cell-alias.rs @@ -6,7 +6,7 @@ use std::cell::Cell; fn helper(val: Box>, ptr: *const Cell) -> u8 { val.set(10); - unsafe { (*ptr).set(20) }; //~ ERROR: does not exist in the borrow stack + unsafe { (&*ptr).set(20) }; //~ ERROR: does not exist in the borrow stack val.get() } diff --git a/src/tools/miri/tests/fail/box-cell-alias.stderr b/src/tools/miri/tests/fail/box-cell-alias.stderr index 8370163997687..52db84a83c05c 100644 --- a/src/tools/miri/tests/fail/box-cell-alias.stderr +++ b/src/tools/miri/tests/fail/box-cell-alias.stderr @@ -1,8 +1,8 @@ error: Undefined Behavior: trying to retag from for SharedReadWrite permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location --> $DIR/box-cell-alias.rs:LL:CC | -LL | unsafe { (*ptr).set(20) }; - | ^^^^^^^^^^^^^^ +LL | unsafe { (&*ptr).set(20) }; + | ^^^^^^^ | | | trying to retag from for SharedReadWrite permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag at ALLOC[0x0..0x1] diff --git a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.rs b/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.rs index 73178980b7e5a..3c02b320356d0 100644 --- a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.rs +++ b/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.rs @@ -29,7 +29,7 @@ pub fn main() { let x_split = split_u32_ptr(x_ptr); unsafe { let hi = x_split as *const u16 as *const AtomicU16; - (*hi).load(Relaxed); //~ ERROR: imperfectly overlapping + (&*hi).load(Relaxed); //~ ERROR: imperfectly overlapping } }); diff --git a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.stderr b/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.stderr index 59fa5c7410237..f0a35f053855d 100644 --- a/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.stderr +++ b/src/tools/miri/tests/fail/weak_memory/racing_mixed_size_read.stderr @@ -1,8 +1,8 @@ error: unsupported operation: racy imperfectly overlapping atomic access is not possible in the C++20 memory model, and not supported by Miri's weak memory emulation --> $DIR/racing_mixed_size_read.rs:LL:CC | -LL | (*hi).load(Relaxed); - | ^^^^^^^^^^^^^^^^^^^ racy imperfectly overlapping atomic access is not possible in the C++20 memory model, and not supported by Miri's weak memory emulation +LL | (&*hi).load(Relaxed); + | ^^^^^^^^^^^^^^^^^^^^ racy imperfectly overlapping atomic access is not possible in the C++20 memory model, and not supported by Miri's weak memory emulation | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/src/tools/miri/tests/pass/dst-raw.rs b/src/tools/miri/tests/pass/dst-raw.rs index f26191a1d5998..935790235fa24 100644 --- a/src/tools/miri/tests/pass/dst-raw.rs +++ b/src/tools/miri/tests/pass/dst-raw.rs @@ -35,7 +35,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } @@ -44,7 +44,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } @@ -72,7 +72,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } @@ -80,7 +80,7 @@ pub fn main() { unsafe { let b = (*a)[2]; assert_eq!(b, 3); - let len = (*a).len(); + let len = (&*a).len(); assert_eq!(len, 3); } diff --git a/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs b/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs index c6373a7eaf135..682d59d394b47 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs @@ -70,7 +70,7 @@ fn unsafe_cell_2phase() { unsafe { let x = &UnsafeCell::new(vec![]); let x2 = &*x; - (*x.get()).push(0); + (&mut *x.get()).push(0); let _val = (*x2.get()).get(0); } } From 274eba7e642cc5771ddaed5345a0e96caf405251 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 4 Nov 2022 14:24:44 +0000 Subject: [PATCH 16/18] `implicit_unsafe_autorefs` support built-in index places --- .../src/implicit_unsafe_autorefs.rs | 30 +++++++++++++++---- .../ui/lint/implicit_unsafe_autorefs.fixed | 10 +++++++ src/test/ui/lint/implicit_unsafe_autorefs.rs | 10 +++++++ .../ui/lint/implicit_unsafe_autorefs.stderr | 26 +++++++++++++++- 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index 234d812b70f9e..7637afc059d98 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -2,7 +2,10 @@ 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}; +use rustc_middle::ty::{ + adjustment::{Adjust, AutoBorrow}, + TyCtxt, TypeckResults, +}; declare_lint! { /// The `implicit_unsafe_autorefs` lint checks for implicitly taken references to dereferences of raw pointers. @@ -62,7 +65,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { // An auto-borrow && let Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) = adjustment.kind // ... of a place derived from a deref - && let ExprKind::Unary(UnOp::Deref, dereferenced) = skip_field_access(&expr.kind) + && let ExprKind::Unary(UnOp::Deref, dereferenced) = peel_place_mappers(cx.tcx, typeck, &expr).kind // ... of a raw pointer && typeck.expr_ty(dereferenced).is_unsafe_ptr() { @@ -85,9 +88,24 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { } } -fn skip_field_access<'a>(mut expr: &'a ExprKind<'a>) -> &'a ExprKind<'a> { - while let ExprKind::Field(e, _) = expr { - expr = &e.kind; +/// Peels expressions from `expr` that can map a place. +/// +/// For example `(*ptr).field[0]/*<-- built-in index */.field` -> `*ptr`, `f(*ptr)` -> `f(*ptr)`, etc. +fn peel_place_mappers<'tcx>( + tcx: TyCtxt<'tcx>, + typeck: &TypeckResults<'tcx>, + mut expr: &'tcx Expr<'tcx>, +) -> &'tcx Expr<'tcx> { + loop { + match expr.kind { + ExprKind::Index(base, idx) + if typeck.expr_ty(base).builtin_index() == Some(typeck.expr_ty(expr)) + && typeck.expr_ty(idx) == tcx.types.usize => + { + expr = &base; + } + ExprKind::Field(e, _) => expr = &e, + _ => break expr, + } } - expr } diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.fixed b/src/test/ui/lint/implicit_unsafe_autorefs.fixed index 5f88f50fbca49..9b7e5cad5efba 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.fixed +++ b/src/test/ui/lint/implicit_unsafe_autorefs.fixed @@ -25,4 +25,14 @@ unsafe fn test_field(ptr: *const Test) -> *const [u8] { //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } +unsafe fn test_builtin_index(a: *mut [String]) { + // built-in (should warn before `.len()`) + _ = (&(*a)[0]).len(); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer + + // overloaded (should warn before index) + _ = (&(*a))[..1][0].len(); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.rs b/src/test/ui/lint/implicit_unsafe_autorefs.rs index 7cee6d0fb69b3..600a89ad9a74d 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.rs +++ b/src/test/ui/lint/implicit_unsafe_autorefs.rs @@ -25,4 +25,14 @@ unsafe fn test_field(ptr: *const Test) -> *const [u8] { //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } +unsafe fn test_builtin_index(a: *mut [String]) { + // built-in (should warn before `.len()`) + _ = (*a)[0].len(); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer + + // overloaded (should warn before index) + _ = (*a)[..1][0].len(); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.stderr b/src/test/ui/lint/implicit_unsafe_autorefs.stderr index bdecf80f49a23..b8e05c3801f4a 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.stderr +++ b/src/test/ui/lint/implicit_unsafe_autorefs.stderr @@ -47,5 +47,29 @@ help: try using a raw pointer method instead; or if this reference is intentiona LL | addr_of!((&(*ptr).field)[..l - 1]) | ++ + -warning: 4 warnings emitted +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:30:9 + | +LL | _ = (*a)[0].len(); + | ^^^^^^^ + | + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements +help: try using a raw pointer method instead; or if this reference is intentional, make it explicit + | +LL | _ = (&(*a)[0]).len(); + | ++ + + +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:34:9 + | +LL | _ = (*a)[..1][0].len(); + | ^^^^ + | + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements +help: try using a raw pointer method instead; or if this reference is intentional, make it explicit + | +LL | _ = (&(*a))[..1][0].len(); + | ++ + + +warning: 6 warnings emitted From 356f20192f0ce561bb9f08371e827897294535fb Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 4 Nov 2022 14:47:30 +0000 Subject: [PATCH 17/18] `implicit_unsafe_autorefs`: support overloaded deref --- .../src/implicit_unsafe_autorefs.rs | 17 +++++++-- .../ui/lint/implicit_unsafe_autorefs.fixed | 11 ++++++ src/test/ui/lint/implicit_unsafe_autorefs.rs | 11 ++++++ .../ui/lint/implicit_unsafe_autorefs.stderr | 38 +++++++++++++++---- 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index 7637afc059d98..979f1aab9b40b 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -1,9 +1,9 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_errors::Applicability; -use rustc_hir::{self as hir, Expr, ExprKind, UnOp}; +use rustc_hir::{Expr, ExprKind, Mutability, UnOp}; use rustc_middle::ty::{ - adjustment::{Adjust, AutoBorrow}, + adjustment::{Adjust, Adjustment, AutoBorrow, OverloadedDeref}, TyCtxt, TypeckResults, }; @@ -63,13 +63,13 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { if let Some(adjustments) = adjustments_table.get(expr.hir_id) && let [adjustment] = &**adjustments // An auto-borrow - && let Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) = adjustment.kind + && let Some(mutbl) = has_implicit_borrow(adjustment) // ... of a place derived from a deref && let ExprKind::Unary(UnOp::Deref, dereferenced) = peel_place_mappers(cx.tcx, typeck, &expr).kind // ... of a raw pointer && typeck.expr_ty(dereferenced).is_unsafe_ptr() { - let mutbl = hir::Mutability::prefix_str(&mutbl.into()); + let mutbl = 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| { @@ -109,3 +109,12 @@ fn peel_place_mappers<'tcx>( } } } + +/// Returns `Some(mutability)` if the argument adjustment has implicit borrow in it. +fn has_implicit_borrow(Adjustment { kind, .. }: &Adjustment<'_>) -> Option { + match kind { + &Adjust::Deref(Some(OverloadedDeref { mutbl, .. })) => Some(mutbl), + &Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) => Some(mutbl.into()), + _ => None, + } +} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.fixed b/src/test/ui/lint/implicit_unsafe_autorefs.fixed index 9b7e5cad5efba..c8a615f0d7881 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.fixed +++ b/src/test/ui/lint/implicit_unsafe_autorefs.fixed @@ -1,6 +1,7 @@ // check-pass // run-rustfix #![allow(dead_code)] +use std::mem::ManuallyDrop; use std::ptr::{addr_of, addr_of_mut}; unsafe fn test_mut(ptr: *mut [u8]) -> *mut [u8] { @@ -35,4 +36,14 @@ unsafe fn test_builtin_index(a: *mut [String]) { //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } +unsafe fn test_overloaded_deref_const(ptr: *const ManuallyDrop) { + _ = addr_of!((&(*ptr)).field); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + +unsafe fn test_overloaded_deref_mut(ptr: *mut ManuallyDrop) { + _ = addr_of_mut!((&mut (*ptr)).field); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.rs b/src/test/ui/lint/implicit_unsafe_autorefs.rs index 600a89ad9a74d..43752a2798abb 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.rs +++ b/src/test/ui/lint/implicit_unsafe_autorefs.rs @@ -1,6 +1,7 @@ // check-pass // run-rustfix #![allow(dead_code)] +use std::mem::ManuallyDrop; use std::ptr::{addr_of, addr_of_mut}; unsafe fn test_mut(ptr: *mut [u8]) -> *mut [u8] { @@ -35,4 +36,14 @@ unsafe fn test_builtin_index(a: *mut [String]) { //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } +unsafe fn test_overloaded_deref_const(ptr: *const ManuallyDrop) { + _ = addr_of!((*ptr).field); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + +unsafe fn test_overloaded_deref_mut(ptr: *mut ManuallyDrop) { + _ = addr_of_mut!((*ptr).field); + //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer +} + fn main() {} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.stderr b/src/test/ui/lint/implicit_unsafe_autorefs.stderr index b8e05c3801f4a..a9936705e9965 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.stderr +++ b/src/test/ui/lint/implicit_unsafe_autorefs.stderr @@ -1,5 +1,5 @@ warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:7:18 + --> $DIR/implicit_unsafe_autorefs.rs:8:18 | LL | addr_of_mut!((*ptr)[..16]) | ^^^^^^ @@ -12,7 +12,7 @@ LL | addr_of_mut!((&mut (*ptr))[..16]) | +++++ + warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:12:14 + --> $DIR/implicit_unsafe_autorefs.rs:13:14 | LL | addr_of!((*ptr)[..16]) | ^^^^^^ @@ -24,7 +24,7 @@ LL | addr_of!((&(*ptr))[..16]) | ++ + warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:21:13 + --> $DIR/implicit_unsafe_autorefs.rs:22:13 | LL | let l = (*ptr).field.len(); | ^^^^^^^^^^^^ @@ -36,7 +36,7 @@ LL | let l = (&(*ptr).field).len(); | ++ + warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:24:14 + --> $DIR/implicit_unsafe_autorefs.rs:25:14 | LL | addr_of!((*ptr).field[..l - 1]) | ^^^^^^^^^^^^ @@ -48,7 +48,7 @@ LL | addr_of!((&(*ptr).field)[..l - 1]) | ++ + warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:30:9 + --> $DIR/implicit_unsafe_autorefs.rs:31:9 | LL | _ = (*a)[0].len(); | ^^^^^^^ @@ -60,7 +60,7 @@ LL | _ = (&(*a)[0]).len(); | ++ + warning: implicit auto-ref creates a reference to a dereference of a raw pointer - --> $DIR/implicit_unsafe_autorefs.rs:34:9 + --> $DIR/implicit_unsafe_autorefs.rs:35:9 | LL | _ = (*a)[..1][0].len(); | ^^^^ @@ -71,5 +71,29 @@ help: try using a raw pointer method instead; or if this reference is intentiona LL | _ = (&(*a))[..1][0].len(); | ++ + -warning: 6 warnings emitted +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:40:18 + | +LL | _ = addr_of!((*ptr).field); + | ^^^^^^ + | + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements +help: try using a raw pointer method instead; or if this reference is intentional, make it explicit + | +LL | _ = addr_of!((&(*ptr)).field); + | ++ + + +warning: implicit auto-ref creates a reference to a dereference of a raw pointer + --> $DIR/implicit_unsafe_autorefs.rs:45:22 + | +LL | _ = addr_of_mut!((*ptr).field); + | ^^^^^^ + | + = note: creating a reference requires the pointer to be valid and imposes aliasing requirements +help: try using a raw pointer method instead; or if this reference is intentional, make it explicit + | +LL | _ = addr_of_mut!((&mut (*ptr)).field); + | +++++ + + +warning: 8 warnings emitted From 6b238e6fbfabaee676911fadd25f878cf09904bb Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 7 Nov 2022 16:27:56 +0000 Subject: [PATCH 18/18] `implicit_unsafe_autorefs`: note the reason why reference is created --- .../src/implicit_unsafe_autorefs.rs | 51 ++++++++++++++++--- .../ui/lint/implicit_unsafe_autorefs.stderr | 8 +++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs index 979f1aab9b40b..41cc395e9175c 100644 --- a/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs +++ b/compiler/rustc_lint/src/implicit_unsafe_autorefs.rs @@ -1,7 +1,7 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, Mutability, UnOp}; +use rustc_hir::{self as hir, Expr, ExprKind, Mutability, UnOp}; use rustc_middle::ty::{ adjustment::{Adjust, Adjustment, AutoBorrow, OverloadedDeref}, TyCtxt, TypeckResults, @@ -63,7 +63,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { if let Some(adjustments) = adjustments_table.get(expr.hir_id) && let [adjustment] = &**adjustments // An auto-borrow - && let Some(mutbl) = has_implicit_borrow(adjustment) + && let Some((mutbl, implicit_borrow)) = has_implicit_borrow(adjustment) // ... of a place derived from a deref && let ExprKind::Unary(UnOp::Deref, dereferenced) = peel_place_mappers(cx.tcx, typeck, &expr).kind // ... of a raw pointer @@ -74,7 +74,13 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitUnsafeAutorefs { 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 requires the pointer to be valid and imposes aliasing requirements") + .note("creating a reference requires the pointer to be valid and imposes aliasing requirements"); + + if let Some(reason) = reason(cx.tcx, implicit_borrow, expr) { + lint.note(format!("a reference is implicitly created {reason}")); + } + + lint .multipart_suggestion( "try using a raw pointer method instead; or if this reference is intentional, make it explicit", vec![ @@ -111,10 +117,43 @@ fn peel_place_mappers<'tcx>( } /// Returns `Some(mutability)` if the argument adjustment has implicit borrow in it. -fn has_implicit_borrow(Adjustment { kind, .. }: &Adjustment<'_>) -> Option { +fn has_implicit_borrow( + Adjustment { kind, .. }: &Adjustment<'_>, +) -> Option<(Mutability, ImplicitBorrow)> { match kind { - &Adjust::Deref(Some(OverloadedDeref { mutbl, .. })) => Some(mutbl), - &Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) => Some(mutbl.into()), + &Adjust::Deref(Some(OverloadedDeref { mutbl, .. })) => Some((mutbl, ImplicitBorrow::Deref)), + &Adjust::Borrow(AutoBorrow::Ref(_, mutbl)) => Some((mutbl.into(), ImplicitBorrow::Borrow)), _ => None, } } + +enum ImplicitBorrow { + Deref, + Borrow, +} + +fn reason(tcx: TyCtxt<'_>, borrow_kind: ImplicitBorrow, expr: &Expr<'_>) -> Option<&'static str> { + match borrow_kind { + ImplicitBorrow::Deref => Some("because a deref coercion is being applied"), + ImplicitBorrow::Borrow => { + let parent = tcx.hir().get(tcx.hir().find_parent_node(expr.hir_id)?); + + let hir::Node::Expr(expr) = parent else { + return None + }; + + let reason = match expr.kind { + ExprKind::MethodCall(_, _, _, _) => "to match the method receiver type", + ExprKind::AssignOp(_, _, _) => { + "because a user-defined assignment with an operator is being used" + } + ExprKind::Index(_, _) => { + "because a user-defined indexing operation is being called" + } + _ => return None, + }; + + Some(reason) + } + } +} diff --git a/src/test/ui/lint/implicit_unsafe_autorefs.stderr b/src/test/ui/lint/implicit_unsafe_autorefs.stderr index a9936705e9965..cee0c5256dfbf 100644 --- a/src/test/ui/lint/implicit_unsafe_autorefs.stderr +++ b/src/test/ui/lint/implicit_unsafe_autorefs.stderr @@ -5,6 +5,7 @@ LL | addr_of_mut!((*ptr)[..16]) | ^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created because a user-defined indexing operation is being called = note: `#[warn(implicit_unsafe_autorefs)]` on by default help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | @@ -18,6 +19,7 @@ LL | addr_of!((*ptr)[..16]) | ^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created because a user-defined indexing operation is being called help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | addr_of!((&(*ptr))[..16]) @@ -30,6 +32,7 @@ LL | let l = (*ptr).field.len(); | ^^^^^^^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created to match the method receiver type help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | let l = (&(*ptr).field).len(); @@ -42,6 +45,7 @@ LL | addr_of!((*ptr).field[..l - 1]) | ^^^^^^^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created because a user-defined indexing operation is being called help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | addr_of!((&(*ptr).field)[..l - 1]) @@ -54,6 +58,7 @@ LL | _ = (*a)[0].len(); | ^^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created to match the method receiver type help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | _ = (&(*a)[0]).len(); @@ -66,6 +71,7 @@ LL | _ = (*a)[..1][0].len(); | ^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created because a user-defined indexing operation is being called help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | _ = (&(*a))[..1][0].len(); @@ -78,6 +84,7 @@ LL | _ = addr_of!((*ptr).field); | ^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created because a deref coercion is being applied help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | _ = addr_of!((&(*ptr)).field); @@ -90,6 +97,7 @@ LL | _ = addr_of_mut!((*ptr).field); | ^^^^^^ | = note: creating a reference requires the pointer to be valid and imposes aliasing requirements + = note: a reference is implicitly created because a deref coercion is being applied help: try using a raw pointer method instead; or if this reference is intentional, make it explicit | LL | _ = addr_of_mut!((&mut (*ptr)).field);