Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nightmarish hacks within] Lower UB checks as a new kind of intrinsic, not calls #121767

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,10 @@ impl<'cx, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx, R> for MirBorro
NonDivergingIntrinsic::CopyNonOverlapping(..) => span_bug!(
span,
"Unexpected CopyNonOverlapping, should only appear after lower_intrinsics",
),
NonDivergingIntrinsic::UbCheck { .. } => span_bug!(
span,
"Unexpected UbCheck, should only appear after lower_intrinsics",
)
}
// Only relevant for mir typeck
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_borrowck/src/polonius/loan_invalidations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ impl<'cx, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'cx, 'tcx> {
self.consume_operand(location, dst);
self.consume_operand(location, count);
}
StatementKind::Intrinsic(box NonDivergingIntrinsic::UbCheck {
kind: _,
func,
args,
destination,
source_info: _,
fn_span: _,
}) => {
self.consume_operand(location, func);
self.consume_operand(location, args);
self.mutate_place(location, *destination, Shallow(None));
}
// Only relevant for mir typeck
StatementKind::AscribeUserType(..)
// Only relevant for liveness and unsafeck
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
stmt.source_info.span,
"Unexpected NonDivergingIntrinsic::CopyNonOverlapping, should only appear after lowering_intrinsics",
),
NonDivergingIntrinsic::UbCheck { .. } => span_bug!(
stmt.source_info.span,
"Unexpected NonDivergingIntrinsic::UbCheck, should only appear after lowering_intrinsics",
),
},
StatementKind::FakeRead(..)
| StatementKind::StorageLive(..)
Expand Down Expand Up @@ -2001,7 +2005,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
ConstraintCategory::SizedBound,
);
}
&Rvalue::NullaryOp(NullOp::DebugAssertions, _) => {}

Rvalue::ShallowInitBox(operand, ty) => {
self.check_operand(operand, location);
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,15 +767,6 @@ fn codegen_stmt<'tcx>(
NullOp::OffsetOf(fields) => {
layout.offset_of_subfield(fx, fields.iter()).bytes()
}
NullOp::DebugAssertions => {
let val = fx.tcx.sess.opts.debug_assertions;
let val = CValue::by_val(
fx.bcx.ins().iconst(types::I8, i64::try_from(val).unwrap()),
fx.layout_of(fx.tcx.types.bool),
);
lval.write_cvalue(fx, val);
return;
}
};
let val = CValue::by_val(
fx.bcx.ins().iconst(fx.pointer_type, i64::try_from(val).unwrap()),
Expand Down Expand Up @@ -845,6 +836,9 @@ fn codegen_stmt<'tcx>(
};
fx.bcx.call_memcpy(fx.target_config, dst, src, bytes);
}
NonDivergingIntrinsic::UbCheck { .. } => {
todo!() // FIXME
}
},
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
StatementKind::Intrinsic(ref intrinsic) => match **intrinsic {
NonDivergingIntrinsic::CopyNonOverlapping(..) => return None,
NonDivergingIntrinsic::Assume(..) => {}
NonDivergingIntrinsic::UbCheck { .. } => {}
},
// conservative handling
StatementKind::Assign(_)
Expand Down
42 changes: 26 additions & 16 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@ use std::cmp;
// can happen when BB jumps directly to its successor and the successor has no
// other predecessors.
#[derive(Debug, PartialEq)]
enum MergingSucc {
pub(crate) enum MergingSucc {
False,
True,
}

/// Used by `FunctionCx::codegen_terminator` for emitting common patterns
/// e.g., creating a basic block, calling a function, etc.
struct TerminatorCodegenHelper<'tcx> {
bb: mir::BasicBlock,
terminator: &'tcx mir::Terminator<'tcx>,
pub(crate) struct TerminatorCodegenHelper<'term, 'tcx> {
pub(crate) bb: mir::BasicBlock,
pub(crate) terminator: &'term mir::Terminator<'tcx>,
}

impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
impl<'term, 'a, 'tcx> TerminatorCodegenHelper<'term, 'tcx> {
/// Returns the appropriate `Funclet` for the current funclet, if on MSVC,
/// either already previously cached, or newly created, by `landing_pad_for`.
fn funclet<'b, Bx: BuilderMethods<'a, 'tcx>>(
Expand Down Expand Up @@ -98,6 +98,10 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
fx: &mut FunctionCx<'a, 'tcx, Bx>,
target: mir::BasicBlock,
) -> (bool, bool) {
if self.bb > fx.mir.basic_blocks.len().into() {
return (false, false);
}

if let Some(ref cleanup_kinds) = fx.cleanup_kinds {
let funclet_bb = cleanup_kinds[self.bb].funclet_bb(self.bb);
let target_funclet = cleanup_kinds[target].funclet_bb(target);
Expand Down Expand Up @@ -226,8 +230,10 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
MergingSucc::False
} else {
let llret = bx.call(fn_ty, fn_attrs, Some(fn_abi), fn_ptr, llargs, self.funclet(fx));
if fx.mir[self.bb].is_cleanup {
bx.apply_attrs_to_cleanup_callsite(llret);
if let Some(bb) = fx.mir.basic_blocks.get(self.bb) {
if bb.is_cleanup {
bx.apply_attrs_to_cleanup_callsite(llret);
}
}

if let Some((ret_dest, target)) = destination {
Expand Down Expand Up @@ -296,7 +302,11 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
/// Codegen implementations for some terminator variants.
impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
/// Generates code for a `Resume` terminator.
fn codegen_resume_terminator(&mut self, helper: TerminatorCodegenHelper<'tcx>, bx: &mut Bx) {
fn codegen_resume_terminator(
&mut self,
helper: TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
) {
if let Some(funclet) = helper.funclet(self) {
bx.cleanup_ret(funclet, None);
} else {
Expand All @@ -313,7 +323,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

fn codegen_switchint_terminator(
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
helper: TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
discr: &mir::Operand<'tcx>,
targets: &SwitchTargets,
Expand Down Expand Up @@ -451,7 +461,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
#[tracing::instrument(level = "trace", skip(self, helper, bx))]
fn codegen_drop_terminator(
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
helper: TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
location: mir::Place<'tcx>,
target: mir::BasicBlock,
Expand Down Expand Up @@ -569,7 +579,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

fn codegen_assert_terminator(
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
helper: TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
terminator: &mir::Terminator<'tcx>,
cond: &mir::Operand<'tcx>,
Expand Down Expand Up @@ -649,7 +659,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

fn codegen_terminate_terminator(
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
helper: TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
terminator: &mir::Terminator<'tcx>,
reason: UnwindTerminateReason,
Expand Down Expand Up @@ -678,7 +688,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
/// Returns `Some` if this is indeed a panic intrinsic and codegen is done.
fn codegen_panic_intrinsic(
&mut self,
helper: &TerminatorCodegenHelper<'tcx>,
helper: &TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
intrinsic: Option<Symbol>,
instance: Option<Instance<'tcx>>,
Expand Down Expand Up @@ -744,9 +754,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
}

fn codegen_call_terminator(
pub(crate) fn codegen_call_terminator(
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
helper: TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
terminator: &mir::Terminator<'tcx>,
func: &mir::Operand<'tcx>,
Expand Down Expand Up @@ -1068,7 +1078,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

fn codegen_asm_terminator(
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
helper: TerminatorCodegenHelper<'_, 'tcx>,
bx: &mut Bx,
terminator: &mir::Terminator<'tcx>,
template: &[ast::InlineAsmTemplatePiece],
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -684,10 +684,6 @@ 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 => {
let val = bx.tcx().sess.opts.debug_assertions;
bx.cx().const_bool(val)
}
};
let tcx = self.cx.tcx();
OperandRef {
Expand Down
40 changes: 40 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/statement.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use rustc_middle::mir;
use rustc_middle::mir::NonDivergingIntrinsic;
use rustc_session::config::OptLevel;
use rustc_span::source_map::respan;

use super::FunctionCx;
use super::LocalRef;
use crate::mir::block::TerminatorCodegenHelper;
use crate::traits::*;

impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Expand Down Expand Up @@ -90,6 +92,44 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let src = src_val.immediate();
bx.memcpy(dst, align, src, align, bytes, crate::MemFlags::empty());
}
mir::StatementKind::Intrinsic(box NonDivergingIntrinsic::UbCheck {
kind: _,
ref func,
ref args,
destination,
fn_span,
source_info,
}) => {
if bx.tcx().sess.opts.debug_assertions {
let bb = mir::BasicBlock::MAX;
let args = vec![respan(source_info.span, args.clone())];
let terminator = mir::Terminator {
source_info,
kind: mir::TerminatorKind::Call {
func: func.clone(),
args: args.clone(),
destination,
target: Some(bb),
unwind: mir::UnwindAction::Unreachable,
call_source: mir::CallSource::Normal,
fn_span,
},
};
let helper = TerminatorCodegenHelper { bb, terminator: &terminator };
self.codegen_call_terminator(
helper,
bx,
&terminator,
func,
args.as_slice(),
destination,
Some(bb),
mir::UnwindAction::Unreachable,
fn_span,
true, // mergeable_succ
);
}
}
mir::StatementKind::FakeRead(..)
| mir::StatementKind::Retag { .. }
| mir::StatementKind::AscribeUserType(..)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
dest,
ret,
mir::UnwindAction::Unreachable,
false,
)?;
Ok(ControlFlow::Break(()))
} else {
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ pub enum StackPopCleanup {
/// wants them leaked to intern what they need (and just throw away
/// the entire `ecx` when it is done).
Root { cleanup: bool },

/// FIXME: We're returning from a UbCheck call, don't do anything.
Nothing,
}

/// State of a local variable including a memoized layout
Expand Down Expand Up @@ -928,6 +931,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let cleanup = match return_to_block {
StackPopCleanup::Goto { .. } => true,
StackPopCleanup::Root { cleanup, .. } => cleanup,
StackPopCleanup::Nothing => true,
};
if cleanup {
// We need to take the locals out, since we need to mutate while iterating.
Expand Down Expand Up @@ -964,6 +968,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
StackPopCleanup::Root { .. } => {
panic!("encountered StackPopCleanup::Root when unwinding!")
}
StackPopCleanup::Nothing => {
panic!("encountered StackPopCleanup::Nothing when unwinding!")
}
};
// This must be the very last thing that happens, since it can in fact push a new stack frame.
self.unwind_to_block(unwind)
Expand All @@ -978,6 +985,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
);
Ok(())
}
StackPopCleanup::Nothing => {
// Advance the program counter.
self.frame_mut().loc.as_mut().left().unwrap().statement_index += 1;
Ok(())
}
}
}
}
Expand Down
Loading
Loading