Skip to content

Commit 3fdd8d5

Browse files
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.
1 parent ada5e2c commit 3fdd8d5

File tree

9 files changed

+123
-9
lines changed

9 files changed

+123
-9
lines changed

compiler/rustc_mir_build/src/check_unsafety.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,24 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
449449
}
450450
}
451451
}
452+
ExprKind::AddressOf { arg, .. } => {
453+
if let ExprKind::Scope { value: arg, .. } = self.thir[arg].kind
454+
// THIR desugars UNSAFE_STATIC into *UNSAFE_STATIC_REF, where
455+
// UNSAFE_STATIC_REF holds the addr of the UNSAFE_STATIC, so: take two steps
456+
&& let ExprKind::Deref { arg } = self.thir[arg].kind
457+
// FIXME(workingjubiee): we lack a clear reason to reject ThreadLocalRef here,
458+
// but we also have no conclusive reason to allow it either!
459+
&& let ExprKind::StaticRef { .. } = self.thir[arg].kind
460+
{
461+
// A raw ref to a place expr, even an "unsafe static", is okay!
462+
// We short-circuit to not recursively traverse this expression.
463+
return;
464+
// note: const_mut_refs enables this code, and it currently remains unsafe:
465+
// static mut BYTE: u8 = 0;
466+
// static mut BYTE_PTR: *mut u8 = unsafe { addr_of_mut!(BYTE) };
467+
// static mut DEREF_BYTE_PTR: *mut u8 = unsafe { addr_of_mut!(*BYTE_PTR) };
468+
}
469+
}
452470
ExprKind::Deref { arg } => {
453471
if let ExprKind::StaticRef { def_id, .. } | ExprKind::ThreadLocalRef(def_id) =
454472
self.thir[arg].kind

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -939,9 +939,11 @@ impl<'tcx> Cx<'tcx> {
939939
}
940940
}
941941

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

tests/ui/consts/const_refs_to_static.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const C1: &i32 = &S;
99
const C1_READ: () = {
1010
assert!(*C1 == 0);
1111
};
12-
const C2: *const i32 = unsafe { std::ptr::addr_of!(S_MUT) };
12+
const C2: *const i32 = std::ptr::addr_of!(S_MUT);
1313

1414
fn main() {
1515
assert_eq!(*C1, 0);

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,9 @@ static mut STATIC: u32 = 42;
1616
static INTERIOR_MUTABLE_STATIC: SyncUnsafeCell<u32> = SyncUnsafeCell::new(42);
1717

1818
// A static that mutably points to STATIC.
19-
static PTR: SyncPtr = SyncPtr {
20-
foo: unsafe { ptr::addr_of_mut!(STATIC) },
21-
};
22-
static INTERIOR_MUTABLE_PTR: SyncPtr = SyncPtr {
23-
foo: ptr::addr_of!(INTERIOR_MUTABLE_STATIC) as *mut u32,
24-
};
19+
static PTR: SyncPtr = SyncPtr { foo: ptr::addr_of_mut!(STATIC) };
20+
static INTERIOR_MUTABLE_PTR: SyncPtr =
21+
SyncPtr { foo: ptr::addr_of!(INTERIOR_MUTABLE_STATIC) as *mut u32 };
2522

2623
fn main() {
2724
let ptr = PTR.foo;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//@ check-pass
2+
#![feature(const_mut_refs)]
3+
use std::ptr;
4+
5+
// This code should remain unsafe because of the two unsafe operations here,
6+
// even if in a hypothetical future we deem all &raw (const|mut) *ptr exprs safe.
7+
8+
static mut BYTE: u8 = 0;
9+
static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE);
10+
// An unsafe static's ident is a place expression in its own right, so despite the above being safe
11+
// (it's fine to create raw refs to places!) the following derefs the ptr before creating its ref
12+
static mut DEREF_BYTE_PTR: *mut u8 = unsafe { ptr::addr_of_mut!(*BYTE_PTR) };
13+
14+
fn main() {
15+
let _ = unsafe { DEREF_BYTE_PTR };
16+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#![feature(const_mut_refs)]
2+
3+
use std::ptr;
4+
5+
// This code should remain unsafe because of the two unsafe operations here,
6+
// even if in a hypothetical future we deem all &raw (const|mut) *ptr exprs safe.
7+
8+
static mut BYTE: u8 = 0;
9+
static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE);
10+
// An unsafe static's ident is a place expression in its own right, so despite the above being safe
11+
// (it's fine to create raw refs to places!) the following derefs the ptr before creating its ref!
12+
static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);
13+
//~^ ERROR: use of mutable static
14+
//~| ERROR: dereference of raw pointer
15+
16+
fn main() {
17+
let _ = unsafe { DEREF_BYTE_PTR };
18+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block
2+
--> $DIR/raw-ref-deref-without-unsafe.rs:12:56
3+
|
4+
LL | static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);
5+
| ^^^^^^^^^ dereference of raw pointer
6+
|
7+
= note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
8+
9+
error[E0133]: use of mutable static is unsafe and requires unsafe function or block
10+
--> $DIR/raw-ref-deref-without-unsafe.rs:12:57
11+
|
12+
LL | static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);
13+
| ^^^^^^^^ use of mutable static
14+
|
15+
= note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior
16+
17+
error: aborting due to 2 previous errors
18+
19+
For more information about this error, try `rustc --explain E0133`.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//@ check-pass
2+
#![feature(raw_ref_op)]
3+
use std::ptr;
4+
5+
// see https://github.com/rust-lang/rust/issues/125833
6+
// notionally, taking the address of an extern static is a safe operation,
7+
// as we only point at it instead of generating a true reference to it
8+
9+
// it may potentially induce linker errors, but the safety of that is not about taking addresses!
10+
// any safety obligation of the extern static's correctness in declaration is on the extern itself,
11+
// see RFC 3484 for more on that: https://rust-lang.github.io/rfcs/3484-unsafe-extern-blocks.html
12+
13+
extern "C" {
14+
static THERE: u8;
15+
static mut SOMEWHERE: u8;
16+
}
17+
18+
fn main() {
19+
let ptr2there = ptr::addr_of!(THERE);
20+
let ptr2somewhere = ptr::addr_of!(SOMEWHERE);
21+
let ptr2somewhere = ptr::addr_of_mut!(SOMEWHERE);
22+
23+
// testing both addr_of and the expression it directly expands to
24+
let raw2there = &raw const THERE;
25+
let raw2somewhere = &raw const SOMEWHERE;
26+
let raw2somewhere = &raw mut SOMEWHERE;
27+
}

tests/ui/static/raw-ref-static-mut.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//@ check-pass
2+
#![feature(raw_ref_op)]
3+
use std::ptr;
4+
5+
// see https://github.com/rust-lang/rust/issues/125833
6+
// notionally, taking the address of a static mut is a safe operation,
7+
// as we only point at it instead of generating a true reference to it
8+
static mut NOWHERE: usize = 0;
9+
10+
fn main() {
11+
let p2nowhere = ptr::addr_of!(NOWHERE);
12+
let p2nowhere = ptr::addr_of_mut!(NOWHERE);
13+
14+
// testing both addr_of and the expression it directly expands to
15+
let raw2nowhere = &raw const NOWHERE;
16+
let raw2nowhere = &raw mut NOWHERE;
17+
}

0 commit comments

Comments
 (0)