Skip to content

Commit 8fb67fb

Browse files
borsbors
committed
Auto merge of #120594 - saethlin:delayed-debug-asserts, r=oli-obk
Toggle assert_unsafe_precondition in codegen instead of expansion The goal of this PR is to make some of the unsafe precondition checks in the standard library available in debug builds. Some UI tests are included to verify that it does that. The diff is large, but most of it is blessing mir-opt tests and I've also split up this PR so it can be reviewed commit-by-commit. This PR: 1. Adds a new intrinsic, `debug_assertions` which is lowered to a new MIR NullOp, and only to a constant after monomorphization 2. Rewrites `assume_unsafe_precondition` to check the new intrinsic, and be monomorphic. 3. Skips codegen of the `assume` intrinsic in unoptimized builds, because that was silly before but with these checks it's *very* silly 4. The checks with the most overhead are `ptr::read`/`ptr::write` and `NonNull::new_unchecked`. I've simply added `#[cfg(debug_assertions)]` to the checks for `ptr::read`/`ptr::write` because I was unable to come up with any (good) ideas for decreasing their impact. But for `NonNull::new_unchecked` I found that the majority of callers can use a different function, often a safe one. Yes, this PR slows down the compile time of some programs. But in our benchmark suite it's never more than 1% icount, and the average icount change in debug-full programs is 0.22%. I think that is acceptable for such an improvement in developer experience. #120539 (comment)
2 parents 972452c + dbf817b commit 8fb67fb

File tree

65 files changed

+1439
-1252
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+1439
-1252
lines changed

‎compiler/rustc_borrowck/src/type_check/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1983,6 +1983,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
19831983
ConstraintCategory::SizedBound,
19841984
);
19851985
}
1986+
&Rvalue::NullaryOp(NullOp::DebugAssertions, _) => {}
19861987

19871988
Rvalue::ShallowInitBox(operand, ty) => {
19881989
self.check_operand(operand, location);

‎compiler/rustc_codegen_cranelift/src/base.rs

+9
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,15 @@ fn codegen_stmt<'tcx>(
767767
NullOp::OffsetOf(fields) => {
768768
layout.offset_of_subfield(fx, fields.iter()).bytes()
769769
}
770+
NullOp::DebugAssertions => {
771+
let val = fx.tcx.sess.opts.debug_assertions;
772+
let val = CValue::by_val(
773+
fx.bcx.ins().iconst(types::I8, i64::try_from(val).unwrap()),
774+
fx.layout_of(fx.tcx.types.bool),
775+
);
776+
lval.write_cvalue(fx, val);
777+
return;
778+
}
770779
};
771780
let val = CValue::by_val(
772781
fx.bcx.ins().iconst(fx.pointer_type, i64::try_from(val).unwrap()),

‎compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -672,17 +672,23 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
672672
let val = match null_op {
673673
mir::NullOp::SizeOf => {
674674
assert!(bx.cx().type_is_sized(ty));
675-
layout.size.bytes()
675+
let val = layout.size.bytes();
676+
bx.cx().const_usize(val)
676677
}
677678
mir::NullOp::AlignOf => {
678679
assert!(bx.cx().type_is_sized(ty));
679-
layout.align.abi.bytes()
680+
let val = layout.align.abi.bytes();
681+
bx.cx().const_usize(val)
680682
}
681683
mir::NullOp::OffsetOf(fields) => {
682-
layout.offset_of_subfield(bx.cx(), fields.iter()).bytes()
684+
let val = layout.offset_of_subfield(bx.cx(), fields.iter()).bytes();
685+
bx.cx().const_usize(val)
686+
}
687+
mir::NullOp::DebugAssertions => {
688+
let val = bx.tcx().sess.opts.debug_assertions;
689+
bx.cx().const_bool(val)
683690
}
684691
};
685-
let val = bx.cx().const_usize(val);
686692
let tcx = self.cx.tcx();
687693
OperandRef {
688694
val: OperandValue::Immediate(val),

‎compiler/rustc_codegen_ssa/src/mir/statement.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use rustc_middle::mir;
22
use rustc_middle::mir::NonDivergingIntrinsic;
3+
use rustc_session::config::OptLevel;
34

45
use super::FunctionCx;
56
use super::LocalRef;
@@ -67,8 +68,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
6768
self.codegen_coverage(bx, coverage, statement.source_info.scope);
6869
}
6970
mir::StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(ref op)) => {
70-
let op_val = self.codegen_operand(bx, op);
71-
bx.assume(op_val.immediate());
71+
if !matches!(bx.tcx().sess.opts.optimize, OptLevel::No | OptLevel::Less) {
72+
let op_val = self.codegen_operand(bx, op);
73+
bx.assume(op_val.immediate());
74+
}
7275
}
7376
mir::StatementKind::Intrinsic(box NonDivergingIntrinsic::CopyNonOverlapping(
7477
mir::CopyNonOverlapping { ref count, ref src, ref dst },

‎compiler/rustc_const_eval/src/interpret/step.rs

+16-4
Original file line numberDiff line numberDiff line change
@@ -246,13 +246,25 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
246246
);
247247
}
248248
let val = match null_op {
249-
mir::NullOp::SizeOf => layout.size.bytes(),
250-
mir::NullOp::AlignOf => layout.align.abi.bytes(),
249+
mir::NullOp::SizeOf => {
250+
let val = layout.size.bytes();
251+
Scalar::from_target_usize(val, self)
252+
}
253+
mir::NullOp::AlignOf => {
254+
let val = layout.align.abi.bytes();
255+
Scalar::from_target_usize(val, self)
256+
}
251257
mir::NullOp::OffsetOf(fields) => {
252-
layout.offset_of_subfield(self, fields.iter()).bytes()
258+
let val = layout.offset_of_subfield(self, fields.iter()).bytes();
259+
Scalar::from_target_usize(val, self)
260+
}
261+
mir::NullOp::DebugAssertions => {
262+
// The checks hidden behind this are always better done by the interpreter
263+
// itself, because it knows the runtime state better.
264+
Scalar::from_bool(false)
253265
}
254266
};
255-
self.write_scalar(Scalar::from_target_usize(val, self), &dest)?;
267+
self.write_scalar(val, &dest)?;
256268
}
257269

258270
ShallowInitBox(ref operand, _) => {

‎compiler/rustc_const_eval/src/transform/check_consts/check.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
544544

545545
Rvalue::Cast(_, _, _) => {}
546546

547-
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_), _) => {}
547+
Rvalue::NullaryOp(
548+
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::DebugAssertions,
549+
_,
550+
) => {}
548551
Rvalue::ShallowInitBox(_, _) => {}
549552

550553
Rvalue::UnaryOp(_, operand) => {

‎compiler/rustc_const_eval/src/transform/validate.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1139,7 +1139,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
11391139
Rvalue::Repeat(_, _)
11401140
| Rvalue::ThreadLocalRef(_)
11411141
| Rvalue::AddressOf(_, _)
1142-
| Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _)
1142+
| Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::DebugAssertions, _)
11431143
| Rvalue::Discriminant(_) => {}
11441144
}
11451145
self.super_rvalue(rvalue, location);

‎compiler/rustc_hir_analysis/src/check/intrinsic.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: DefId) -> hir
112112
| sym::forget
113113
| sym::black_box
114114
| sym::variant_count
115-
| sym::ptr_mask => hir::Unsafety::Normal,
115+
| sym::ptr_mask
116+
| sym::debug_assertions => hir::Unsafety::Normal,
116117
_ => hir::Unsafety::Unsafe,
117118
};
118119

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

465+
sym::debug_assertions => (0, Vec::new(), tcx.types.bool),
466+
464467
other => {
465468
tcx.dcx().emit_err(UnrecognizedIntrinsicFunction { span: it.span, name: other });
466469
return;

‎compiler/rustc_middle/src/mir/pretty.rs

+1
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
907907
NullOp::SizeOf => write!(fmt, "SizeOf({t})"),
908908
NullOp::AlignOf => write!(fmt, "AlignOf({t})"),
909909
NullOp::OffsetOf(fields) => write!(fmt, "OffsetOf({t}, {fields:?})"),
910+
NullOp::DebugAssertions => write!(fmt, "cfg!(debug_assertions)"),
910911
}
911912
}
912913
ThreadLocalRef(did) => ty::tls::with(|tcx| {

‎compiler/rustc_middle/src/mir/syntax.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1361,6 +1361,8 @@ pub enum NullOp<'tcx> {
13611361
AlignOf,
13621362
/// Returns the offset of a field
13631363
OffsetOf(&'tcx List<(VariantIdx, FieldIdx)>),
1364+
/// cfg!(debug_assertions), but expanded in codegen
1365+
DebugAssertions,
13641366
}
13651367

13661368
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]

‎compiler/rustc_middle/src/mir/tcx.rs

+1
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ impl<'tcx> Rvalue<'tcx> {
194194
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {
195195
tcx.types.usize
196196
}
197+
Rvalue::NullaryOp(NullOp::DebugAssertions, _) => tcx.types.bool,
197198
Rvalue::Aggregate(ref ak, ref ops) => match **ak {
198199
AggregateKind::Array(ty) => Ty::new_array(tcx, ty, ops.len() as u64),
199200
AggregateKind::Tuple => {

‎compiler/rustc_mir_dataflow/src/move_paths/builder.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,10 @@ impl<'b, 'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> Gatherer<'b, 'a, 'tcx, F> {
429429
| Rvalue::AddressOf(..)
430430
| Rvalue::Discriminant(..)
431431
| Rvalue::Len(..)
432-
| Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {}
432+
| Rvalue::NullaryOp(
433+
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..) | NullOp::DebugAssertions,
434+
_,
435+
) => {}
433436
}
434437
}
435438

‎compiler/rustc_mir_transform/src/const_prop_lint.rs

+1
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
634634
NullOp::OffsetOf(fields) => {
635635
op_layout.offset_of_subfield(self, fields.iter()).bytes()
636636
}
637+
NullOp::DebugAssertions => return None,
637638
};
638639
ImmTy::from_scalar(Scalar::from_target_usize(val, self), layout).into()
639640
}

‎compiler/rustc_mir_transform/src/gvn.rs

+1
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
489489
NullOp::OffsetOf(fields) => {
490490
layout.offset_of_subfield(&self.ecx, fields.iter()).bytes()
491491
}
492+
NullOp::DebugAssertions => return None,
492493
};
493494
let usize_layout = self.ecx.layout_of(self.tcx.types.usize).unwrap();
494495
let imm = ImmTy::try_from_uint(val, usize_layout)?;

‎compiler/rustc_mir_transform/src/instsimplify.rs

+25
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
33
use crate::simplify::simplify_duplicate_switch_targets;
44
use rustc_middle::mir::*;
5+
use rustc_middle::ty::layout;
56
use rustc_middle::ty::layout::ValidityRequirement;
67
use rustc_middle::ty::{self, GenericArgsRef, ParamEnv, Ty, TyCtxt};
78
use rustc_span::symbol::Symbol;
89
use rustc_target::abi::FieldIdx;
10+
use rustc_target::spec::abi::Abi;
911

1012
pub struct InstSimplify;
1113

@@ -38,6 +40,7 @@ impl<'tcx> MirPass<'tcx> for InstSimplify {
3840
block.terminator.as_mut().unwrap(),
3941
&mut block.statements,
4042
);
43+
ctx.simplify_nounwind_call(block.terminator.as_mut().unwrap());
4144
simplify_duplicate_switch_targets(block.terminator.as_mut().unwrap());
4245
}
4346
}
@@ -252,6 +255,28 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {
252255
terminator.kind = TerminatorKind::Goto { target: destination_block };
253256
}
254257

258+
fn simplify_nounwind_call(&self, terminator: &mut Terminator<'tcx>) {
259+
let TerminatorKind::Call { func, unwind, .. } = &mut terminator.kind else {
260+
return;
261+
};
262+
263+
let Some((def_id, _)) = func.const_fn_def() else {
264+
return;
265+
};
266+
267+
let body_ty = self.tcx.type_of(def_id).skip_binder();
268+
let body_abi = match body_ty.kind() {
269+
ty::FnDef(..) => body_ty.fn_sig(self.tcx).abi(),
270+
ty::Closure(..) => Abi::RustCall,
271+
ty::Coroutine(..) => Abi::Rust,
272+
_ => bug!("unexpected body ty: {:?}", body_ty),
273+
};
274+
275+
if !layout::fn_can_unwind(self.tcx, Some(def_id), body_abi) {
276+
*unwind = UnwindAction::Unreachable;
277+
}
278+
}
279+
255280
fn simplify_intrinsic_assert(
256281
&self,
257282
terminator: &mut Terminator<'tcx>,

‎compiler/rustc_mir_transform/src/lower_intrinsics.rs

+11
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,17 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
2121
sym::unreachable => {
2222
terminator.kind = TerminatorKind::Unreachable;
2323
}
24+
sym::debug_assertions => {
25+
let target = target.unwrap();
26+
block.statements.push(Statement {
27+
source_info: terminator.source_info,
28+
kind: StatementKind::Assign(Box::new((
29+
*destination,
30+
Rvalue::NullaryOp(NullOp::DebugAssertions, tcx.types.bool),
31+
))),
32+
});
33+
terminator.kind = TerminatorKind::Goto { target };
34+
}
2435
sym::forget => {
2536
if let Some(target) = *target {
2637
block.statements.push(Statement {

‎compiler/rustc_mir_transform/src/promote_consts.rs

+1
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ impl<'tcx> Validator<'_, 'tcx> {
446446
NullOp::SizeOf => {}
447447
NullOp::AlignOf => {}
448448
NullOp::OffsetOf(_) => {}
449+
NullOp::DebugAssertions => {}
449450
},
450451

451452
Rvalue::ShallowInitBox(_, _) => return Err(Unpromotable),

‎compiler/rustc_smir/src/rustc_smir/convert/mir.rs

+1
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ impl<'tcx> Stable<'tcx> for mir::NullOp<'tcx> {
257257
OffsetOf(indices) => stable_mir::mir::NullOp::OffsetOf(
258258
indices.iter().map(|idx| idx.stable(tables)).collect(),
259259
),
260+
DebugAssertions => stable_mir::mir::NullOp::DebugAssertions,
260261
}
261262
}
262263
}

‎compiler/stable_mir/src/mir/body.rs

+3
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,7 @@ impl Rvalue {
639639
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {
640640
Ok(Ty::usize_ty())
641641
}
642+
Rvalue::NullaryOp(NullOp::DebugAssertions, _) => Ok(Ty::bool_ty()),
642643
Rvalue::Aggregate(ak, ops) => match *ak {
643644
AggregateKind::Array(ty) => Ty::try_new_array(ty, ops.len() as u64),
644645
AggregateKind::Tuple => Ok(Ty::new_tuple(
@@ -1005,6 +1006,8 @@ pub enum NullOp {
10051006
AlignOf,
10061007
/// Returns the offset of a field.
10071008
OffsetOf(Vec<(VariantIdx, FieldIdx)>),
1009+
/// cfg!(debug_assertions), but at codegen time
1010+
DebugAssertions,
10081011
}
10091012

10101013
impl Operand {

‎library/alloc/src/raw_vec.rs

+7-6
Original file line numberDiff line numberDiff line change
@@ -207,11 +207,7 @@ impl<T, A: Allocator> RawVec<T, A> {
207207
// Allocators currently return a `NonNull<[u8]>` whose length
208208
// matches the size requested. If that ever changes, the capacity
209209
// here should change to `ptr.len() / mem::size_of::<T>()`.
210-
Self {
211-
ptr: unsafe { Unique::new_unchecked(ptr.cast().as_ptr()) },
212-
cap: unsafe { Cap(capacity) },
213-
alloc,
214-
}
210+
Self { ptr: Unique::from(ptr.cast()), cap: unsafe { Cap(capacity) }, alloc }
215211
}
216212
}
217213

@@ -239,6 +235,11 @@ impl<T, A: Allocator> RawVec<T, A> {
239235
self.ptr.as_ptr()
240236
}
241237

238+
#[inline]
239+
pub fn non_null(&self) -> NonNull<T> {
240+
NonNull::from(self.ptr)
241+
}
242+
242243
/// Gets the capacity of the allocation.
243244
///
244245
/// This will always be `usize::MAX` if `T` is zero-sized.
@@ -398,7 +399,7 @@ impl<T, A: Allocator> RawVec<T, A> {
398399
// Allocators currently return a `NonNull<[u8]>` whose length matches
399400
// the size requested. If that ever changes, the capacity here should
400401
// change to `ptr.len() / mem::size_of::<T>()`.
401-
self.ptr = unsafe { Unique::new_unchecked(ptr.cast().as_ptr()) };
402+
self.ptr = Unique::from(ptr.cast());
402403
self.cap = unsafe { Cap(cap) };
403404
}
404405

‎library/alloc/src/vec/into_iter.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
136136
// struct and then overwriting &mut self.
137137
// this creates less assembly
138138
self.cap = 0;
139-
self.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) };
139+
self.buf = RawVec::NEW.non_null();
140140
self.ptr = self.buf;
141141
self.end = self.buf.as_ptr();
142142

‎library/alloc/src/vec/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -2861,16 +2861,16 @@ impl<T, A: Allocator> IntoIterator for Vec<T, A> {
28612861
#[inline]
28622862
fn into_iter(self) -> Self::IntoIter {
28632863
unsafe {
2864-
let mut me = ManuallyDrop::new(self);
2864+
let me = ManuallyDrop::new(self);
28652865
let alloc = ManuallyDrop::new(ptr::read(me.allocator()));
2866-
let begin = me.as_mut_ptr();
2866+
let buf = me.buf.non_null();
2867+
let begin = buf.as_ptr();
28672868
let end = if T::IS_ZST {
28682869
begin.wrapping_byte_add(me.len())
28692870
} else {
28702871
begin.add(me.len()) as *const T
28712872
};
28722873
let cap = me.buf.capacity();
2873-
let buf = NonNull::new_unchecked(begin);
28742874
IntoIter { buf, phantom: PhantomData, cap, alloc, ptr: buf, end }
28752875
}
28762876
}

‎library/core/src/char/convert.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
2727
unsafe {
2828
assert_unsafe_precondition!(
2929
"invalid value for `char`",
30-
(i: u32) => char_try_from_u32(i).is_ok()
30+
(i: u32 = i) => char_try_from_u32(i).is_ok()
3131
);
3232
transmute(i)
3333
}

‎library/core/src/hint.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ pub const unsafe fn assert_unchecked(cond: bool) {
148148
unsafe {
149149
intrinsics::assert_unsafe_precondition!(
150150
"hint::assert_unchecked must never be called when the condition is false",
151-
(cond: bool) => cond,
151+
(cond: bool = cond) => cond,
152152
);
153153
crate::intrinsics::assume(cond);
154154
}

0 commit comments

Comments
 (0)