Skip to content

Commit

Permalink
Auto merge of #120594 - saethlin:delayed-debug-asserts, r=<try>
Browse files Browse the repository at this point in the history
Toggle assert_unsafe_precondition in codegen instead of expansion

r? `@ghost`

#120539 (comment)
  • Loading branch information
bors committed Feb 3, 2024
2 parents b11fbfb + 3ef4a60 commit 4da3573
Show file tree
Hide file tree
Showing 31 changed files with 1,012 additions and 1,206 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,14 @@ fn codegen_regular_intrinsic_call<'tcx>(
fx.bcx.ins().trap(TrapCode::User(0));
return;
}
sym::debug_assertions => {
let bool_layout = fx.layout_of(fx.tcx.types.bool);
let val = CValue::by_val(
fx.bcx.ins().iconst(types::I8, fx.tcx.sess.opts.debug_assertions as i64),
bool_layout,
);
ret.write_cvalue(fx, val);
}
sym::likely | sym::unlikely => {
intrinsic_args!(fx, args => (a); intrinsic);

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/mir/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
return;
}

sym::debug_assertions => bx.const_bool(bx.tcx().sess.opts.debug_assertions),
sym::va_start => bx.va_start(args[0].immediate()),
sym::va_end => bx.va_end(args[0].immediate()),
sym::size_of_val => {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// (We know the value here in the machine of course, but this is the runtime of that code,
// not the optimization stage.)
sym::is_val_statically_known => ecx.write_scalar(Scalar::from_bool(false), dest)?,

sym::debug_assertions => {
ecx.write_scalar(Scalar::from_bool(ecx.tcx.sess.opts.debug_assertions), dest)?
}

_ => {
throw_unsup_format!(
"intrinsic `{intrinsic_name}` is not supported at compile-time"
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: DefId) -> hir
| sym::forget
| sym::black_box
| sym::variant_count
| sym::ptr_mask => hir::Unsafety::Normal,
| sym::ptr_mask
| sym::debug_assertions => hir::Unsafety::Normal,
_ => hir::Unsafety::Unsafe,
};

Expand Down Expand Up @@ -461,6 +462,8 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
(0, vec![Ty::new_imm_ptr(tcx, Ty::new_unit(tcx))], tcx.types.usize)
}

sym::debug_assertions => (0, Vec::new(), tcx.types.bool),

other => {
tcx.dcx().emit_err(UnrecognizedIntrinsicFunction { span: it.span, name: other });
return;
Expand Down
24 changes: 21 additions & 3 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2569,6 +2569,12 @@ extern "rust-intrinsic" {
#[rustc_nounwind]
#[cfg(not(bootstrap))]
pub fn is_val_statically_known<T: Copy>(arg: T) -> bool;

#[rustc_const_unstable(feature = "delayed_debug_assertions", issue = "none")]
#[rustc_safe_intrinsic]
#[rustc_nounwind]
#[cfg(not(bootstrap))]
pub(crate) fn debug_assertions() -> bool;
}

// FIXME: Seems using `unstable` here completely ignores `rustc_allow_const_fn_unstable`
Expand Down Expand Up @@ -2604,10 +2610,18 @@ pub const unsafe fn is_val_statically_known<T: Copy>(_arg: T) -> bool {
///
/// So in a sense it is UB if this macro is useful, but we expect callers of `unsafe fn` to make
/// 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
#[allow_internal_unstable(const_eval_select, delayed_debug_assertions)] // permit this to be called in stably-const fn
macro_rules! assert_unsafe_precondition {
($name:expr, $([$($tt:tt)*])?($($i:ident:$ty:ty),*$(,)?) => $e:expr $(,)?) => {
if cfg!(debug_assertions) {
{
#[cfg(bootstrap)]
let should_check = cfg!(debug_assertions);

// Turn assertions off in Miri, but otherwise check in codegen
#[cfg(not(bootstrap))]
let should_check = !cfg!(miri) && ::core::intrinsics::debug_assertions();

if should_check {
// allow non_snake_case to allow capturing const generics
#[allow(non_snake_case)]
#[inline(always)]
Expand All @@ -2625,6 +2639,7 @@ macro_rules! assert_unsafe_precondition {

::core::intrinsics::const_eval_select(($($i,)*), comptime, runtime);
}
}
};
}
pub(crate) use assert_unsafe_precondition;
Expand All @@ -2633,7 +2648,10 @@ pub(crate) use assert_unsafe_precondition;
/// `align_of::<T>()`.
#[inline]
pub(crate) fn is_aligned_and_not_null<T>(ptr: *const T) -> bool {
!ptr.is_null() && ptr.is_aligned()
let mask = crate::mem::align_of::<T>() - 1;
let is_aligned = (ptr.addr() & mask) == 0;
let not_null = ptr.addr() != 0;
is_aligned && not_null
}

/// Checks whether an allocation of `len` instances of `T` exceeds
Expand Down
4 changes: 4 additions & 0 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1208,10 +1208,12 @@ pub const unsafe fn read<T>(src: *const T) -> T {

// SAFETY: the caller must guarantee that `src` is valid for reads.
unsafe {
/*
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)
);
*/
crate::intrinsics::read_via_copy(src)
}
}
Expand Down Expand Up @@ -1408,10 +1410,12 @@ 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!(
"ptr::write requires that the pointer argument is aligned and non-null",
[T](dst: *mut T) => is_aligned_and_not_null(dst)
);
*/
intrinsics::write_via_move(dst, src)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,28 @@
debug ptr => _6;
scope 11 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: *const [bool; 0];
let mut _9: *mut [bool; 0];
let mut _9: *const [bool; 0];
scope 12 {
scope 13 (inlined NonNull::<T>::new_unchecked::runtime::<[bool; 0]>) {
debug ptr => _9;
scope 14 (inlined std::ptr::mut_ptr::<impl *mut [bool; 0]>::is_null) {
debug self => _9;
let mut _10: *mut u8;
scope 15 {
scope 16 (inlined std::ptr::mut_ptr::<impl *mut T>::is_null::runtime_impl) {
debug ptr => _10;
scope 17 (inlined std::ptr::mut_ptr::<impl *mut u8>::addr) {
debug self => _10;
scope 18 {
scope 19 (inlined std::ptr::mut_ptr::<impl *mut u8>::cast::<()>) {
debug self => _10;
let _8: bool;
scope 13 {
debug should_check => _8;
scope 14 (inlined NonNull::<T>::new_unchecked::runtime::<[bool; 0]>) {
debug ptr => _6;
let _10: !;
scope 15 (inlined std::ptr::mut_ptr::<impl *mut [bool; 0]>::is_null) {
debug self => _6;
let mut _11: *mut u8;
scope 16 {
scope 17 (inlined std::ptr::mut_ptr::<impl *mut T>::is_null::runtime_impl) {
debug ptr => _11;
let mut _12: usize;
scope 18 (inlined std::ptr::mut_ptr::<impl *mut u8>::addr) {
debug self => _11;
let mut _13: *mut ();
scope 19 {
scope 20 (inlined std::ptr::mut_ptr::<impl *mut u8>::cast::<()>) {
debug self => _11;
}
}
}
}
Expand All @@ -66,6 +72,7 @@
StorageLive(_1);
StorageLive(_2);
StorageLive(_3);
StorageLive(_10);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
Expand All @@ -75,10 +82,33 @@
StorageDead(_7);
StorageLive(_8);
StorageLive(_9);
StorageLive(_10);
_8 = const {0x1 as *mut [bool; 0]} as *const [bool; 0] (PointerCoercion(MutToConstPointer));
_5 = NonNull::<[bool; 0]> { pointer: _8 };
StorageDead(_10);
_8 = intrinsics::debug_assertions() -> [return: bb2, unwind unreachable];
}

bb1: {
StorageDead(_1);
return;
}

bb2: {
switchInt(_8) -> [0: bb4, otherwise: bb3];
}

bb3: {
StorageLive(_11);
_11 = const {0x1 as *mut [bool; 0]} as *mut u8 (PtrToPtr);
StorageLive(_12);
StorageLive(_13);
_13 = _11 as *mut () (PtrToPtr);
_12 = move _13 as usize (Transmute);
StorageDead(_13);
StorageDead(_11);
switchInt(move _12) -> [0: bb5, otherwise: bb6];
}

bb4: {
_9 = const {0x1 as *mut [bool; 0]} as *const [bool; 0] (PointerCoercion(MutToConstPointer));
_5 = NonNull::<[bool; 0]> { pointer: _9 };
StorageDead(_9);
StorageDead(_8);
StorageDead(_6);
Expand All @@ -87,16 +117,22 @@
_3 = move _4 as std::ptr::Unique<[bool]> (PointerCoercion(Unsize));
StorageDead(_4);
_2 = Box::<[bool]>(_3, const std::alloc::Global);
StorageDead(_10);
StorageDead(_3);
_1 = A { foo: move _2 };
StorageDead(_2);
_0 = const ();
drop(_1) -> [return: bb1, unwind unreachable];
}

bb1: {
StorageDead(_1);
return;
bb5: {
StorageDead(_12);
_10 = core::panicking::panic_nounwind(const "unsafe precondition(s) violated: NonNull::new_unchecked requires that the pointer is non-null") -> unwind unreachable;
}

bb6: {
StorageDead(_12);
goto -> bb4;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,28 @@
debug ptr => _6;
scope 11 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: *const [bool; 0];
let mut _9: *mut [bool; 0];
let mut _9: *const [bool; 0];
scope 12 {
scope 13 (inlined NonNull::<T>::new_unchecked::runtime::<[bool; 0]>) {
debug ptr => _9;
scope 14 (inlined std::ptr::mut_ptr::<impl *mut [bool; 0]>::is_null) {
debug self => _9;
let mut _10: *mut u8;
scope 15 {
scope 16 (inlined std::ptr::mut_ptr::<impl *mut T>::is_null::runtime_impl) {
debug ptr => _10;
scope 17 (inlined std::ptr::mut_ptr::<impl *mut u8>::addr) {
debug self => _10;
scope 18 {
scope 19 (inlined std::ptr::mut_ptr::<impl *mut u8>::cast::<()>) {
debug self => _10;
let _8: bool;
scope 13 {
debug should_check => _8;
scope 14 (inlined NonNull::<T>::new_unchecked::runtime::<[bool; 0]>) {
debug ptr => _6;
let _10: !;
scope 15 (inlined std::ptr::mut_ptr::<impl *mut [bool; 0]>::is_null) {
debug self => _6;
let mut _11: *mut u8;
scope 16 {
scope 17 (inlined std::ptr::mut_ptr::<impl *mut T>::is_null::runtime_impl) {
debug ptr => _11;
let mut _12: usize;
scope 18 (inlined std::ptr::mut_ptr::<impl *mut u8>::addr) {
debug self => _11;
let mut _13: *mut ();
scope 19 {
scope 20 (inlined std::ptr::mut_ptr::<impl *mut u8>::cast::<()>) {
debug self => _11;
}
}
}
}
Expand All @@ -66,6 +72,7 @@
StorageLive(_1);
StorageLive(_2);
StorageLive(_3);
StorageLive(_10);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
Expand All @@ -75,10 +82,37 @@
StorageDead(_7);
StorageLive(_8);
StorageLive(_9);
StorageLive(_10);
_8 = const {0x1 as *mut [bool; 0]} as *const [bool; 0] (PointerCoercion(MutToConstPointer));
_5 = NonNull::<[bool; 0]> { pointer: _8 };
StorageDead(_10);
_8 = intrinsics::debug_assertions() -> [return: bb3, unwind unreachable];
}

bb1: {
StorageDead(_1);
return;
}

bb2 (cleanup): {
resume;
}

bb3: {
switchInt(_8) -> [0: bb5, otherwise: bb4];
}

bb4: {
StorageLive(_11);
_11 = const {0x1 as *mut [bool; 0]} as *mut u8 (PtrToPtr);
StorageLive(_12);
StorageLive(_13);
_13 = _11 as *mut () (PtrToPtr);
_12 = move _13 as usize (Transmute);
StorageDead(_13);
StorageDead(_11);
switchInt(move _12) -> [0: bb6, otherwise: bb7];
}

bb5: {
_9 = const {0x1 as *mut [bool; 0]} as *const [bool; 0] (PointerCoercion(MutToConstPointer));
_5 = NonNull::<[bool; 0]> { pointer: _9 };
StorageDead(_9);
StorageDead(_8);
StorageDead(_6);
Expand All @@ -87,20 +121,22 @@
_3 = move _4 as std::ptr::Unique<[bool]> (PointerCoercion(Unsize));
StorageDead(_4);
_2 = Box::<[bool]>(_3, const std::alloc::Global);
StorageDead(_10);
StorageDead(_3);
_1 = A { foo: move _2 };
StorageDead(_2);
_0 = const ();
drop(_1) -> [return: bb1, unwind: bb2];
}

bb1: {
StorageDead(_1);
return;
bb6: {
StorageDead(_12);
_10 = core::panicking::panic_nounwind(const "unsafe precondition(s) violated: NonNull::new_unchecked requires that the pointer is non-null") -> unwind unreachable;
}

bb2 (cleanup): {
resume;
bb7: {
StorageDead(_12);
goto -> bb5;
}
}

Loading

0 comments on commit 4da3573

Please sign in to comment.