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

Even nicer errors from assert_unsafe_precondition #103035

Merged
merged 1 commit into from
Oct 27, 2022
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
2 changes: 1 addition & 1 deletion library/core/src/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub const unsafe fn unreachable_unchecked() -> ! {
// SAFETY: the safety contract for `intrinsics::unreachable` must
// be upheld by the caller.
unsafe {
intrinsics::assert_unsafe_precondition!(() => false);
intrinsics::assert_unsafe_precondition!("hint::unreachable_unchecked must never be reached", () => false);
intrinsics::unreachable()
}
}
Expand Down
23 changes: 17 additions & 6 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2203,15 +2203,17 @@ extern "rust-intrinsic" {
/// the occasional mistake, and this check should help them figure things out.
#[allow_internal_unstable(const_eval_select)] // permit this to be called in stably-const fn
macro_rules! assert_unsafe_precondition {
($([$($tt:tt)*])?($($i:ident:$ty:ty),*$(,)?) => $e:expr) => {
($name:expr, $([$($tt:tt)*])?($($i:ident:$ty:ty),*$(,)?) => $e:expr) => {
if cfg!(debug_assertions) {
// allow non_snake_case to allow capturing const generics
#[allow(non_snake_case)]
#[inline(always)]
fn runtime$(<$($tt)*>)?($($i:$ty),*) {
if !$e {
// don't unwind to reduce impact on code size
::core::panicking::panic_str_nounwind("unsafe precondition violated");
::core::panicking::panic_str_nounwind(
concat!("unsafe precondition(s) violated: ", $name)
);
}
}
#[allow(non_snake_case)]
Expand Down Expand Up @@ -2350,7 +2352,10 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
// SAFETY: the safety contract for `copy_nonoverlapping` must be
// upheld by the caller.
unsafe {
assert_unsafe_precondition!([T](src: *const T, dst: *mut T, count: usize) =>
assert_unsafe_precondition!(
"ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null \
and the specified memory ranges do not overlap",
[T](src: *const T, dst: *mut T, count: usize) =>
is_aligned_and_not_null(src)
&& is_aligned_and_not_null(dst)
&& is_nonoverlapping(src, dst, count)
Expand Down Expand Up @@ -2436,8 +2441,11 @@ pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {

// SAFETY: the safety contract for `copy` must be upheld by the caller.
unsafe {
assert_unsafe_precondition!([T](src: *const T, dst: *mut T) =>
is_aligned_and_not_null(src) && is_aligned_and_not_null(dst));
assert_unsafe_precondition!(
"ptr::copy requires that both pointer arguments are aligned aligned and non-null",
[T](src: *const T, dst: *mut T) =>
is_aligned_and_not_null(src) && is_aligned_and_not_null(dst)
);
copy(src, dst, count)
}
}
Expand Down Expand Up @@ -2505,7 +2513,10 @@ pub const unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {

// SAFETY: the safety contract for `write_bytes` must be upheld by the caller.
unsafe {
assert_unsafe_precondition!([T](dst: *mut T) => is_aligned_and_not_null(dst));
assert_unsafe_precondition!(
"ptr::write_bytes requires that the destination pointer is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
);
write_bytes(dst, val, count)
}
}
5 changes: 4 additions & 1 deletion library/core/src/num/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ macro_rules! nonzero_integers {
pub const unsafe fn new_unchecked(n: $Int) -> Self {
// SAFETY: this is guaranteed to be safe by the caller.
unsafe {
core::intrinsics::assert_unsafe_precondition!((n: $Int) => n != 0);
core::intrinsics::assert_unsafe_precondition!(
concat!(stringify!($Ty), "::new_unchecked requires a non-zero argument"),
(n: $Int) => n != 0
);
Self(n)
}
}
Expand Down
7 changes: 6 additions & 1 deletion library/core/src/ops/index_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ impl IndexRange {
#[inline]
pub const unsafe fn new_unchecked(start: usize, end: usize) -> Self {
// SAFETY: comparisons on usize are pure
unsafe { assert_unsafe_precondition!((start: usize, end: usize) => start <= end) };
unsafe {
assert_unsafe_precondition!(
"IndexRange::new_unchecked requires `start <= end`",
(start: usize, end: usize) => start <= end
)
};
IndexRange { start, end }
}

Expand Down
7 changes: 6 additions & 1 deletion library/core/src/ptr/alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ impl Alignment {
#[inline]
pub const unsafe fn new_unchecked(align: usize) -> Self {
// SAFETY: Precondition passed to the caller.
unsafe { assert_unsafe_precondition!((align: usize) => align.is_power_of_two()) };
unsafe {
assert_unsafe_precondition!(
"Alignment::new_unchecked requires a power of two",
(align: usize) => align.is_power_of_two()
)
};

// SAFETY: By precondition, this must be a power of two, and
// our variants encompass all possible powers of two.
Expand Down
5 changes: 4 additions & 1 deletion library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,10 @@ impl<T: ?Sized> *const T {
// SAFETY: The comparison has no side-effects, and the intrinsic
// does this check internally in the CTFE implementation.
unsafe {
assert_unsafe_precondition!([T](this: *const T, origin: *const T) => this >= origin)
assert_unsafe_precondition!(
"ptr::sub_ptr requires `this >= origin`",
[T](this: *const T, origin: *const T) => this >= origin
)
};

let pointee_size = mem::size_of::<T>();
Expand Down
30 changes: 24 additions & 6 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,10 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
// SAFETY: the caller must guarantee that `x` and `y` are
// valid for writes and properly aligned.
unsafe {
assert_unsafe_precondition!([T](x: *mut T, y: *mut T, count: usize) =>
assert_unsafe_precondition!(
"ptr::swap_nonoverlapping requires that both pointer arguments are aligned and non-null \
and the specified memory ranges do not overlap",
[T](x: *mut T, y: *mut T, count: usize) =>
is_aligned_and_not_null(x)
&& is_aligned_and_not_null(y)
&& is_nonoverlapping(x, y, count)
Expand Down Expand Up @@ -986,7 +989,10 @@ pub const unsafe fn replace<T>(dst: *mut T, mut src: T) -> T {
// and cannot overlap `src` since `dst` must point to a distinct
// allocated object.
unsafe {
assert_unsafe_precondition!([T](dst: *mut T) => is_aligned_and_not_null(dst));
assert_unsafe_precondition!(
"ptr::replace requires that the pointer argument is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
);
mem::swap(&mut *dst, &mut src); // cannot overlap
}
src
Expand Down Expand Up @@ -1117,7 +1123,10 @@ pub const unsafe fn read<T>(src: *const T) -> T {
// Also, since we just wrote a valid value into `tmp`, it is guaranteed
// to be properly initialized.
unsafe {
assert_unsafe_precondition!([T](src: *const T) => is_aligned_and_not_null(src));
assert_unsafe_precondition!(
"ptr::read requires that the pointer argument is aligned and non-null",
[T](src: *const T) => is_aligned_and_not_null(src)
);
copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
tmp.assume_init()
}
Expand Down Expand Up @@ -1311,7 +1320,10 @@ pub const unsafe fn write<T>(dst: *mut T, src: T) {
// `dst` cannot overlap `src` because the caller has mutable access
// to `dst` while `src` is owned by this function.
unsafe {
assert_unsafe_precondition!([T](dst: *mut T) => is_aligned_and_not_null(dst));
assert_unsafe_precondition!(
"ptr::write requires that the pointer argument is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
);
copy_nonoverlapping(&src as *const T, dst, 1);
intrinsics::forget(src);
}
Expand Down Expand Up @@ -1475,7 +1487,10 @@ pub const unsafe fn write_unaligned<T>(dst: *mut T, src: T) {
pub unsafe fn read_volatile<T>(src: *const T) -> T {
// SAFETY: the caller must uphold the safety contract for `volatile_load`.
unsafe {
assert_unsafe_precondition!([T](src: *const T) => is_aligned_and_not_null(src));
assert_unsafe_precondition!(
"ptr::read_volatile requires that the pointer argument is aligned and non-null",
[T](src: *const T) => is_aligned_and_not_null(src)
);
intrinsics::volatile_load(src)
}
}
Expand Down Expand Up @@ -1546,7 +1561,10 @@ pub unsafe fn read_volatile<T>(src: *const T) -> T {
pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
// SAFETY: the caller must uphold the safety contract for `volatile_store`.
unsafe {
assert_unsafe_precondition!([T](dst: *mut T) => is_aligned_and_not_null(dst));
assert_unsafe_precondition!(
"ptr::write_volatile requires that the pointer argument is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
);
intrinsics::volatile_store(dst, src);
}
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/ptr/non_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl<T: ?Sized> NonNull<T> {
pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
// SAFETY: the caller must guarantee that `ptr` is non-null.
unsafe {
assert_unsafe_precondition!([T: ?Sized](ptr: *mut T) => !ptr.is_null());
assert_unsafe_precondition!("NonNull::new_unchecked requires that the pointer is non-null", [T: ?Sized](ptr: *mut T) => !ptr.is_null());
NonNull { pointer: ptr as _ }
}
}
Expand Down
36 changes: 26 additions & 10 deletions library/core/src/slice/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,10 @@ unsafe impl<T> const SliceIndex<[T]> for usize {
// `self` is in bounds of `slice` so `self` cannot overflow an `isize`,
// so the call to `add` is safe.
unsafe {
assert_unsafe_precondition!([T](this: usize, slice: *const [T]) => this < slice.len());
assert_unsafe_precondition!(
"slice::get_unchecked requires that the index is within the slice",
[T](this: usize, slice: *const [T]) => this < slice.len()
);
slice.as_ptr().add(self)
}
}
Expand All @@ -242,7 +245,10 @@ unsafe impl<T> const SliceIndex<[T]> for usize {
let this = self;
// SAFETY: see comments for `get_unchecked` above.
unsafe {
assert_unsafe_precondition!([T](this: usize, slice: *mut [T]) => this < slice.len());
assert_unsafe_precondition!(
"slice::get_unchecked_mut requires that the index is within the slice",
[T](this: usize, slice: *mut [T]) => this < slice.len()
);
slice.as_mut_ptr().add(self)
}
}
Expand Down Expand Up @@ -295,8 +301,10 @@ unsafe impl<T> const SliceIndex<[T]> for ops::IndexRange {
// so the call to `add` is safe.

unsafe {
assert_unsafe_precondition!([T](end: usize, slice: *const [T]) =>
end <= slice.len());
assert_unsafe_precondition!(
"slice::get_unchecked requires that the index is within the slice",
[T](end: usize, slice: *const [T]) => end <= slice.len()
);
ptr::slice_from_raw_parts(slice.as_ptr().add(self.start()), self.len())
}
}
Expand All @@ -306,8 +314,10 @@ unsafe impl<T> const SliceIndex<[T]> for ops::IndexRange {
let end = self.end();
// SAFETY: see comments for `get_unchecked` above.
unsafe {
assert_unsafe_precondition!([T](end: usize, slice: *mut [T]) =>
end <= slice.len());
assert_unsafe_precondition!(
"slice::get_unchecked_mut requires that the index is within the slice",
[T](end: usize, slice: *mut [T]) => end <= slice.len()
);
ptr::slice_from_raw_parts_mut(slice.as_mut_ptr().add(self.start()), self.len())
}
}
Expand Down Expand Up @@ -367,8 +377,11 @@ unsafe impl<T> const SliceIndex<[T]> for ops::Range<usize> {
// so the call to `add` is safe.

unsafe {
assert_unsafe_precondition!([T](this: ops::Range<usize>, slice: *const [T]) =>
this.end >= this.start && this.end <= slice.len());
assert_unsafe_precondition!(
"slice::get_unchecked requires that the range is within the slice",
[T](this: ops::Range<usize>, slice: *const [T]) =>
this.end >= this.start && this.end <= slice.len()
);
ptr::slice_from_raw_parts(slice.as_ptr().add(self.start), self.end - self.start)
}
}
Expand All @@ -378,8 +391,11 @@ unsafe impl<T> const SliceIndex<[T]> for ops::Range<usize> {
let this = ops::Range { start: self.start, end: self.end };
// SAFETY: see comments for `get_unchecked` above.
unsafe {
assert_unsafe_precondition!([T](this: ops::Range<usize>, slice: *mut [T]) =>
this.end >= this.start && this.end <= slice.len());
assert_unsafe_precondition!(
"slice::get_unchecked_mut requires that the range is within the slice",
[T](this: ops::Range<usize>, slice: *mut [T]) =>
this.end >= this.start && this.end <= slice.len()
);
ptr::slice_from_raw_parts_mut(slice.as_mut_ptr().add(self.start), self.end - self.start)
}
}
Expand Down
20 changes: 16 additions & 4 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,10 @@ impl<T> [T] {
let ptr = this.as_mut_ptr();
// SAFETY: caller has to guarantee that `a < self.len()` and `b < self.len()`
unsafe {
assert_unsafe_precondition!([T](a: usize, b: usize, this: &mut [T]) => a < this.len() && b < this.len());
assert_unsafe_precondition!(
"slice::swap_unchecked requires that the indices are within the slice",
[T](a: usize, b: usize, this: &mut [T]) => a < this.len() && b < this.len()
);
ptr::swap(ptr.add(a), ptr.add(b));
}
}
Expand Down Expand Up @@ -969,7 +972,10 @@ impl<T> [T] {
let this = self;
// SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length
let new_len = unsafe {
assert_unsafe_precondition!([T](this: &[T], N: usize) => N != 0 && this.len() % N == 0);
assert_unsafe_precondition!(
"slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks",
[T](this: &[T], N: usize) => N != 0 && this.len() % N == 0
);
exact_div(self.len(), N)
};
// SAFETY: We cast a slice of `new_len * N` elements into
Expand Down Expand Up @@ -1109,7 +1115,10 @@ impl<T> [T] {
let this = &*self;
// SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length
let new_len = unsafe {
assert_unsafe_precondition!([T](this: &[T], N: usize) => N != 0 && this.len() % N == 0);
assert_unsafe_precondition!(
"slice::as_chunks_unchecked_mut requires `N != 0` and the slice to split exactly into `N`-element chunks",
[T](this: &[T], N: usize) => N != 0 && this.len() % N == 0
);
exact_div(this.len(), N)
};
// SAFETY: We cast a slice of `new_len * N` elements into
Expand Down Expand Up @@ -1685,7 +1694,10 @@ impl<T> [T] {
// `[ptr; mid]` and `[mid; len]` are not overlapping, so returning a mutable reference
// is fine.
unsafe {
assert_unsafe_precondition!((mid: usize, len: usize) => mid <= len);
assert_unsafe_precondition!(
"slice::split_at_mut_unchecked requires the index to be within the slice",
(mid: usize, len: usize) => mid <= len
);
(from_raw_parts_mut(ptr, mid), from_raw_parts_mut(ptr.add(mid), len - mid))
}
}
Expand Down
12 changes: 8 additions & 4 deletions library/core/src/slice/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,10 @@ use crate::ptr;
pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
// SAFETY: the caller must uphold the safety contract for `from_raw_parts`.
unsafe {
assert_unsafe_precondition!([T](data: *const T, len: usize) =>
is_aligned_and_not_null(data) && is_valid_allocation_size::<T>(len)
assert_unsafe_precondition!(
"slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`",
[T](data: *const T, len: usize) => is_aligned_and_not_null(data)
&& is_valid_allocation_size::<T>(len)
);
&*ptr::slice_from_raw_parts(data, len)
}
Expand Down Expand Up @@ -135,8 +137,10 @@ pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T]
pub const unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] {
// SAFETY: the caller must uphold the safety contract for `from_raw_parts_mut`.
unsafe {
assert_unsafe_precondition!([T](data: *mut T, len: usize) =>
is_aligned_and_not_null(data) && is_valid_allocation_size::<T>(len)
assert_unsafe_precondition!(
"slice::from_raw_parts_mut requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`",
[T](data: *mut T, len: usize) => is_aligned_and_not_null(data)
&& is_valid_allocation_size::<T>(len)
);
&mut *ptr::slice_from_raw_parts_mut(data, len)
}
Expand Down
25 changes: 25 additions & 0 deletions library/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#![feature(is_terminal)]
#![feature(staged_api)]
#![feature(process_exitcode_internals)]
#![feature(panic_can_unwind)]
#![feature(test)]

// Public reexports
Expand Down Expand Up @@ -54,6 +55,7 @@ use std::{
collections::VecDeque,
env, io,
io::prelude::Write,
mem::ManuallyDrop,
panic::{self, catch_unwind, AssertUnwindSafe, PanicInfo},
process::{self, Command, Termination},
sync::mpsc::{channel, Sender},
Expand Down Expand Up @@ -112,6 +114,29 @@ pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Option<Opt
process::exit(ERROR_EXIT_CODE);
}
} else {
if !opts.nocapture {
// If we encounter a non-unwinding panic, flush any captured output from the current test,
// and stop capturing output to ensure that the non-unwinding panic message is visible.
// We also acquire the locks for both output streams to prevent output from other threads
// from interleaving with the panic message or appearing after it.
let builtin_panic_hook = panic::take_hook();
let hook = Box::new({
move |info: &'_ PanicInfo<'_>| {
if !info.can_unwind() {
std::mem::forget(std::io::stderr().lock());
let mut stdout = ManuallyDrop::new(std::io::stdout().lock());
if let Some(captured) = io::set_output_capture(None) {
if let Ok(data) = captured.lock() {
let _ = stdout.write_all(&data);
let _ = stdout.flush();
}
}
}
builtin_panic_hook(info);
}
});
panic::set_hook(hook);
}
match console::run_tests_console(&opts, tests) {
Ok(true) => {}
Ok(false) => process::exit(ERROR_EXIT_CODE),
Expand Down