about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJubilee Young <workingjubilee@gmail.com>2024-05-31 18:48:42 -0700
committerJubilee Young <workingjubilee@gmail.com>2024-07-22 14:54:36 -0700
commit3fdd8d5ef3a8b26c3e24d9baf706fc7fb16717f9 (patch)
tree32a432fa477f9a67331bb7767e4ad848604aabad
parentada5e2c7b5427a591e30baeeee2698a5eb6db0bd (diff)
downloadrust-3fdd8d5ef3a8b26c3e24d9baf706fc7fb16717f9.tar.gz
rust-3fdd8d5ef3a8b26c3e24d9baf706fc7fb16717f9.zip
compiler: treat `&raw (const|mut) UNSAFE_STATIC` implied deref as safe
The implied deref to statics introduced by HIR->THIR lowering is only
used to create place expressions, it lacks unsafe semantics.
It is also confusing, as there is no visible `*ident` in the source.
For both classes of "unsafe static" (extern static and static mut)
allow this operation.

We lack a clear story around `thread_local! { static mut }`, which
is actually its own category of item that reuses the static syntax but
has its own rules. It's possible they should be similarly included, but
in the absence of a good reason one way or another, we do not bless it.
-rw-r--r--compiler/rustc_mir_build/src/check_unsafety.rs18
-rw-r--r--compiler/rustc_mir_build/src/thir/cx/expr.rs6
-rw-r--r--tests/ui/consts/const_refs_to_static.rs2
-rw-r--r--tests/ui/consts/mut-ptr-to-static.rs9
-rw-r--r--tests/ui/static/raw-ref-deref-with-unsafe.rs16
-rw-r--r--tests/ui/static/raw-ref-deref-without-unsafe.rs18
-rw-r--r--tests/ui/static/raw-ref-deref-without-unsafe.stderr19
-rw-r--r--tests/ui/static/raw-ref-extern-static.rs27
-rw-r--r--tests/ui/static/raw-ref-static-mut.rs17
9 files changed, 123 insertions, 9 deletions
diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs
index 24098282d93..b4a69d7d7b7 100644
--- a/compiler/rustc_mir_build/src/check_unsafety.rs
+++ b/compiler/rustc_mir_build/src/check_unsafety.rs
@@ -449,6 +449,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
diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs
index bd66257e6b6..0ba31f820b5 100644
--- a/compiler/rustc_mir_build/src/thir/cx/expr.rs
+++ b/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
diff --git a/tests/ui/consts/const_refs_to_static.rs b/tests/ui/consts/const_refs_to_static.rs
index 1baa8535b2c..f41725b786e 100644
--- a/tests/ui/consts/const_refs_to_static.rs
+++ b/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);
diff --git a/tests/ui/consts/mut-ptr-to-static.rs b/tests/ui/consts/mut-ptr-to-static.rs
index d8a788bb37d..e921365c7d4 100644
--- a/tests/ui/consts/mut-ptr-to-static.rs
+++ b/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;
diff --git a/tests/ui/static/raw-ref-deref-with-unsafe.rs b/tests/ui/static/raw-ref-deref-with-unsafe.rs
new file mode 100644
index 00000000000..4b8f72de5b7
--- /dev/null
+++ b/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 };
+}
diff --git a/tests/ui/static/raw-ref-deref-without-unsafe.rs b/tests/ui/static/raw-ref-deref-without-unsafe.rs
new file mode 100644
index 00000000000..f9bce4368c1
--- /dev/null
+++ b/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 };
+}
diff --git a/tests/ui/static/raw-ref-deref-without-unsafe.stderr b/tests/ui/static/raw-ref-deref-without-unsafe.stderr
new file mode 100644
index 00000000000..ac4df8b410c
--- /dev/null
+++ b/tests/ui/static/raw-ref-deref-without-unsafe.stderr
@@ -0,0 +1,19 @@
+error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block
+  --> $DIR/raw-ref-deref-without-unsafe.rs:12:56
+   |
+LL | static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);
+   |                                                        ^^^^^^^^^ dereference of raw pointer
+   |
+   = note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
+
+error[E0133]: use of mutable static is unsafe and requires unsafe function or block
+  --> $DIR/raw-ref-deref-without-unsafe.rs:12:57
+   |
+LL | static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);
+   |                                                         ^^^^^^^^ use of mutable static
+   |
+   = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0133`.
diff --git a/tests/ui/static/raw-ref-extern-static.rs b/tests/ui/static/raw-ref-extern-static.rs
new file mode 100644
index 00000000000..95a53a8640d
--- /dev/null
+++ b/tests/ui/static/raw-ref-extern-static.rs
@@ -0,0 +1,27 @@
+//@ check-pass
+#![feature(raw_ref_op)]
+use std::ptr;
+
+// see https://github.com/rust-lang/rust/issues/125833
+// notionally, taking the address of an extern static is a safe operation,
+// as we only point at it instead of generating a true reference to it
+
+// it may potentially induce linker errors, but the safety of that is not about taking addresses!
+// any safety obligation of the extern static's correctness in declaration is on the extern itself,
+// see RFC 3484 for more on that: https://rust-lang.github.io/rfcs/3484-unsafe-extern-blocks.html
+
+extern "C" {
+    static THERE: u8;
+    static mut SOMEWHERE: u8;
+}
+
+fn main() {
+    let ptr2there = ptr::addr_of!(THERE);
+    let ptr2somewhere = ptr::addr_of!(SOMEWHERE);
+    let ptr2somewhere = ptr::addr_of_mut!(SOMEWHERE);
+
+    // testing both addr_of and the expression it directly expands to
+    let raw2there = &raw const THERE;
+    let raw2somewhere = &raw const SOMEWHERE;
+    let raw2somewhere = &raw mut SOMEWHERE;
+}
diff --git a/tests/ui/static/raw-ref-static-mut.rs b/tests/ui/static/raw-ref-static-mut.rs
new file mode 100644
index 00000000000..6855cc7b050
--- /dev/null
+++ b/tests/ui/static/raw-ref-static-mut.rs
@@ -0,0 +1,17 @@
+//@ check-pass
+#![feature(raw_ref_op)]
+use std::ptr;
+
+// see https://github.com/rust-lang/rust/issues/125833
+// notionally, taking the address of a static mut is a safe operation,
+// as we only point at it instead of generating a true reference to it
+static mut NOWHERE: usize = 0;
+
+fn main() {
+    let p2nowhere = ptr::addr_of!(NOWHERE);
+    let p2nowhere = ptr::addr_of_mut!(NOWHERE);
+
+    // testing both addr_of and the expression it directly expands to
+    let raw2nowhere  = &raw const NOWHERE;
+    let raw2nowhere  = &raw mut NOWHERE;
+}