Skip to content

Commit

Permalink
Rollup merge of #131384 - saethlin:precondition-tests, r=ibraheemdev
Browse files Browse the repository at this point in the history
Update precondition tests (especially for zero-size access to null)

I don't much like the current way I've updated the precondition check helpers, but I couldn't come up with anything better. Ideas welcome.

I've organized `tests/ui/precondition-checks` mostly with one file per function that has `assert_unsafe_precondition` in it, with revisions that check each precondition. The important new test is `tests/ui/precondition-checks/zero-size-null.rs`.
  • Loading branch information
matthiaskrgr authored Oct 14, 2024
2 parents 43bf4f1 + a3606d7 commit 32062b4
Show file tree
Hide file tree
Showing 39 changed files with 500 additions and 55 deletions.
2 changes: 1 addition & 1 deletion library/core/src/ascii/ascii_char.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ impl AsciiChar {
pub const unsafe fn digit_unchecked(d: u8) -> Self {
assert_unsafe_precondition!(
check_language_ub,
"`AsciiChar::digit_unchecked` input cannot exceed 9.",
"`ascii::Char::digit_unchecked` input cannot exceed 9.",
(d: u8 = d) => d < 10
);

Expand Down
19 changes: 12 additions & 7 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
#![allow(missing_docs)]

use crate::marker::{DiscriminantKind, Tuple};
use crate::mem::SizedTypeProperties;
use crate::{ptr, ub_checks};

pub mod mir;
Expand Down Expand Up @@ -3364,10 +3365,12 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
size: usize = size_of::<T>(),
align: usize = align_of::<T>(),
count: usize = count,
) =>
ub_checks::is_aligned_and_not_null(src, align)
&& ub_checks::is_aligned_and_not_null(dst, align)
&& ub_checks::is_nonoverlapping(src, dst, size, count)
) => {
let zero_size = count == 0 || size == 0;
ub_checks::is_aligned_and_not_null(src, align, zero_size)
&& ub_checks::is_aligned_and_not_null(dst, align, zero_size)
&& ub_checks::is_nonoverlapping(src, dst, size, count)
}
);

// SAFETY: the safety contract for `copy_nonoverlapping` must be
Expand Down Expand Up @@ -3465,9 +3468,10 @@ pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
src: *const () = src as *const (),
dst: *mut () = dst as *mut (),
align: usize = align_of::<T>(),
zero_size: bool = T::IS_ZST || count == 0,
) =>
ub_checks::is_aligned_and_not_null(src, align)
&& ub_checks::is_aligned_and_not_null(dst, align)
ub_checks::is_aligned_and_not_null(src, align, zero_size)
&& ub_checks::is_aligned_and_not_null(dst, align, zero_size)
);
copy(src, dst, count)
}
Expand Down Expand Up @@ -3544,7 +3548,8 @@ pub const unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {
(
addr: *const () = dst as *const (),
align: usize = align_of::<T>(),
) => ub_checks::is_aligned_and_not_null(addr, align)
zero_size: bool = T::IS_ZST || count == 0,
) => ub_checks::is_aligned_and_not_null(addr, align, zero_size)
);
write_bytes(dst, val, count)
}
Expand Down
27 changes: 17 additions & 10 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@

use crate::cmp::Ordering;
use crate::marker::FnPtr;
use crate::mem::{self, MaybeUninit};
use crate::mem::{self, MaybeUninit, SizedTypeProperties};
use crate::{fmt, hash, intrinsics, ub_checks};

mod alignment;
Expand Down Expand Up @@ -1165,10 +1165,12 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
size: usize = size_of::<T>(),
align: usize = align_of::<T>(),
count: usize = count,
) =>
ub_checks::is_aligned_and_not_null(x, align)
&& ub_checks::is_aligned_and_not_null(y, align)
&& ub_checks::is_nonoverlapping(x, y, size, count)
) => {
let zero_size = size == 0 || count == 0;
ub_checks::is_aligned_and_not_null(x, align, zero_size)
&& ub_checks::is_aligned_and_not_null(y, align, zero_size)
&& ub_checks::is_nonoverlapping(x, y, size, count)
}
);

// Split up the slice into small power-of-two-sized chunks that LLVM is able
Expand Down Expand Up @@ -1278,7 +1280,8 @@ pub const unsafe fn replace<T>(dst: *mut T, src: T) -> T {
(
addr: *const () = dst as *const (),
align: usize = align_of::<T>(),
) => ub_checks::is_aligned_and_not_null(addr, align)
is_zst: bool = T::IS_ZST,
) => ub_checks::is_aligned_and_not_null(addr, align, is_zst)
);
mem::replace(&mut *dst, src)
}
Expand Down Expand Up @@ -1430,7 +1433,8 @@ pub const unsafe fn read<T>(src: *const T) -> T {
(
addr: *const () = src as *const (),
align: usize = align_of::<T>(),
) => ub_checks::is_aligned_and_not_null(addr, align)
is_zst: bool = T::IS_ZST,
) => ub_checks::is_aligned_and_not_null(addr, align, is_zst)
);
crate::intrinsics::read_via_copy(src)
}
Expand Down Expand Up @@ -1635,7 +1639,8 @@ pub const unsafe fn write<T>(dst: *mut T, src: T) {
(
addr: *mut () = dst as *mut (),
align: usize = align_of::<T>(),
) => ub_checks::is_aligned_and_not_null(addr, align)
is_zst: bool = T::IS_ZST,
) => ub_checks::is_aligned_and_not_null(addr, align, is_zst)
);
intrinsics::write_via_move(dst, src)
}
Expand Down Expand Up @@ -1808,7 +1813,8 @@ pub unsafe fn read_volatile<T>(src: *const T) -> T {
(
addr: *const () = src as *const (),
align: usize = align_of::<T>(),
) => ub_checks::is_aligned_and_not_null(addr, align)
is_zst: bool = T::IS_ZST,
) => ub_checks::is_aligned_and_not_null(addr, align, is_zst)
);
intrinsics::volatile_load(src)
}
Expand Down Expand Up @@ -1887,7 +1893,8 @@ pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
(
addr: *mut () = dst as *mut (),
align: usize = align_of::<T>(),
) => ub_checks::is_aligned_and_not_null(addr, align)
is_zst: bool = T::IS_ZST,
) => ub_checks::is_aligned_and_not_null(addr, align, is_zst)
);
intrinsics::volatile_store(dst, src);
}
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/slice/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ pub const unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T]
align: usize = align_of::<T>(),
len: usize = len,
) =>
ub_checks::is_aligned_and_not_null(data, align)
ub_checks::is_aligned_and_not_null(data, align, false)
&& ub_checks::is_valid_allocation_size(size, len)
);
&*ptr::slice_from_raw_parts(data, len)
Expand Down Expand Up @@ -187,7 +187,7 @@ pub const unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a m
align: usize = align_of::<T>(),
len: usize = len,
) =>
ub_checks::is_aligned_and_not_null(data, align)
ub_checks::is_aligned_and_not_null(data, align, false)
&& ub_checks::is_valid_allocation_size(size, len)
);
&mut *ptr::slice_from_raw_parts_mut(data, len)
Expand Down
8 changes: 4 additions & 4 deletions library/core/src/ub_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,15 @@ pub(crate) const fn check_language_ub() -> bool {
intrinsics::ub_checks() && const_eval_select((), comptime, runtime)
}

/// Checks whether `ptr` is properly aligned with respect to
/// `align_of::<T>()`.
/// Checks whether `ptr` is properly aligned with respect to the given alignment, and
/// if `is_zst == false`, that `ptr` is not null.
///
/// In `const` this is approximate and can fail spuriously. It is primarily intended
/// for `assert_unsafe_precondition!` with `check_language_ub`, in which case the
/// check is anyway not executed in `const`.
#[inline]
pub(crate) const fn is_aligned_and_not_null(ptr: *const (), align: usize) -> bool {
!ptr.is_null() && ptr.is_aligned_to(align)
pub(crate) const fn is_aligned_and_not_null(ptr: *const (), align: usize, is_zst: bool) -> bool {
ptr.is_aligned_to(align) && (is_zst || !ptr.is_null())
}

#[inline]
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/precondition-checks/alignment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: Alignment::new_unchecked requires

#![feature(ptr_alignment_type)]

fn main() {
unsafe {
std::ptr::Alignment::new_unchecked(0);
}
}
11 changes: 11 additions & 0 deletions tests/ui/precondition-checks/ascii-char-digit_unchecked.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: `ascii::Char::digit_unchecked` input cannot exceed 9

#![feature(ascii_char)]

fn main() {
unsafe {
std::ascii::Char::digit_unchecked(b'a');
}
}
9 changes: 9 additions & 0 deletions tests/ui/precondition-checks/assert_unchecked.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: hint::assert_unchecked must never be called when the condition is false

fn main() {
unsafe {
std::hint::assert_unchecked(false);
}
}
9 changes: 9 additions & 0 deletions tests/ui/precondition-checks/char-from_u32_unchecked.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: invalid value for `char`

fn main() {
unsafe {
char::from_u32_unchecked(0xD801);
}
}
25 changes: 25 additions & 0 deletions tests/ui/precondition-checks/copy-nonoverlapping.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: ptr::copy_nonoverlapping requires
//@ revisions: null_src null_dst misaligned_src misaligned_dst overlapping

use std::ptr;

fn main() {
let src = [0u16; 3];
let mut dst = [0u16; 3];
let src = src.as_ptr();
let dst = dst.as_mut_ptr();
unsafe {
#[cfg(null_src)]
ptr::copy_nonoverlapping(ptr::null(), dst, 1);
#[cfg(null_dst)]
ptr::copy_nonoverlapping(src, ptr::null_mut(), 1);
#[cfg(misaligned_src)]
ptr::copy_nonoverlapping(src.byte_add(1), dst, 1);
#[cfg(misaligned_dst)]
ptr::copy_nonoverlapping(src, dst.byte_add(1), 1);
#[cfg(overlapping)]
ptr::copy_nonoverlapping(dst, dst.add(1), 2);
}
}
23 changes: 23 additions & 0 deletions tests/ui/precondition-checks/copy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: ptr::copy requires
//@ revisions: null_src null_dst misaligned_src misaligned_dst

use std::ptr;

fn main() {
let src = [0u16; 3];
let mut dst = [0u16; 3];
let src = src.as_ptr();
let dst = dst.as_mut_ptr();
unsafe {
#[cfg(null_src)]
ptr::copy(ptr::null(), dst, 1);
#[cfg(null_dst)]
ptr::copy(src, ptr::null_mut(), 1);
#[cfg(misaligned_src)]
ptr::copy(src.byte_add(1), dst, 1);
#[cfg(misaligned_dst)]
ptr::copy(src, dst.byte_add(1), 1);
}
}
15 changes: 15 additions & 0 deletions tests/ui/precondition-checks/layout.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: Layout::from_size_align_unchecked requires
//@ revisions: toolarge badalign
//@[toolarge] compile-flags: --cfg toolarge
//@[badalign] compile-flags: --cfg badalign

fn main() {
unsafe {
#[cfg(toolarge)]
std::alloc::Layout::from_size_align_unchecked(isize::MAX as usize, 2);
#[cfg(badalign)]
std::alloc::Layout::from_size_align_unchecked(1, 3);
}
}
10 changes: 0 additions & 10 deletions tests/ui/precondition-checks/misaligned-slice.rs

This file was deleted.

9 changes: 9 additions & 0 deletions tests/ui/precondition-checks/nonnull.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: NonNull::new_unchecked requires

fn main() {
unsafe {
std::ptr::NonNull::new_unchecked(std::ptr::null_mut::<u8>());
}
}
12 changes: 12 additions & 0 deletions tests/ui/precondition-checks/nonzero-from_mut_unchecked.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: NonZero::from_mut_unchecked requires

#![feature(nonzero_from_mut)]

fn main() {
unsafe {
let mut num = 0u8;
std::num::NonZeroU8::from_mut_unchecked(&mut num);
}
}
9 changes: 9 additions & 0 deletions tests/ui/precondition-checks/nonzero-new_unchecked.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: NonZero::new_unchecked requires

fn main() {
unsafe {
std::num::NonZeroU8::new_unchecked(0);
}
}
10 changes: 0 additions & 10 deletions tests/ui/precondition-checks/null-slice.rs

This file was deleted.

11 changes: 0 additions & 11 deletions tests/ui/precondition-checks/out-of-bounds-get-unchecked.rs

This file was deleted.

18 changes: 18 additions & 0 deletions tests/ui/precondition-checks/read.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: ptr::read requires
//@ revisions: null misaligned
//@ ignore-test

use std::ptr;

fn main() {
let src = [0u16; 2];
let src = src.as_ptr();
unsafe {
#[cfg(null)]
ptr::read(ptr::null::<u8>());
#[cfg(misaligned)]
ptr::read(src.byte_add(1));
}
}
17 changes: 17 additions & 0 deletions tests/ui/precondition-checks/read_volatile.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//@ run-fail
//@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes
//@ error-pattern: unsafe precondition(s) violated: ptr::read_volatile requires
//@ revisions: null misaligned

use std::ptr;

fn main() {
let src = [0u16; 2];
let src = src.as_ptr();
unsafe {
#[cfg(null)]
ptr::read_volatile(ptr::null::<u8>());
#[cfg(misaligned)]
ptr::read_volatile(src.byte_add(1));
}
}
Loading

0 comments on commit 32062b4

Please sign in to comment.