Skip to content

Commit

Permalink
Distinguish between library and lang UB in assert_unsafe_precondition
Browse files Browse the repository at this point in the history
  • Loading branch information
saethlin committed Feb 28, 2024
1 parent bf9c7a6 commit f8d0380
Show file tree
Hide file tree
Showing 46 changed files with 364 additions and 238 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
ConstraintCategory::SizedBound,
);
}
&Rvalue::NullaryOp(NullOp::DebugAssertions, _) => {}
&Rvalue::NullaryOp(NullOp::UbCheck(_), _) => {}

Rvalue::ShallowInitBox(operand, ty) => {
self.check_operand(operand, location);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ fn codegen_stmt<'tcx>(
NullOp::OffsetOf(fields) => {
layout.offset_of_subfield(fx, fields.iter()).bytes()
}
NullOp::DebugAssertions => {
NullOp::UbCheck(_) => {
let val = fx.tcx.sess.opts.debug_assertions;
let val = CValue::by_val(
fx.bcx.ins().iconst(types::I8, i64::try_from(val).unwrap()),
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let val = layout.offset_of_subfield(bx.cx(), fields.iter()).bytes();
bx.cx().const_usize(val)
}
mir::NullOp::DebugAssertions => {
mir::NullOp::UbCheck(_) => {
// In codegen, we want to check for language UB and library UB
let val = bx.tcx().sess.opts.debug_assertions;
bx.cx().const_bool(val)
}
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let val = layout.offset_of_subfield(self, fields.iter()).bytes();
Scalar::from_target_usize(val, self)
}
mir::NullOp::DebugAssertions => {
// The checks hidden behind this are always better done by the interpreter
// itself, because it knows the runtime state better.
Scalar::from_bool(false)
mir::NullOp::UbCheck(kind) => {
// We want to enable checks for library UB, because the interpreter doesn't
// know about those on its own.
// But we want to disable checks for language UB, because the interpreter
// has its own better checks for that.
let should_check = match kind {
mir::UbKind::LibraryUb => true,
mir::UbKind::LanguageUb => false,
};
Scalar::from_bool(should_check)
}
};
self.write_scalar(val, &dest)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
Rvalue::Cast(_, _, _) => {}

Rvalue::NullaryOp(
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::DebugAssertions,
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) | NullOp::UbCheck(_),
_,
) => {}
Rvalue::ShallowInitBox(_, _) => {}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
Rvalue::Repeat(_, _)
| Rvalue::ThreadLocalRef(_)
| Rvalue::AddressOf(_, _)
| Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::DebugAssertions, _)
| Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::UbCheck(_), _)
| Rvalue::Discriminant(_) => {}
}
self.super_rvalue(rvalue, location);
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -
| sym::variant_count
| sym::is_val_statically_known
| sym::ptr_mask
| sym::debug_assertions
| sym::check_language_ub
| sym::check_library_ub
| sym::fadd_algebraic
| sym::fsub_algebraic
| sym::fmul_algebraic
Expand Down Expand Up @@ -508,7 +509,7 @@ pub fn check_intrinsic_type(
(0, 0, vec![Ty::new_imm_ptr(tcx, Ty::new_unit(tcx))], tcx.types.usize)
}

sym::debug_assertions => (0, 1, Vec::new(), tcx.types.bool),
sym::check_language_ub | sym::check_library_ub => (0, 1, Vec::new(), tcx.types.bool),

sym::simd_eq
| sym::simd_ne
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
NullOp::SizeOf => write!(fmt, "SizeOf({t})"),
NullOp::AlignOf => write!(fmt, "AlignOf({t})"),
NullOp::OffsetOf(fields) => write!(fmt, "OffsetOf({t}, {fields:?})"),
NullOp::DebugAssertions => write!(fmt, "cfg!(debug_assertions)"),
NullOp::UbCheck(kind) => write!(fmt, "UbCheck({kind:?})"),
}
}
ThreadLocalRef(did) => ty::tls::with(|tcx| {
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,8 +1361,13 @@ pub enum NullOp<'tcx> {
AlignOf,
/// Returns the offset of a field
OffsetOf(&'tcx List<(VariantIdx, FieldIdx)>),
/// cfg!(debug_assertions), but expanded in codegen
DebugAssertions,
UbCheck(UbKind),
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)]
pub enum UbKind {
LanguageUb,
LibraryUb,
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ impl<'tcx> Rvalue<'tcx> {
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {
tcx.types.usize
}
Rvalue::NullaryOp(NullOp::DebugAssertions, _) => tcx.types.bool,
Rvalue::NullaryOp(NullOp::UbCheck(_), _) => tcx.types.bool,
Rvalue::Aggregate(ref ak, ref ops) => match **ak {
AggregateKind::Array(ty) => Ty::new_array(tcx, ty, ops.len() as u64),
AggregateKind::Tuple => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_dataflow/src/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ impl<'b, 'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> Gatherer<'b, 'a, 'tcx, F> {
| Rvalue::Discriminant(..)
| Rvalue::Len(..)
| Rvalue::NullaryOp(
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..) | NullOp::DebugAssertions,
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..) | NullOp::UbCheck(_),
_,
) => {}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
NullOp::OffsetOf(fields) => {
layout.offset_of_subfield(&self.ecx, fields.iter()).bytes()
}
NullOp::DebugAssertions => return None,
NullOp::UbCheck(_) => return None,
};
let usize_layout = self.ecx.layout_of(self.tcx.types.usize).unwrap();
let imm = ImmTy::try_from_uint(val, usize_layout)?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/known_panics_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
NullOp::OffsetOf(fields) => {
op_layout.offset_of_subfield(self, fields.iter()).bytes()
}
NullOp::DebugAssertions => return None,
NullOp::UbCheck(_) => return None,
};
ImmTy::from_scalar(Scalar::from_target_usize(val, self), layout).into()
}
Expand Down
21 changes: 19 additions & 2 deletions compiler/rustc_mir_transform/src/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,30 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
sym::unreachable => {
terminator.kind = TerminatorKind::Unreachable;
}
sym::debug_assertions => {
sym::check_language_ub => {
let target = target.unwrap();
block.statements.push(Statement {
source_info: terminator.source_info,
kind: StatementKind::Assign(Box::new((
*destination,
Rvalue::NullaryOp(NullOp::DebugAssertions, tcx.types.bool),
Rvalue::NullaryOp(
NullOp::UbCheck(UbKind::LanguageUb),
tcx.types.bool,
),
))),
});
terminator.kind = TerminatorKind::Goto { target };
}
sym::check_library_ub => {
let target = target.unwrap();
block.statements.push(Statement {
source_info: terminator.source_info,
kind: StatementKind::Assign(Box::new((
*destination,
Rvalue::NullaryOp(
NullOp::UbCheck(UbKind::LibraryUb),
tcx.types.bool,
),
))),
});
terminator.kind = TerminatorKind::Goto { target };
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ impl<'tcx> Validator<'_, 'tcx> {
NullOp::SizeOf => {}
NullOp::AlignOf => {}
NullOp::OffsetOf(_) => {}
NullOp::DebugAssertions => {}
NullOp::UbCheck(_) => {}
},

Rvalue::ShallowInitBox(_, _) => return Err(Unpromotable),
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_smir/src/rustc_smir/convert/mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,19 @@ impl<'tcx> Stable<'tcx> for mir::NullOp<'tcx> {
type T = stable_mir::mir::NullOp;
fn stable(&self, tables: &mut Tables<'_>) -> Self::T {
use rustc_middle::mir::NullOp::*;
use rustc_middle::mir::UbKind;
match self {
SizeOf => stable_mir::mir::NullOp::SizeOf,
AlignOf => stable_mir::mir::NullOp::AlignOf,
OffsetOf(indices) => stable_mir::mir::NullOp::OffsetOf(
indices.iter().map(|idx| idx.stable(tables)).collect(),
),
DebugAssertions => stable_mir::mir::NullOp::DebugAssertions,
UbCheck(UbKind::LanguageUb) => {
stable_mir::mir::NullOp::UbCheck(stable_mir::mir::UbKind::LanguageUb)
}
UbCheck(UbKind::LibraryUb) => {
stable_mir::mir::NullOp::UbCheck(stable_mir::mir::UbKind::LibraryUb)
}
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ symbols! {
cfi,
cfi_encoding,
char,
check_language_ub,
check_library_ub,
client,
clippy,
clobber_abi,
Expand Down
10 changes: 8 additions & 2 deletions compiler/stable_mir/src/mir/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ impl Rvalue {
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {
Ok(Ty::usize_ty())
}
Rvalue::NullaryOp(NullOp::DebugAssertions, _) => Ok(Ty::bool_ty()),
Rvalue::NullaryOp(NullOp::UbCheck(_), _) => Ok(Ty::bool_ty()),
Rvalue::Aggregate(ak, ops) => match *ak {
AggregateKind::Array(ty) => Ty::try_new_array(ty, ops.len() as u64),
AggregateKind::Tuple => Ok(Ty::new_tuple(
Expand Down Expand Up @@ -1007,7 +1007,13 @@ pub enum NullOp {
/// Returns the offset of a field.
OffsetOf(Vec<(VariantIdx, FieldIdx)>),
/// cfg!(debug_assertions), but at codegen time
DebugAssertions,
UbCheck(UbKind),
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub enum UbKind {
LanguageUb,
LibraryUb,
}

impl Operand {
Expand Down
1 change: 1 addition & 0 deletions library/core/src/char/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub(super) const unsafe fn from_u32_unchecked(i: u32) -> char {
// SAFETY: the caller must guarantee that `i` is a valid char value.
unsafe {
assert_unsafe_precondition!(
check_language_ub,
"invalid value for `char`",
(i: u32 = i) => char_try_from_u32(i).is_ok()
);
Expand Down
7 changes: 6 additions & 1 deletion library/core/src/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ pub const unsafe fn unreachable_unchecked() -> ! {
// SAFETY: the safety contract for `intrinsics::unreachable` must
// be upheld by the caller.
unsafe {
intrinsics::assert_unsafe_precondition!("hint::unreachable_unchecked must never be reached", () => false);
intrinsics::assert_unsafe_precondition!(
check_language_ub,
"hint::unreachable_unchecked must never be reached",
() => false
);
intrinsics::unreachable()
}
}
Expand Down Expand Up @@ -147,6 +151,7 @@ pub const unsafe fn assert_unchecked(cond: bool) {
// SAFETY: The caller promised `cond` is true.
unsafe {
intrinsics::assert_unsafe_precondition!(
check_language_ub,
"hint::assert_unchecked must never be called when the condition is false",
(cond: bool = cond) => cond,
);
Expand Down
Loading

0 comments on commit f8d0380

Please sign in to comment.