Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
workingjubilee marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions library/panic_unwind/src/seh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
};
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/extern_static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions src/tools/miri/tests/fail/extern_static.stderr
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/pass/static_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const_refs_to_static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 3 additions & 6 deletions tests/ui/consts/mut-ptr-to-static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/static/raw-ref-deref-with-unsafe.rs
Original file line number Diff line number Diff line change
@@ -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 };
}
18 changes: 18 additions & 0 deletions tests/ui/static/raw-ref-deref-without-unsafe.rs
Original file line number Diff line number Diff line change
@@ -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 };
}
19 changes: 19 additions & 0 deletions tests/ui/static/raw-ref-deref-without-unsafe.stderr
Original file line number Diff line number Diff line change
@@ -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`.
27 changes: 27 additions & 0 deletions tests/ui/static/raw-ref-extern-static.rs
Original file line number Diff line number Diff line change
@@ -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;
}
17 changes: 17 additions & 0 deletions tests/ui/static/raw-ref-static-mut.rs
Original file line number Diff line number Diff line change
@@ -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;
}
Loading