Skip to content

Commit 2dec4af

Browse files
RalfJungRalfJung
committed
refactor check_{lang,library}_ub: use a single intrinsic, put policy into library
1 parent 470d7de commit 2dec4af

34 files changed

+134
-119
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2000,7 +2000,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
20002000
ConstraintCategory::SizedBound,
20012001
);
20022002
}
2003-
&Rvalue::NullaryOp(NullOp::UbCheck(_), _) => {}
2003+
&Rvalue::NullaryOp(NullOp::UbChecks, _) => {}
20042004

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

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -680,8 +680,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
680680
let val = layout.offset_of_subfield(bx.cx(), fields.iter()).bytes();
681681
bx.cx().const_usize(val)
682682
}
683-
mir::NullOp::UbCheck(_) => {
684-
// In codegen, we want to check for language UB and library UB
683+
mir::NullOp::UbChecks => {
685684
let val = bx.tcx().sess.opts.debug_assertions;
686685
bx.cx().const_bool(val)
687686
}

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -258,17 +258,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
258258
let val = layout.offset_of_subfield(self, fields.iter()).bytes();
259259
Scalar::from_target_usize(val, self)
260260
}
261-
mir::NullOp::UbCheck(kind) => {
262-
// We want to enable checks for library UB, because the interpreter doesn't
263-
// know about those on its own.
264-
// But we want to disable checks for language UB, because the interpreter
265-
// has its own better checks for that.
266-
let should_check = match kind {
267-
mir::UbKind::LibraryUb => self.tcx.sess.opts.debug_assertions,
268-
mir::UbKind::LanguageUb => false,
269-
};
270-
Scalar::from_bool(should_check)
271-
}
261+
mir::NullOp::UbChecks => Scalar::from_bool(self.tcx.sess.opts.debug_assertions),
272262
};
273263
self.write_scalar(val, &dest)?;
274264
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
558558
Rvalue::Cast(_, _, _) => {}
559559

560560
Rvalue::NullaryOp(
561-
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::UbCheck(_),
561+
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::UbChecks,
562562
_,
563563
) => {}
564564
Rvalue::ShallowInitBox(_, _) => {}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1168,7 +1168,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
11681168
Rvalue::Repeat(_, _)
11691169
| Rvalue::ThreadLocalRef(_)
11701170
| Rvalue::AddressOf(_, _)
1171-
| Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::UbCheck(_), _)
1171+
| Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::UbChecks, _)
11721172
| Rvalue::Discriminant(_) => {}
11731173
}
11741174
self.super_rvalue(rvalue, location);

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -
127127
| sym::variant_count
128128
| sym::is_val_statically_known
129129
| sym::ptr_mask
130-
| sym::check_language_ub
131-
| sym::check_library_ub
130+
| sym::ub_checks
132131
| sym::fadd_algebraic
133132
| sym::fsub_algebraic
134133
| sym::fmul_algebraic
@@ -569,7 +568,7 @@ pub fn check_intrinsic_type(
569568
(0, 0, vec![Ty::new_imm_ptr(tcx, Ty::new_unit(tcx))], tcx.types.usize)
570569
}
571570

572-
sym::check_language_ub | sym::check_library_ub => (0, 1, Vec::new(), tcx.types.bool),
571+
sym::ub_checks => (0, 1, Vec::new(), tcx.types.bool),
573572

574573
sym::simd_eq
575574
| sym::simd_ne

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ impl<'tcx> Body<'tcx> {
796796
}
797797

798798
match rvalue {
799-
Rvalue::NullaryOp(NullOp::UbCheck(_), _) => {
799+
Rvalue::NullaryOp(NullOp::UbChecks, _) => {
800800
Some((tcx.sess.opts.debug_assertions as u128, targets))
801801
}
802802
Rvalue::Use(Operand::Constant(constant)) => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -944,7 +944,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
944944
NullOp::SizeOf => write!(fmt, "SizeOf({t})"),
945945
NullOp::AlignOf => write!(fmt, "AlignOf({t})"),
946946
NullOp::OffsetOf(fields) => write!(fmt, "OffsetOf({t}, {fields:?})"),
947-
NullOp::UbCheck(kind) => write!(fmt, "UbCheck({kind:?})"),
947+
NullOp::UbChecks => write!(fmt, "UbChecks()"),
948948
}
949949
}
950950
ThreadLocalRef(did) => ty::tls::with(|tcx| {

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,16 +1367,9 @@ pub enum NullOp<'tcx> {
13671367
AlignOf,
13681368
/// Returns the offset of a field
13691369
OffsetOf(&'tcx List<(VariantIdx, FieldIdx)>),
1370-
/// Returns whether we want to check for library UB or language UB at monomorphization time.
1371-
/// Both kinds of UB evaluate to `true` in codegen, and only library UB evalutes to `true` in
1372-
/// const-eval/Miri, because the interpreter has its own better checks for language UB.
1373-
UbCheck(UbKind),
1374-
}
1375-
1376-
#[derive(Clone, Copy, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)]
1377-
pub enum UbKind {
1378-
LanguageUb,
1379-
LibraryUb,
1370+
/// Returns whether we want to check for UB.
1371+
/// This returns the value of `cfg!(debug_assertions)` at monomorphization time.
1372+
UbChecks,
13801373
}
13811374

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ impl<'tcx> Rvalue<'tcx> {
194194
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {
195195
tcx.types.usize
196196
}
197-
Rvalue::NullaryOp(NullOp::UbCheck(_), _) => tcx.types.bool,
197+
Rvalue::NullaryOp(NullOp::UbChecks, _) => tcx.types.bool,
198198
Rvalue::Aggregate(ref ak, ref ops) => match **ak {
199199
AggregateKind::Array(ty) => Ty::new_array(tcx, ty, ops.len() as u64),
200200
AggregateKind::Tuple => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ impl<'b, 'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> Gatherer<'b, 'a, 'tcx, F> {
433433
| Rvalue::Discriminant(..)
434434
| Rvalue::Len(..)
435435
| Rvalue::NullaryOp(
436-
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..) | NullOp::UbCheck(_),
436+
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..) | NullOp::UbChecks,
437437
_,
438438
) => {}
439439
}

‎compiler/rustc_mir_transform/src/gvn.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
487487
NullOp::OffsetOf(fields) => {
488488
layout.offset_of_subfield(&self.ecx, fields.iter()).bytes()
489489
}
490-
NullOp::UbCheck(_) => return None,
490+
NullOp::UbChecks => return None,
491491
};
492492
let usize_layout = self.ecx.layout_of(self.tcx.types.usize).unwrap();
493493
let imm = ImmTy::try_from_uint(val, usize_layout)?;

‎compiler/rustc_mir_transform/src/known_panics_lint.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
639639
NullOp::OffsetOf(fields) => {
640640
op_layout.offset_of_subfield(self, fields.iter()).bytes()
641641
}
642-
NullOp::UbCheck(_) => return None,
642+
NullOp::UbChecks => return None,
643643
};
644644
ImmTy::from_scalar(Scalar::from_target_usize(val, self), layout).into()
645645
}

‎compiler/rustc_mir_transform/src/lower_intrinsics.rs

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,30 +20,13 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
2020
sym::unreachable => {
2121
terminator.kind = TerminatorKind::Unreachable;
2222
}
23-
sym::check_language_ub => {
23+
sym::ub_checks => {
2424
let target = target.unwrap();
2525
block.statements.push(Statement {
2626
source_info: terminator.source_info,
2727
kind: StatementKind::Assign(Box::new((
2828
*destination,
29-
Rvalue::NullaryOp(
30-
NullOp::UbCheck(UbKind::LanguageUb),
31-
tcx.types.bool,
32-
),
33-
))),
34-
});
35-
terminator.kind = TerminatorKind::Goto { target };
36-
}
37-
sym::check_library_ub => {
38-
let target = target.unwrap();
39-
block.statements.push(Statement {
40-
source_info: terminator.source_info,
41-
kind: StatementKind::Assign(Box::new((
42-
*destination,
43-
Rvalue::NullaryOp(
44-
NullOp::UbCheck(UbKind::LibraryUb),
45-
tcx.types.bool,
46-
),
29+
Rvalue::NullaryOp(NullOp::UbChecks, tcx.types.bool),
4730
))),
4831
});
4932
terminator.kind = TerminatorKind::Goto { target };

‎compiler/rustc_mir_transform/src/promote_consts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ impl<'tcx> Validator<'_, 'tcx> {
446446
NullOp::SizeOf => {}
447447
NullOp::AlignOf => {}
448448
NullOp::OffsetOf(_) => {}
449-
NullOp::UbCheck(_) => {}
449+
NullOp::UbChecks => {}
450450
},
451451

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

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -251,19 +251,13 @@ impl<'tcx> Stable<'tcx> for mir::NullOp<'tcx> {
251251
type T = stable_mir::mir::NullOp;
252252
fn stable(&self, tables: &mut Tables<'_>) -> Self::T {
253253
use rustc_middle::mir::NullOp::*;
254-
use rustc_middle::mir::UbKind;
255254
match self {
256255
SizeOf => stable_mir::mir::NullOp::SizeOf,
257256
AlignOf => stable_mir::mir::NullOp::AlignOf,
258257
OffsetOf(indices) => stable_mir::mir::NullOp::OffsetOf(
259258
indices.iter().map(|idx| idx.stable(tables)).collect(),
260259
),
261-
UbCheck(UbKind::LanguageUb) => {
262-
stable_mir::mir::NullOp::UbCheck(stable_mir::mir::UbKind::LanguageUb)
263-
}
264-
UbCheck(UbKind::LibraryUb) => {
265-
stable_mir::mir::NullOp::UbCheck(stable_mir::mir::UbKind::LibraryUb)
266-
}
260+
UbChecks => stable_mir::mir::NullOp::UbChecks,
267261
}
268262
}
269263
}

‎compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,8 +518,6 @@ symbols! {
518518
cfi,
519519
cfi_encoding,
520520
char,
521-
check_language_ub,
522-
check_library_ub,
523521
client,
524522
clippy,
525523
clobber_abi,
@@ -1866,6 +1864,7 @@ symbols! {
18661864
u8_legacy_fn_max_value,
18671865
u8_legacy_fn_min_value,
18681866
u8_legacy_mod,
1867+
ub_checks,
18691868
unaligned_volatile_load,
18701869
unaligned_volatile_store,
18711870
unboxed_closures,

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ impl Rvalue {
621621
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {
622622
Ok(Ty::usize_ty())
623623
}
624-
Rvalue::NullaryOp(NullOp::UbCheck(_), _) => Ok(Ty::bool_ty()),
624+
Rvalue::NullaryOp(NullOp::UbChecks, _) => Ok(Ty::bool_ty()),
625625
Rvalue::Aggregate(ak, ops) => match *ak {
626626
AggregateKind::Array(ty) => Ty::try_new_array(ty, ops.len() as u64),
627627
AggregateKind::Tuple => Ok(Ty::new_tuple(
@@ -989,13 +989,7 @@ pub enum NullOp {
989989
/// Returns the offset of a field.
990990
OffsetOf(Vec<(VariantIdx, FieldIdx)>),
991991
/// cfg!(debug_assertions), but at codegen time
992-
UbCheck(UbKind),
993-
}
994-
995-
#[derive(Clone, Debug, Eq, PartialEq)]
996-
pub enum UbKind {
997-
LanguageUb,
998-
LibraryUb,
992+
UbChecks,
999993
}
1000994

1001995
impl Operand {

‎library/core/src/intrinsics.rs

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2639,38 +2639,22 @@ pub const fn is_val_statically_known<T: Copy>(_arg: T) -> bool {
26392639
false
26402640
}
26412641

2642-
/// Returns whether we should check for library UB. This evaluate to the value of `cfg!(debug_assertions)`
2643-
/// during monomorphization.
2644-
///
2645-
/// This intrinsic is evaluated after monomorphization, and therefore branching on this value can
2646-
/// be used to implement debug assertions that are included in the precompiled standard library,
2647-
/// but can be optimized out by builds that monomorphize the standard library code with debug
2648-
/// assertions disabled. This intrinsic is primarily used by [`assert_unsafe_precondition`].
2649-
///
2650-
/// We have separate intrinsics for library UB and language UB because checkers like the const-eval
2651-
/// interpreter and Miri already implement checks for language UB. Since such checkers do not know
2652-
/// about library preconditions, checks guarded by this intrinsic let them find more UB.
2653-
#[rustc_const_unstable(feature = "ub_checks", issue = "none")]
2642+
/// Returns whether we should perform some UB-checking at runtime. This evaluate to the value of
2643+
/// `cfg!(debug_assertions)` during monomorphization.
2644+
///
2645+
/// This intrinsic is evaluated after monomorphization, which is relevant when mixing crates
2646+
/// compiled with and without debug_assertions. The common case here is a user program built with
2647+
/// debug_assertions linked against the distributed sysroot which is built without debug_assertions.
2648+
/// For code that gets monomorphized in the user crate (i.e., generic functions and functions with
2649+
/// `#[inline]`), gating assertions on `ub_checks()` rather than `cfg!(debug_assertions)` means that
2650+
/// assertions are enabled whenever the *user crate* has debug assertions enabled. However if the
2651+
/// user has debug assertions disabled, the checks will still get optimized out. This intrinsic is
2652+
/// primarily used by [`ub_checks::assert_unsafe_precondition`].
2653+
#[rustc_const_unstable(feature = "const_ub_checks", issue = "none")]
26542654
#[unstable(feature = "core_intrinsics", issue = "none")]
26552655
#[inline(always)]
2656-
#[rustc_intrinsic]
2657-
pub(crate) const fn check_library_ub() -> bool {
2658-
cfg!(debug_assertions)
2659-
}
2660-
2661-
/// Returns whether we should check for language UB. This evaluate to the value of `cfg!(debug_assertions)`
2662-
/// during monomorphization.
2663-
///
2664-
/// Since checks implemented at the source level must come strictly before the operation that
2665-
/// executes UB, if we enabled language UB checks in const-eval/Miri we would miss out on the
2666-
/// interpreter's improved diagnostics for the cases that our source-level checks catch.
2667-
///
2668-
/// See `check_library_ub` for more information.
2669-
#[rustc_const_unstable(feature = "ub_checks", issue = "none")]
2670-
#[unstable(feature = "core_intrinsics", issue = "none")]
2671-
#[inline(always)]
2672-
#[rustc_intrinsic]
2673-
pub(crate) const fn check_language_ub() -> bool {
2656+
#[cfg_attr(not(bootstrap), rustc_intrinsic)] // just make it a regular fn in bootstrap
2657+
pub(crate) const fn ub_checks() -> bool {
26742658
cfg!(debug_assertions)
26752659
}
26762660

‎library/core/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
#![feature(const_try)]
171171
#![feature(const_type_id)]
172172
#![feature(const_type_name)]
173+
#![feature(const_ub_checks)]
173174
#![feature(const_unicode_case_lookup)]
174175
#![feature(const_unsafecell_get_mut)]
175176
#![feature(const_waker)]

‎library/core/src/ub_checks.rs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Provides the [`assert_unsafe_precondition`] macro as well as some utility functions that cover
22
//! common preconditions.
33
4-
use crate::intrinsics::const_eval_select;
4+
use crate::intrinsics::{self, const_eval_select};
55

66
/// Check that the preconditions of an unsafe function are followed. The check is enabled at
77
/// runtime if debug assertions are enabled when the caller is monomorphized. In const-eval/Miri
@@ -45,7 +45,7 @@ use crate::intrinsics::const_eval_select;
4545
/// order to call it. Since the precompiled standard library is built with full debuginfo and these
4646
/// variables cannot be optimized out in MIR, an innocent-looking `let` can produce enough
4747
/// debuginfo to have a measurable compile-time impact on debug builds.
48-
#[allow_internal_unstable(ub_checks)] // permit this to be called in stably-const fn
48+
#[allow_internal_unstable(const_ub_checks)] // permit this to be called in stably-const fn
4949
macro_rules! assert_unsafe_precondition {
5050
($kind:ident, $message:expr, ($($name:ident:$ty:ty = $arg:expr),*$(,)?) => $e:expr $(,)?) => {
5151
{
@@ -60,7 +60,7 @@ macro_rules! assert_unsafe_precondition {
6060
#[rustc_no_mir_inline]
6161
#[inline]
6262
#[rustc_nounwind]
63-
#[rustc_const_unstable(feature = "ub_checks", issue = "none")]
63+
#[rustc_const_unstable(feature = "const_ub_checks", issue = "none")]
6464
const fn precondition_check($($name:$ty),*) {
6565
if !$e {
6666
::core::panicking::panic_nounwind(
@@ -69,14 +69,41 @@ macro_rules! assert_unsafe_precondition {
6969
}
7070
}
7171

72-
if ::core::intrinsics::$kind() {
72+
if ::core::ub_checks::$kind() {
7373
precondition_check($($arg,)*);
7474
}
7575
}
7676
};
7777
}
7878
pub(crate) use assert_unsafe_precondition;
7979

80+
/// Checking library UB is always enabled when UB-checking is done
81+
/// (and we use a reexport so that there is no unnecessary wrapper function).
82+
pub(crate) use intrinsics::ub_checks as check_library_ub;
83+
84+
/// Determines whether we should check for language UB.
85+
///
86+
/// The intention is to not do that when running in the interpreter, as that one has its own
87+
/// language UB checks which generally produce better errors.
88+
#[rustc_const_unstable(feature = "const_ub_checks", issue = "none")]
89+
#[inline]
90+
pub(crate) const fn check_language_ub() -> bool {
91+
#[inline]
92+
fn runtime() -> bool {
93+
// Disable UB checks in Miri.
94+
!cfg!(miri)
95+
}
96+
97+
#[inline]
98+
const fn comptime() -> bool {
99+
// Always disable UB checks.
100+
false
101+
}
102+
103+
// Only used for UB checks so we may const_eval_select.
104+
intrinsics::ub_checks() && const_eval_select((), comptime, runtime)
105+
}
106+
80107
/// Checks whether `ptr` is properly aligned with respect to
81108
/// `align_of::<T>()`.
82109
///

0 commit comments

Comments
 (0)