Rollup merge of rust-lang#103035 - saethlin:assert_unsafe_preconditio…

…n3, r=thomcc

Even nicer errors from assert_unsafe_precondition

For example, now running `cargo test` with this patch I get things like:
```
$ cargo +stage1 test
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests src/lib.rs (target/debug/deps/malloc_buf-9d105ddf86862995)

running 5 tests
thread 'tests::test_null_buf' panicked at 'unsafe precondition violated: is_aligned_and_not_null(data) &&
    crate::mem::size_of::<T>().saturating_mul(len) <= isize::MAX as usize', /home/ben/rust/library/core/src/slice/raw.rs:93:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread panicked while panicking. aborting.
error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/tmp/malloc_buf-1.0.0/target/debug/deps/malloc_buf-9d105ddf86862995` (signal: 6, SIGABRT: process abort signal)
```

This is still not perfect, but these are better for another PR:
* `stringify!` is trying to do clever pretty-printing on the `expr` inside `assert_unsafe_precondition` and can even add a newline.
* It would be nice to print a bit more information about where the problem is. Perhaps this is `cfg_attr(debug_assertions, track_caller)`, or perhaps it the function name added to `Location`.

cc `@RalfJung` this is what I was thinking of for rust-lang#102732 (comment)

library/core/src/hint.rs

@@ -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()
     }
 }

library/core/src/intrinsics.rs

@@ -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)]
@@ -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)
@@ -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)
     }
 }
@@ -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)
     }
 }

library/core/src/num/nonzero.rs

@@ -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)
                     }
                 }

library/core/src/ops/index_range.rs

@@ -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 }
     }
 

library/core/src/ptr/alignment.rs

@@ -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.

library/core/src/ptr/const_ptr.rs

@@ -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>();

library/core/src/ptr/mod.rs

@@ -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)
@@ -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
@@ -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()
     }
@@ -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);
     }
@@ -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)
     }
 }
@@ -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);
     }
 }

library/core/src/ptr/non_null.rs

@@ -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 _ }
         }
     }

library/core/src/slice/index.rs

@@ -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)
         }
     }
@@ -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)
         }
     }
@@ -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())
         }
     }
@@ -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())
         }
     }
@@ -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)
         }
     }
@@ -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)
         }
     }

library/core/src/slice/mod.rs

@@ -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));
         }
     }
@@ -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
@@ -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
@@ -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))
         }
     }

library/core/src/slice/raw.rs

@@ -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)
     }
@@ -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)
     }

library/test/src/lib.rs

@@ -20,6 +20,7 @@
 #![feature(is_terminal)]
 #![feature(staged_api)]
 #![feature(process_exitcode_internals)]
+#![feature(panic_can_unwind)]
 #![feature(test)]
 
 // Public reexports
@@ -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},
@@ -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),