Skip to content

Commit 82ccdab

Browse files
committed
Disallow missing unsafe blocks in unsafe fn in panicking.rs
This adds SAFETY comments where necessary, explaining the preconditions and how they are respected.
1 parent 2c28244 commit 82ccdab

File tree

2 files changed

+49
-7
lines changed

2 files changed

+49
-7
lines changed

library/std/src/panicking.rs

+44-6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
//! * Executing a panic up to doing the actual implementation
88
//! * Shims around "try"
99
10+
#![deny(unsafe_op_in_unsafe_fn)]
11+
1012
use core::panic::{BoxMeUp, Location, PanicInfo};
1113

1214
use crate::any::Any;
@@ -322,25 +324,48 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
322324
let mut data = Data { f: ManuallyDrop::new(f) };
323325

324326
let data_ptr = &mut data as *mut _ as *mut u8;
325-
return if intrinsics::r#try(do_call::<F, R>, data_ptr, do_catch::<F, R>) == 0 {
326-
Ok(ManuallyDrop::into_inner(data.r))
327-
} else {
328-
Err(ManuallyDrop::into_inner(data.p))
329-
};
327+
// SAFETY:
328+
//
329+
// Access to the union's fields: this is `std` and we know that the `r#try`
330+
// intrinsic fills in the `r` or `p` union field based on its return value.
331+
//
332+
// The call to `intrinsics::r#try` is made safe by:
333+
// - `do_call`, the first argument, can be called with the initial `data_ptr`.
334+
// - `do_catch`, the second argument, can be called with the `data_ptr` as well.
335+
// See their safety preconditions for more informations
336+
unsafe {
337+
return if intrinsics::r#try(do_call::<F, R>, data_ptr, do_catch::<F, R>) == 0 {
338+
Ok(ManuallyDrop::into_inner(data.r))
339+
} else {
340+
Err(ManuallyDrop::into_inner(data.p))
341+
};
342+
}
330343

331344
// We consider unwinding to be rare, so mark this function as cold. However,
332345
// do not mark it no-inline -- that decision is best to leave to the
333346
// optimizer (in most cases this function is not inlined even as a normal,
334347
// non-cold function, though, as of the writing of this comment).
335348
#[cold]
336349
unsafe fn cleanup(payload: *mut u8) -> Box<dyn Any + Send + 'static> {
337-
let obj = Box::from_raw(__rust_panic_cleanup(payload));
350+
// SAFETY: The whole unsafe block hinges on a correct implementation of
351+
// the panic handler `__rust_panic_cleanup`. As such we can only
352+
// assume it returns the correct thing for `Box::from_raw` to work
353+
// without undefined behavior.
354+
let obj = unsafe { Box::from_raw(__rust_panic_cleanup(payload)) };
338355
panic_count::decrease();
339356
obj
340357
}
341358

359+
// SAFETY:
360+
// data must be non-NUL, correctly aligned, and a pointer to a `Data<F, R>`
361+
// Its must contains a valid `f` (type: F) value that can be use to fill
362+
// `data.r`.
363+
//
364+
// This function cannot be marked as `unsafe` because `intrinsics::r#try`
365+
// expects normal function pointers.
342366
#[inline]
343367
fn do_call<F: FnOnce() -> R, R>(data: *mut u8) {
368+
// SAFETY: this is the responsibilty of the caller, see above.
344369
unsafe {
345370
let data = data as *mut Data<F, R>;
346371
let data = &mut (*data);
@@ -352,8 +377,21 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
352377
// We *do* want this part of the catch to be inlined: this allows the
353378
// compiler to properly track accesses to the Data union and optimize it
354379
// away most of the time.
380+
//
381+
// SAFETY:
382+
// data must be non-NUL, correctly aligned, and a pointer to a `Data<F, R>`
383+
// Since this uses `cleanup` it also hinges on a correct implementation of
384+
// `__rustc_panic_cleanup`.
385+
//
386+
// This function cannot be marked as `unsafe` because `intrinsics::r#try`
387+
// expects normal function pointers.
355388
#[inline]
356389
fn do_catch<F: FnOnce() -> R, R>(data: *mut u8, payload: *mut u8) {
390+
// SAFETY: this is the responsibilty of the caller, see above.
391+
//
392+
// When `__rustc_panic_cleaner` is correctly implemented we can rely
393+
// on `obj` being the correct thing to pass to `data.p` (after wrapping
394+
// in `ManuallyDrop`).
357395
unsafe {
358396
let data = data as *mut Data<F, R>;
359397
let data = &mut (*data);

library/std/src/thread/local.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,11 @@ macro_rules! __thread_local_inner {
172172
static __KEY: $crate::thread::__OsLocalKeyInner<$t> =
173173
$crate::thread::__OsLocalKeyInner::new();
174174

175-
__KEY.get(__init)
175+
// FIXME: remove the #[allow(...)] marker when macros don't
176+
// raise warning for missing/extraneous unsafe blocks anymore.
177+
// See https://github.com/rust-lang/rust/issues/74838.
178+
#[allow(unused_unsafe)]
179+
unsafe { __KEY.get(__init) }
176180
}
177181

178182
unsafe {

0 commit comments

Comments
 (0)