Rollup merge of #159960 - Urgau:invalid_ref-safe-interior-mut, r=mejrs

Allow `UnsafeCell` content access without `get` in `invalid_reference_casting` lint

#159730 relaxed the rules so that it's no longer necessary to use `UnsafeCell::raw_get` or `UnsafeCell::get` to access the content of an unsafe cell.

As a consequence this means that code this like is no longer invalid:
```rust
use std::cell::UnsafeCell;

unsafe fn get_mut_unchecked<T>(ptr: &UnsafeCell<T>) -> &mut T {
    let t = ptr as *const UnsafeCell<T> as *mut T;
    unsafe { &mut *t }
}
```

Fixes #159915
cc @RalfJung

Author: jhpratt

compiler/rustc_lint/src/dangling.rs

@@ -179,7 +179,7 @@ fn lint_addr_of_local<'a>(
     expr: &'a Expr<'a>,
 ) {
     // peel casts as they do not interest us here, we want the inner expression.
-    let (inner, _) = super::utils::peel_casts(cx, expr);
+    let inner = super::utils::peel_casts(cx, expr);
 
     if let ExprKind::AddrOf(_, _, inner_of) = inner.kind
         && let ExprKind::Path(ref qpath) = inner_of.peel_blocks().kind

compiler/rustc_lint/src/lints.rs

@@ -993,10 +993,6 @@ pub(crate) enum InvalidReferenceCastingDiag<'tcx> {
     BorrowAsMut {
         #[label("casting happened here")]
         orig_cast: Option<Span>,
-        #[note(
-            "even for types with interior mutability, the only legal way to obtain a mutable pointer from a shared reference is through `UnsafeCell::get`"
-        )]
-        ty_has_interior_mutability: bool,
     },
     #[diag("assigning to `&T` is undefined behavior, consider using an `UnsafeCell`")]
     #[note(
@@ -1005,10 +1001,6 @@ pub(crate) enum InvalidReferenceCastingDiag<'tcx> {
     AssignToRef {
         #[label("casting happened here")]
         orig_cast: Option<Span>,
-        #[note(
-            "even for types with interior mutability, the only legal way to obtain a mutable pointer from a shared reference is through `UnsafeCell::get`"
-        )]
-        ty_has_interior_mutability: bool,
     },
     #[diag(
         "casting references to a bigger memory layout than the backing allocation is undefined behavior, even if the reference is unused"

compiler/rustc_lint/src/ptr_nulls.rs

@@ -112,7 +112,7 @@ fn useless_check<'a, 'tcx: 'a>(
 
 /// Checks if the given expression is a null pointer (modulo casting)
 fn is_null_ptr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Span> {
-    let (expr, _) = peel_casts(cx, expr);
+    let expr = peel_casts(cx, expr);
 
     if let ExprKind::Call(path, []) = expr.kind
         && let ExprKind::Path(ref qpath) = path.kind

compiler/rustc_lint/src/reference_casting.rs

@@ -52,22 +52,15 @@ impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting {
             };
 
             if matches!(pat, PatternKind::Borrow { mutbl: Mutability::Mut } | PatternKind::Assign)
-                && let Some(ty_has_interior_mutability) =
-                    is_cast_from_ref_to_mut_ptr(cx, init, &mut peel_casts)
+                && is_invalid_cast_from_ref_to_mut_ptr(cx, init, &mut peel_casts)
             {
                 cx.emit_span_lint(
                     INVALID_REFERENCE_CASTING,
                     expr.span,
                     if pat == PatternKind::Assign {
-                        InvalidReferenceCastingDiag::AssignToRef {
-                            orig_cast,
-                            ty_has_interior_mutability,
-                        }
+                        InvalidReferenceCastingDiag::AssignToRef { orig_cast }
                     } else {
-                        InvalidReferenceCastingDiag::BorrowAsMut {
-                            orig_cast,
-                            ty_has_interior_mutability,
-                        }
+                        InvalidReferenceCastingDiag::BorrowAsMut { orig_cast }
                     },
                 );
             }
@@ -146,49 +139,51 @@ fn borrow_or_assign<'tcx>(
     deref_assign_or_addr_of(e).or_else(|| ptr_write(cx, e))
 }
 
-fn is_cast_from_ref_to_mut_ptr<'tcx>(
+fn is_invalid_cast_from_ref_to_mut_ptr<'tcx>(
     cx: &LateContext<'tcx>,
     orig_expr: &'tcx Expr<'tcx>,
-    mut peel_casts: impl FnMut() -> (&'tcx Expr<'tcx>, bool),
-) -> Option<bool> {
+    mut peel_casts: impl FnMut() -> &'tcx Expr<'tcx>,
+) -> bool {
     let end_ty = cx.typeck_results().node_type(orig_expr.hir_id);
 
     // Bail out early if the end type is **not** a mutable pointer.
     if !matches!(end_ty.kind(), ty::RawPtr(_, Mutability::Mut)) {
-        return None;
+        return false;
     }
 
-    let (e, need_check_freeze) = peel_casts();
-
+    let e = peel_casts();
     let start_ty = cx.typeck_results().node_type(e.hir_id);
+
     if let ty::Ref(_, inner_ty, Mutability::Not) = start_ty.kind() {
-        // If an UnsafeCell method is involved, we need to additionally check the
-        // inner type for the presence of the Freeze trait (ie does NOT contain
-        // an UnsafeCell), since in that case we would incorrectly lint on valid casts.
+        // We need to additionally check the inner type for the presence of the Freeze trait
+        // (ie does NOT contain an UnsafeCell), since in that case we would incorrectly lint
+        // on valid casts (see https://github.com/rust-lang/unsafe-code-guidelines/issues/281).
         //
         // Except on the presence of non concrete skeleton types (ie generics)
         // since there is no way to make it safe for arbitrary types.
+        //
+        // However this does mean we miss out on some cases where the user doesn't go
+        // through an UnsafeCell but there's an UnsafeCell somewhere else in the type.
         let inner_ty_has_interior_mutability =
             !inner_ty.is_freeze(cx.tcx, cx.typing_env()) && inner_ty.has_concrete_skeleton();
-        (!need_check_freeze || !inner_ty_has_interior_mutability)
-            .then_some(inner_ty_has_interior_mutability)
+        !inner_ty_has_interior_mutability
     } else {
-        None
+        false
     }
 }
 
 fn is_cast_to_bigger_memory_layout<'tcx>(
     cx: &LateContext<'tcx>,
     orig_expr: &'tcx Expr<'tcx>,
-    mut peel_casts: impl FnMut() -> (&'tcx Expr<'tcx>, bool),
+    mut peel_casts: impl FnMut() -> &'tcx Expr<'tcx>,
 ) -> Option<(TyAndLayout<'tcx>, TyAndLayout<'tcx>, Expr<'tcx>)> {
     let end_ty = cx.typeck_results().node_type(orig_expr.hir_id);
 
     let ty::RawPtr(inner_end_ty, _) = end_ty.kind() else {
         return None;
     };
 
-    let (e, _) = peel_casts();
+    let e = peel_casts();
     let start_ty = cx.typeck_results().node_type(e.hir_id);
 
     let ty::Ref(_, inner_start_ty, _) = start_ty.kind() else {

compiler/rustc_lint/src/utils.rs

@@ -6,14 +6,11 @@ use crate::LateContext;
 /// Given an expression, peel all of casts (`<expr> as ...`, `<expr>.cast{,_mut,_const}()`,
 /// `ptr::from_ref(<expr>)`, ...) and init expressions.
 ///
-/// Returns the innermost expression and a boolean representing if one of the casts was
-/// `UnsafeCell::raw_get(<expr>)`
+/// Returns the innermost expression.
 pub(crate) fn peel_casts<'tcx>(
     cx: &LateContext<'tcx>,
     mut e: &'tcx Expr<'tcx>,
-) -> (&'tcx Expr<'tcx>, bool) {
-    let mut gone_trough_unsafe_cell_raw_get = false;
-
+) -> &'tcx Expr<'tcx> {
     loop {
         e = e.peel_blocks();
         // <expr> as ...
@@ -37,9 +34,6 @@ pub(crate) fn peel_casts<'tcx>(
                 Some(sym::ptr_from_ref | sym::unsafe_cell_raw_get | sym::transmute)
             )
         {
-            if cx.tcx.is_diagnostic_item(sym::unsafe_cell_raw_get, def_id) {
-                gone_trough_unsafe_cell_raw_get = true;
-            }
             arg
         } else {
             let init = cx.expr_or_init(e);
@@ -51,5 +45,5 @@ pub(crate) fn peel_casts<'tcx>(
         };
     }
 
-    (e, gone_trough_unsafe_cell_raw_get)
+    e
 }

library/core/src/cell.rs

@@ -2236,7 +2236,6 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
 /// /// # Safety
 /// /// The caller must not call `get_mut_unchecked` again (on any alias of `ptr`) for the duration
 /// /// of the lifetime of the returned reference.
-/// # #[allow(invalid_reference_casting)] // FIXME should the lint really fire here?
 /// unsafe fn get_mut_unchecked<T>(ptr: &UnsafeCell<T>) -> &mut T {
 ///   let t = ptr as *const UnsafeCell<T> as *mut T;
 ///   unsafe { &mut *t }

tests/ui/lint/reference_casting.rs

@@ -37,8 +37,8 @@ unsafe fn ref_to_mut() {
     let _num = &mut *(std::mem::transmute::<_, *mut i32>(num) as *mut i32);
     //~^ ERROR casting `&T` to `&mut T` is undefined behavior
     let _num = &mut *std::cell::UnsafeCell::raw_get(
-    //~^ ERROR casting `&T` to `&mut T` is undefined behavior
-        num as *const i32 as *const std::cell::UnsafeCell<i32>
+        //~^ ERROR casting `&T` to `&mut T` is undefined behavior
+        num as *const i32 as *const std::cell::UnsafeCell<i32>,
     );
 
     let deferred = num as *const i32 as *mut i32;
@@ -62,10 +62,6 @@ unsafe fn ref_to_mut() {
     let _num = &mut *num;
     //~^ ERROR casting `&T` to `&mut T` is undefined behavior
 
-    let cell = &std::cell::UnsafeCell::new(0);
-    let _num = &mut *(cell as *const _ as *mut i32);
-    //~^ ERROR casting `&T` to `&mut T` is undefined behavior
-
     unsafe fn generic_ref_cast_mut<T>(this: &T) -> &mut T {
         &mut *((this as *const _) as *mut _)
         //~^ ERROR casting `&T` to `&mut T` is undefined behavior
@@ -104,12 +100,10 @@ unsafe fn assign_to_ref() {
     *(std::mem::transmute::<_, *mut i32>(num) as *mut i32) += 1;
     //~^ ERROR assigning to `&T` is undefined behavior
     std::ptr::write(
-    //~^ ERROR assigning to `&T` is undefined behavior
+        //~^ ERROR assigning to `&T` is undefined behavior
         std::mem::transmute::<*const i32, *mut i32>(num),
         -1i32,
     );
-    *((&std::cell::UnsafeCell::new(0)) as *const _ as *mut i32) = 5;
-    //~^ ERROR assigning to `&T` is undefined behavior
 
     let value = num as *const i32 as *mut i32;
     *value = 1;
@@ -207,14 +201,16 @@ unsafe fn bigger_layout() {
     }
 
     {
-        let mut l: [u8; 2] = [0,1];
+        let mut l: [u8; 2] = [0, 1];
         let w: *mut [u16; 2] = &mut l as *mut [u8; 2] as *mut _;
-        let w: *mut [u16] = unsafe {&mut *w};
+        let w: *mut [u16] = unsafe { &mut *w };
         //~^ ERROR casting references to a bigger memory layout
     }
 
     {
-        fn foo() -> [i32; 1] { todo!() }
+        fn foo() -> [i32; 1] {
+            todo!()
+        }
 
         let num = foo();
         let _num = &*(&num as *const i32 as *const i64);
@@ -224,15 +220,19 @@ unsafe fn bigger_layout() {
     }
 
     {
-        fn bar(_a: &[i32; 2]) -> &[i32; 1] { todo!() }
+        fn bar(_a: &[i32; 2]) -> &[i32; 1] {
+            todo!()
+        }
 
         let num = bar(&[0, 0]);
         let _num = &*(num as *const i32 as *const i64);
         let _num = &*(bar(&[0, 0]) as *const i32 as *const i64);
     }
 
     {
-        fn foi<T>() -> T { todo!() }
+        fn foi<T>() -> T {
+            todo!()
+        }
 
         let num = foi::<i32>();
         let _num = &*(&num as *const i32 as *const i64);
@@ -286,24 +286,41 @@ unsafe fn no_warn() {
     let value: *const i32 = &mut value;
     *(value as *const i16 as *mut i16) = 42;
     *RAW_PTR = 42; // RAW_PTR is defined outside the function body,
-                   // make sure we don't ICE on it when trying to
-                   // determine if we should lint on it or not.
+    // make sure we don't ICE on it when trying to
+    // determine if we should lint on it or not.
+    *((&std::cell::UnsafeCell::new(0)) as *const _ as *mut i32) = 5;
+
     let cell = &std::cell::UnsafeCell::new(0);
     let _num = &mut *(cell.get() as *mut i32);
+    let _num = &mut *(cell as *const _ as *mut i32);
 
-    fn safe_as_mut<T>(x: &std::cell::UnsafeCell<T>) -> &mut T {
+    unsafe fn get_mut_unchecked<T>(x: &std::cell::UnsafeCell<T>) -> &mut T {
         unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
     }
 
-    fn cell_as_mut(x: &std::cell::Cell<i32>) -> &mut i32 {
+    unsafe fn get_mut_unchecked2<T>(ptr: &std::cell::UnsafeCell<T>) -> &mut T {
+        let t = ptr as *const std::cell::UnsafeCell<T> as *mut T;
+        unsafe { &mut *t }
+    }
+
+    unsafe fn cell_as_mut(x: &std::cell::Cell<i32>) -> &mut i32 {
         unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
     }
 
+    unsafe fn cell_as_mut2(x: &std::cell::Cell<i32>) -> &mut i32 {
+        unsafe { &mut *(x as *const std::cell::Cell<i32> as *mut i32) }
+    }
+
     #[repr(transparent)]
     struct DoesContainUnsafeCell(std::cell::UnsafeCell<i32>);
-    fn safe_as_mut2(x: &DoesContainUnsafeCell) -> &mut DoesContainUnsafeCell {
+
+    unsafe fn get_mut_unchecked3(x: &DoesContainUnsafeCell) -> &mut DoesContainUnsafeCell {
         unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
     }
+
+    unsafe fn get_mut_unchecked4(x: &DoesContainUnsafeCell) -> &mut DoesContainUnsafeCell {
+        unsafe { &mut *(x as *const DoesContainUnsafeCell as *mut _) }
+    }
 }
 
 fn main() {}