Skip to content

Commit

Permalink
Auto merge of rust-lang#104862 - saethlin:mir-niche-checks, r=<try>
Browse files Browse the repository at this point in the history
Check for occupied niches

Implementation of rust-lang/compiler-team#624

Crater run has 62 crates that hit the check, 43 of those are published to crates.io. I see a lot of null function pointers and use of `mem::uninitialized` where the 0x1-filling collides with an enum niche.

But that is with full niche checks; checking transmute, plus any place where that we Copy, Move, or Inspect. Such checking is definitely too thorough to be on by default because it is 2x compile time overhead.

---

During implementation, this ran into llvm/llvm-project#68381

r? `@ghost`
  • Loading branch information
bors committed Oct 28, 2023
2 parents 6f349cd + 5ac1992 commit 3ca7ba3
Show file tree
Hide file tree
Showing 37 changed files with 631 additions and 35 deletions.
23 changes: 23 additions & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,29 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
source_info.span,
);
}
AssertKind::OccupiedNiche {
ref found,
ref start,
ref end,
ref type_name,
ref offset,
ref niche_ty,
} => {
let found = codegen_operand(fx, found).load_scalar(fx);
let start = codegen_operand(fx, start).load_scalar(fx);
let end = codegen_operand(fx, end).load_scalar(fx);
let type_name = fx.anonymous_str(type_name);
let offset = codegen_operand(fx, offset).load_scalar(fx);
let niche_ty = fx.anonymous_str(niche_ty);
let location = fx.get_caller_location(source_info).load_scalar(fx);

codegen_panic_inner(
fx,
rustc_hir::LangItem::PanicOccupiedNiche,
&[found, start, end, type_name, offset, niche_ty, location],
source_info.span,
)
}
_ => {
let msg_str = msg.description();
codegen_panic(fx, msg_str, source_info);
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// and `#[track_caller]` adds an implicit third argument.
(LangItem::PanicMisalignedPointerDereference, vec![required, found, location])
}
AssertKind::OccupiedNiche { ref found, ref start, ref end } => {
let found = self.codegen_operand(bx, found).immediate();
let start = self.codegen_operand(bx, start).immediate();
let end = self.codegen_operand(bx, end).immediate();
(LangItem::PanicOccupiedNiche, vec![found, start, end, location])
}
_ => {
let msg = bx.const_str(msg.description());
// It's `pub fn panic(expr: &str)`, with the wide reference being passed
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 @@ -542,6 +542,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
found: eval_to_int(found)?,
}
}
OccupiedNiche { ref found, ref start, ref end } => OccupiedNiche {
found: eval_to_int(found)?,
start: eval_to_int(start)?,
end: eval_to_int(end)?,
},
};
Err(ConstEvalErrKind::AssertFailure(err).into())
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_middle::mir::interpret::{InterpResult, PointerArithmetic, Scalar};
use rustc_middle::mir::CastKind;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::layout::{IntegerExt, LayoutOf, TyAndLayout};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, FloatTy, Ty, TypeAndMut};
use rustc_target::abi::Integer;
use rustc_type_ir::TyKind::*;
Expand Down Expand Up @@ -147,8 +148,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if src.layout.size != dest.layout.size {
let src_bytes = src.layout.size.bytes();
let dest_bytes = dest.layout.size.bytes();
let src_ty = format!("{}", src.layout.ty);
let dest_ty = format!("{}", dest.layout.ty);
let src_ty = with_no_trimmed_paths!(format!("{}", src.layout.ty));
let dest_ty = with_no_trimmed_paths!(format!("{}", dest.layout.ty));
throw_ub_custom!(
fluent::const_eval_invalid_transmute,
src_bytes = src_bytes,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ language_item_table! {
ConstPanicFmt, sym::const_panic_fmt, const_panic_fmt, Target::Fn, GenericRequirement::None;
PanicBoundsCheck, sym::panic_bounds_check, panic_bounds_check_fn, Target::Fn, GenericRequirement::Exact(0);
PanicMisalignedPointerDereference, sym::panic_misaligned_pointer_dereference, panic_misaligned_pointer_dereference_fn, Target::Fn, GenericRequirement::Exact(0);
PanicOccupiedNiche, sym::panic_occupied_niche, panic_occupied_niche_fn, Target::Fn, GenericRequirement::Exact(0);
PanicInfo, sym::panic_info, panic_info, Target::Struct, GenericRequirement::None;
PanicLocation, sym::panic_location, panic_location, Target::Struct, GenericRequirement::None;
PanicImpl, sym::panic_impl, panic_impl, Target::Fn, GenericRequirement::None;
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ middle_assert_divide_by_zero =
middle_assert_misaligned_ptr_deref =
misaligned pointer dereference: address must be a multiple of {$required} but is {$found}
middle_assert_occupied_niche =
occupied niche: {$found} must be in {$start}..={$end}
middle_assert_op_overflow =
attempt to compute `{$left} {$op} {$right}`, which would overflow
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,7 @@ pub enum AssertKind<O> {
ResumedAfterReturn(CoroutineKind),
ResumedAfterPanic(CoroutineKind),
MisalignedPointerDereference { required: O, found: O },
OccupiedNiche { found: O, start: O, end: O },
}

#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]
Expand Down
16 changes: 14 additions & 2 deletions compiler/rustc_middle/src/mir/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl<O> AssertKind<O> {
ResumedAfterReturn(CoroutineKind::Async(_)) => "`async fn` resumed after completion",
ResumedAfterPanic(CoroutineKind::Coroutine) => "coroutine resumed after panicking",
ResumedAfterPanic(CoroutineKind::Async(_)) => "`async fn` resumed after panicking",
BoundsCheck { .. } | MisalignedPointerDereference { .. } => {
BoundsCheck { .. } | MisalignedPointerDereference { .. } | OccupiedNiche { .. } => {
bug!("Unexpected AssertKind")
}
}
Expand Down Expand Up @@ -213,6 +213,13 @@ impl<O> AssertKind<O> {
"\"misaligned pointer dereference: address must be a multiple of {{}} but is {{}}\", {required:?}, {found:?}"
)
}
OccupiedNiche { found, start, end } => {
write!(
f,
"\"occupied niche: {{}} must be in {{}}..={{}}\" {:?} {:?} {:?}",
found, start, end
)
}
_ => write!(f, "\"{}\"", self.description()),
}
}
Expand Down Expand Up @@ -243,8 +250,8 @@ impl<O> AssertKind<O> {
ResumedAfterPanic(CoroutineKind::Coroutine) => {
middle_assert_coroutine_resume_after_panic
}

MisalignedPointerDereference { .. } => middle_assert_misaligned_ptr_deref,
OccupiedNiche { .. } => middle_assert_occupied_niche,
}
}

Expand Down Expand Up @@ -281,6 +288,11 @@ impl<O> AssertKind<O> {
add!("required", format!("{required:#?}"));
add!("found", format!("{found:#?}"));
}
OccupiedNiche { found, start, end } => {
add!("found", format!("{found:?}"));
add!("start", format!("{start:?}"));
add!("end", format!("{end:?}"));
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,11 @@ macro_rules! make_mir_visitor {
self.visit_operand(required, location);
self.visit_operand(found, location);
}
OccupiedNiche { found, start, end } => {
self.visit_operand(found, location);
self.visit_operand(start, location);
self.visit_operand(end, location);
}
}
}

Expand Down
Loading

0 comments on commit 3ca7ba3

Please sign in to comment.