Skip to content

Commit 8fb67fb

Browse files
committed
Auto merge of rust-lang#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. rust-lang#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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 9 additions & 0 deletions
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

Lines changed: 10 additions & 4 deletions
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

Lines changed: 5 additions & 2 deletions
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

Lines changed: 16 additions & 4 deletions
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

Lines changed: 4 additions & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 4 additions & 1 deletion
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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 2 additions & 0 deletions
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)]

0 commit comments

Comments
 (0)