Rollup merge of #125834 - workingjubilee:weaken-thir-unsafeck-for-addr-of-static-mut, r=compiler-errors

treat `&raw (const|mut) UNSAFE_STATIC` implied deref as safe

Fixes #125833

As reported in that and related issues, `static mut STATIC_MUT: T` is very often used in embedded code, and is in many ways equivalent to `static STATIC_CELL: SyncUnsafeCell<T>`. The Rust expression of `&raw mut STATIC_MUT` and `SyncUnsafeCell::get(&STATIC_CELL)` are approximately equal, and both evaluate to `*mut T`. The library function is safe because it has *declared itself* to be safe. However, the raw ref operator is unsafe because all uses of `static mut` are considered unsafe, even though the static's value is not used by this expression (unlike, for example, `&STATIC_MUT`).

We can fix this unnatural difference by simply adding the proper exclusion for the safety check inside the THIR unsafeck, so that we do not declare it unsafe if it is not.

While the primary concern here is `static mut`, this change is made for all instances of an "unsafe static", which includes a static declared inside `extern "abi" {}`. Hypothetically, we could go as far as generalizing this to all instances of `&raw (const|mut) *ptr`, but today we do not, as we have not actually considered the range of possible expressions that use a similar encoding. We do not even extend this to thread-local equivalents, because they have less clear semantics.

Author: matthiaskrgr

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -466,6 +466,24 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                     }
                 }
             }
+            ExprKind::AddressOf { arg, .. } => {
+                if let ExprKind::Scope { value: arg, .. } = self.thir[arg].kind
+                // THIR desugars UNSAFE_STATIC into *UNSAFE_STATIC_REF, where
+                // UNSAFE_STATIC_REF holds the addr of the UNSAFE_STATIC, so: take two steps
+                    && let ExprKind::Deref { arg } = self.thir[arg].kind
+                    // FIXME(workingjubiee): we lack a clear reason to reject ThreadLocalRef here,
+                    // but we also have no conclusive reason to allow it either!
+                    && let ExprKind::StaticRef { .. } = self.thir[arg].kind
+                {
+                    // A raw ref to a place expr, even an "unsafe static", is okay!
+                    // We short-circuit to not recursively traverse this expression.
+                    return;
+                    // note: const_mut_refs enables this code, and it currently remains unsafe:
+                    // static mut BYTE: u8 = 0;
+                    // static mut BYTE_PTR: *mut u8 = unsafe { addr_of_mut!(BYTE) };
+                    // static mut DEREF_BYTE_PTR: *mut u8 = unsafe { addr_of_mut!(*BYTE_PTR) };
+                }
+            }
             ExprKind::Deref { arg } => {
                 if let ExprKind::StaticRef { def_id, .. } | ExprKind::ThreadLocalRef(def_id) =
                     self.thir[arg].kind

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -939,9 +939,11 @@ impl<'tcx> Cx<'tcx> {
                 }
             }
 
-            // We encode uses of statics as a `*&STATIC` where the `&STATIC` part is
-            // a constant reference (or constant raw pointer for `static mut`) in MIR
+            // A source Rust `path::to::STATIC` is a place expr like *&ident is.
+            // In THIR, we make them exactly equivalent by inserting the implied *& or *&raw,
+            // but distinguish between &STATIC and &THREAD_LOCAL as they have different semantics
             Res::Def(DefKind::Static { .. }, id) => {
+                // this is &raw for extern static or static mut, and & for other statics
                 let ty = self.tcx.static_ptr_ty(id);
                 let temp_lifetime = self
                     .rvalue_scopes

library/panic_unwind/src/seh.rs

@@ -157,7 +157,10 @@ mod imp {
             // going to be cross-lang LTOed anyway. However, using expose is shorter and
             // requires less unsafe.
             let addr: usize = ptr.expose_provenance();
+            #[cfg(bootstrap)]
             let image_base = unsafe { addr_of!(__ImageBase) }.addr();
+            #[cfg(not(bootstrap))]
+            let image_base = addr_of!(__ImageBase).addr();
             let offset: usize = addr - image_base;
             Self(offset as u32)
         }
@@ -250,7 +253,10 @@ extern "C" {
 // This is fine since the MSVC runtime uses string comparison on the type name
 // to match TypeDescriptors rather than pointer equality.
 static mut TYPE_DESCRIPTOR: _TypeDescriptor = _TypeDescriptor {
+    #[cfg(bootstrap)]
     pVFTable: unsafe { addr_of!(TYPE_INFO_VTABLE) } as *const _,
+    #[cfg(not(bootstrap))]
+    pVFTable: addr_of!(TYPE_INFO_VTABLE) as *const _,
     spare: core::ptr::null_mut(),
     name: TYPE_NAME,
 };

src/tools/miri/tests/fail/extern_static.rs

@@ -5,5 +5,5 @@ extern "C" {
 }
 
 fn main() {
-    let _val = unsafe { std::ptr::addr_of!(FOO) }; //~ ERROR: is not supported by Miri
+    let _val = std::ptr::addr_of!(FOO); //~ ERROR: is not supported by Miri
 }

src/tools/miri/tests/fail/extern_static.stderr

@@ -1,8 +1,8 @@
 error: unsupported operation: extern static `FOO` is not supported by Miri
   --> $DIR/extern_static.rs:LL:CC
    |
-LL |     let _val = unsafe { std::ptr::addr_of!(FOO) };
-   |                                            ^^^ extern static `FOO` is not supported by Miri
+LL |     let _val = std::ptr::addr_of!(FOO);
+   |                                   ^^^ extern static `FOO` is not supported by Miri
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support
    = note: BACKTRACE:

src/tools/miri/tests/pass/static_mut.rs

@@ -2,7 +2,7 @@ use std::ptr::addr_of;
 
 static mut FOO: i32 = 42;
 
-static BAR: Foo = Foo(unsafe { addr_of!(FOO) });
+static BAR: Foo = Foo(addr_of!(FOO));
 
 #[allow(dead_code)]
 struct Foo(*const i32);

tests/ui/consts/const_refs_to_static.rs

@@ -9,7 +9,7 @@ const C1: &i32 = &S;
 const C1_READ: () = {
     assert!(*C1 == 0);
 };
-const C2: *const i32 = unsafe { std::ptr::addr_of!(S_MUT) };
+const C2: *const i32 = std::ptr::addr_of!(S_MUT);
 
 fn main() {
     assert_eq!(*C1, 0);

tests/ui/consts/mut-ptr-to-static.rs

@@ -16,12 +16,9 @@ static mut STATIC: u32 = 42;
 static INTERIOR_MUTABLE_STATIC: SyncUnsafeCell<u32> = SyncUnsafeCell::new(42);
 
 // A static that mutably points to STATIC.
-static PTR: SyncPtr = SyncPtr {
-    foo: unsafe { ptr::addr_of_mut!(STATIC) },
-};
-static INTERIOR_MUTABLE_PTR: SyncPtr = SyncPtr {
-    foo: ptr::addr_of!(INTERIOR_MUTABLE_STATIC) as *mut u32,
-};
+static PTR: SyncPtr = SyncPtr { foo: ptr::addr_of_mut!(STATIC) };
+static INTERIOR_MUTABLE_PTR: SyncPtr =
+    SyncPtr { foo: ptr::addr_of!(INTERIOR_MUTABLE_STATIC) as *mut u32 };
 
 fn main() {
     let ptr = PTR.foo;

tests/ui/static/raw-ref-deref-with-unsafe.rs

@@ -0,0 +1,16 @@
+//@ check-pass
+#![feature(const_mut_refs)]
+use std::ptr;
+
+// This code should remain unsafe because of the two unsafe operations here,
+// even if in a hypothetical future we deem all &raw (const|mut) *ptr exprs safe.
+
+static mut BYTE: u8 = 0;
+static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE);
+// An unsafe static's ident is a place expression in its own right, so despite the above being safe
+// (it's fine to create raw refs to places!) the following derefs the ptr before creating its ref
+static mut DEREF_BYTE_PTR: *mut u8 = unsafe { ptr::addr_of_mut!(*BYTE_PTR) };
+
+fn main() {
+    let _ = unsafe { DEREF_BYTE_PTR };
+}

tests/ui/static/raw-ref-deref-without-unsafe.rs

@@ -0,0 +1,18 @@
+#![feature(const_mut_refs)]
+
+use std::ptr;
+
+// This code should remain unsafe because of the two unsafe operations here,
+// even if in a hypothetical future we deem all &raw (const|mut) *ptr exprs safe.
+
+static mut BYTE: u8 = 0;
+static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE);
+// An unsafe static's ident is a place expression in its own right, so despite the above being safe
+// (it's fine to create raw refs to places!) the following derefs the ptr before creating its ref!
+static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);
+//~^ ERROR: use of mutable static
+//~| ERROR: dereference of raw pointer
+
+fn main() {
+    let _ = unsafe { DEREF_BYTE_PTR };
+}