From fa98120bb50efba062847938ddfe3574e97fa882 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Mon, 30 Oct 2023 22:29:32 -0400 Subject: [PATCH] Move alignment checks to codegen --- .../example/mini_core.rs | 14 + compiler/rustc_codegen_cranelift/src/base.rs | 77 +++++- .../rustc_codegen_gcc/example/mini_core.rs | 14 + compiler/rustc_codegen_ssa/src/mir/block.rs | 9 +- compiler/rustc_codegen_ssa/src/mir/mod.rs | 2 + .../src/mir/pointer_alignment_check.rs | 162 ++++++++++++ .../rustc_codegen_ssa/src/mir/statement.rs | 31 +++ .../src/const_eval/machine.rs | 6 - compiler/rustc_middle/messages.ftl | 3 - compiler/rustc_middle/src/mir/syntax.rs | 1 - compiler/rustc_middle/src/mir/terminator.rs | 14 +- compiler/rustc_middle/src/mir/visit.rs | 4 - compiler/rustc_middle/src/ty/context.rs | 18 ++ .../src/check_alignment.rs | 245 ------------------ compiler/rustc_mir_transform/src/lib.rs | 3 - compiler/rustc_monomorphize/src/collector.rs | 3 - .../rustc_smir/src/rustc_smir/convert/mir.rs | 6 - compiler/stable_mir/src/mir/body.rs | 12 +- compiler/stable_mir/src/mir/pretty.rs | 1 + compiler/stable_mir/src/mir/visit.rs | 1 + library/core/src/panicking.rs | 2 +- src/tools/miri/src/lib.rs | 1 - src/tools/miri/src/shims/panic.rs | 21 -- tests/debuginfo/simple-struct.rs | 2 +- 24 files changed, 322 insertions(+), 330 deletions(-) create mode 100644 compiler/rustc_codegen_ssa/src/mir/pointer_alignment_check.rs delete mode 100644 compiler/rustc_mir_transform/src/check_alignment.rs diff --git a/compiler/rustc_codegen_cranelift/example/mini_core.rs b/compiler/rustc_codegen_cranelift/example/mini_core.rs index e45c16ee280a7..320b72bb5d02f 100644 --- a/compiler/rustc_codegen_cranelift/example/mini_core.rs +++ b/compiler/rustc_codegen_cranelift/example/mini_core.rs @@ -517,6 +517,20 @@ fn panic_cannot_unwind() -> ! { } } +#[lang = "panic_misaligned_pointer_dereference"] +#[track_caller] +fn panic_misaligned_pointer_dereference(required: usize, found: usize) -> ! { + unsafe { + libc::printf( + "misaligned pointer dereference: address must be a multiple of %d but is %d\n\0" + as *const str as *const i8, + required, + found, + ); + intrinsics::abort(); + } +} + #[lang = "eh_personality"] fn eh_personality() -> ! { loop {} diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 1307a62a60d09..99a9d86b81f7b 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -4,6 +4,7 @@ use cranelift_codegen::ir::UserFuncName; use cranelift_codegen::CodegenError; use cranelift_module::ModuleError; use rustc_ast::InlineAsmOptions; +use rustc_codegen_ssa::mir::pointers_to_check; use rustc_index::IndexVec; use rustc_middle::ty::adjustment::PointerCoercion; use rustc_middle::ty::layout::FnAbiOf; @@ -359,18 +360,6 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { Some(source_info.span), ); } - AssertKind::MisalignedPointerDereference { ref required, ref found } => { - let required = codegen_operand(fx, required).load_scalar(fx); - let found = codegen_operand(fx, found).load_scalar(fx); - let location = fx.get_caller_location(source_info).load_scalar(fx); - - codegen_panic_inner( - fx, - rustc_hir::LangItem::PanicMisalignedPointerDereference, - &[required, found, location], - Some(source_info.span), - ); - } _ => { let location = fx.get_caller_location(source_info).load_scalar(fx); @@ -513,6 +502,49 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { } } +fn codegen_alignment_check<'tcx>( + fx: &mut FunctionCx<'_, '_, 'tcx>, + pointer: mir::Operand<'tcx>, + required_alignment: u64, + source_info: mir::SourceInfo, +) { + // Compute the alignment mask + let required_alignment = required_alignment as i64; + let mask = fx.bcx.ins().iconst(fx.pointer_type, required_alignment - 1); + let required = fx.bcx.ins().iconst(fx.pointer_type, required_alignment); + + // And the pointer with the mask + let pointer = codegen_operand(fx, &pointer); + let pointer = match pointer.layout().abi { + Abi::Scalar(_) => pointer.load_scalar(fx), + Abi::ScalarPair(..) => pointer.load_scalar_pair(fx).0, + _ => unreachable!(), + }; + let masked = fx.bcx.ins().band(pointer, mask); + + // Branch on whether the masked value is zero + let is_zero = fx.bcx.ins().icmp_imm(IntCC::Equal, masked, 0); + + // Create destination blocks, branching on is_zero + let panic = fx.bcx.create_block(); + let success = fx.bcx.create_block(); + fx.bcx.ins().brif(is_zero, success, &[], panic, &[]); + + // Switch to the failure block and codegen a call to the panic intrinsic + fx.bcx.switch_to_block(panic); + let location = fx.get_caller_location(source_info).load_scalar(fx); + codegen_panic_inner( + fx, + rustc_hir::LangItem::PanicMisalignedPointerDereference, + &[required, pointer, location], + Some(source_info.span), + ); + + // Continue codegen in the success block + fx.bcx.switch_to_block(success); + fx.bcx.ins().nop(); +} + fn codegen_stmt<'tcx>( fx: &mut FunctionCx<'_, '_, 'tcx>, #[allow(unused_variables)] cur_block: Block, @@ -534,6 +566,27 @@ fn codegen_stmt<'tcx>( } } + let required_align_of = |pointer| { + let pointer_ty = fx.mir.local_decls[pointer].ty; + let pointer_ty = fx.monomorphize(pointer_ty); + if !pointer_ty.is_unsafe_ptr() { + return None; + } + + let pointee_ty = + pointer_ty.builtin_deref(true).expect("no builtin_deref for an unsafe pointer").ty; + let pointee_layout = fx.layout_of(pointee_ty); + + Some(pointee_layout.align.abi.bytes() as u64) + }; + + if fx.tcx.may_insert_alignment_checks() { + for (pointer, required_alignment) in pointers_to_check(stmt, required_align_of) { + let pointer = mir::Operand::Copy(pointer.into()); + codegen_alignment_check(fx, pointer, required_alignment, stmt.source_info); + } + } + match &stmt.kind { StatementKind::SetDiscriminant { place, variant_index } => { let place = codegen_place(fx, **place); diff --git a/compiler/rustc_codegen_gcc/example/mini_core.rs b/compiler/rustc_codegen_gcc/example/mini_core.rs index 4665009e191e6..9f5d5fd1919f2 100644 --- a/compiler/rustc_codegen_gcc/example/mini_core.rs +++ b/compiler/rustc_codegen_gcc/example/mini_core.rs @@ -474,6 +474,20 @@ fn panic_bounds_check(index: usize, len: usize) -> ! { } } +#[lang = "panic_misaligned_pointer_dereference"] +#[track_caller] +fn panic_misaligned_pointer_dereference(required: usize, found: usize) -> ! { + unsafe { + libc::printf( + "misaligned pointer dereference: address must be a multiple of %d but is %d\n\0" + as *const str as *const i8, + required, + found, + ); + intrinsics::abort(); + } +} + #[lang = "eh_personality"] fn eh_personality() -> ! { loop {} diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 1aa52a985ef7d..7344c59a12a62 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -674,13 +674,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // and `#[track_caller]` adds an implicit third argument. (LangItem::PanicBoundsCheck, vec![index, len, location]) } - AssertKind::MisalignedPointerDereference { ref required, ref found } => { - let required = self.codegen_operand(bx, required).immediate(); - let found = self.codegen_operand(bx, found).immediate(); - // It's `fn panic_misaligned_pointer_dereference(required: usize, found: usize)`, - // and `#[track_caller]` adds an implicit third argument. - (LangItem::PanicMisalignedPointerDereference, vec![required, found, location]) - } _ => { // It's `pub fn panic_...()` and `#[track_caller]` adds an implicit argument. (msg.panic_function(), vec![location]) @@ -1583,7 +1576,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { tuple.layout.fields.count() } - fn get_caller_location( + pub fn get_caller_location( &mut self, bx: &mut Bx, source_info: mir::SourceInfo, diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 387a5366b209b..068a2bc4873c6 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -20,12 +20,14 @@ mod intrinsic; mod locals; pub mod operand; pub mod place; +mod pointer_alignment_check; mod rvalue; mod statement; use self::debuginfo::{FunctionDebugContext, PerLocalVarDebugInfo}; use self::operand::{OperandRef, OperandValue}; use self::place::PlaceRef; +pub use self::pointer_alignment_check::pointers_to_check; // Used for tracking the state of generated basic blocks. enum CachedLlbb { diff --git a/compiler/rustc_codegen_ssa/src/mir/pointer_alignment_check.rs b/compiler/rustc_codegen_ssa/src/mir/pointer_alignment_check.rs new file mode 100644 index 0000000000000..364d2c7220b7b --- /dev/null +++ b/compiler/rustc_codegen_ssa/src/mir/pointer_alignment_check.rs @@ -0,0 +1,162 @@ +use rustc_hir::LangItem; +use rustc_middle::mir; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext}; +use rustc_span::Span; + +use super::FunctionCx; +use crate::base; +use crate::common; +use crate::mir::OperandValue; +use crate::traits::*; + +pub fn pointers_to_check( + statement: &mir::Statement<'_>, + required_align_of: F, +) -> Vec<(mir::Local, u64)> +where + F: Fn(mir::Local) -> Option, +{ + let mut finder = PointerFinder { required_align_of, pointers: Vec::new() }; + finder.visit_statement(statement, rustc_middle::mir::Location::START); + finder.pointers +} + +struct PointerFinder { + pointers: Vec<(mir::Local, u64)>, + required_align_of: F, +} + +impl<'tcx, F> Visitor<'tcx> for PointerFinder +where + F: Fn(mir::Local) -> Option, +{ + fn visit_place( + &mut self, + place: &mir::Place<'tcx>, + context: PlaceContext, + location: mir::Location, + ) { + // We want to only check reads and writes to Places, so we specifically exclude + // Borrows and AddressOf. + match context { + PlaceContext::MutatingUse( + MutatingUseContext::Store + | MutatingUseContext::AsmOutput + | MutatingUseContext::Call + | MutatingUseContext::Yield + | MutatingUseContext::Drop, + ) => {} + PlaceContext::NonMutatingUse( + NonMutatingUseContext::Copy | NonMutatingUseContext::Move, + ) => {} + _ => { + return; + } + } + + if !place.is_indirect() { + return; + } + + let pointer = place.local; + let Some(required_alignment) = (self.required_align_of)(pointer) else { + return; + }; + + if required_alignment == 1 { + return; + } + + // Ensure that this place is based on an aligned pointer. + self.pointers.push((pointer, required_alignment)); + + self.super_place(place, context, location); + } +} + +impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { + #[instrument(level = "debug", skip(self, bx))] + pub fn codegen_alignment_check( + &mut self, + bx: &mut Bx, + pointer: mir::Operand<'tcx>, + required_alignment: u64, + source_info: mir::SourceInfo, + ) { + // Compute the alignment mask + let mask = bx.const_usize(required_alignment - 1); + let zero = bx.const_usize(0); + let required_alignment = bx.const_usize(required_alignment); + + // And the pointer with the mask + let pointer = match self.codegen_operand(bx, &pointer).val { + OperandValue::Immediate(imm) => imm, + OperandValue::Pair(ptr, _) => ptr, + _ => { + unreachable!("{pointer:?}"); + } + }; + let addr = bx.ptrtoint(pointer, bx.cx().type_isize()); + let masked = bx.and(addr, mask); + + // Branch on whether the masked value is zero + let is_zero = bx.icmp( + base::bin_op_to_icmp_predicate(mir::BinOp::Eq.to_hir_binop(), false), + masked, + zero, + ); + + // Create destination blocks, branching on is_zero + let panic = bx.append_sibling_block("panic"); + let success = bx.append_sibling_block("success"); + bx.cond_br(is_zero, success, panic); + + // Switch to the failure block and codegen a call to the panic intrinsic + bx.switch_to_block(panic); + self.set_debug_loc(bx, source_info); + let location = self.get_caller_location(bx, source_info).immediate(); + self.codegen_nounwind_panic( + bx, + LangItem::PanicMisalignedPointerDereference, + &[required_alignment, addr, location], + source_info.span, + ); + + // Continue codegen in the success block. + bx.switch_to_block(success); + self.set_debug_loc(bx, source_info); + } + + /// Emit a call to a diverging and `rustc_nounwind` panic helper. + #[instrument(level = "debug", skip(self, bx))] + fn codegen_nounwind_panic( + &mut self, + bx: &mut Bx, + lang_item: LangItem, + args: &[Bx::Value], + span: Span, + ) { + let (fn_abi, fn_ptr, instance) = common::build_langcall(bx, Some(span), lang_item); + let fn_ty = bx.fn_decl_backend_type(&fn_abi); + let fn_attrs = if bx.tcx().def_kind(self.instance.def_id()).has_codegen_attrs() { + Some(bx.tcx().codegen_fn_attrs(self.instance.def_id())) + } else { + None + }; + + // bx.call requires that the call not unwind. Double-check that this LangItem can't unwind. + assert!(!fn_abi.can_unwind); + + bx.call( + fn_ty, + fn_attrs, + Some(&fn_abi), + fn_ptr, + args, + None, /* funclet */ + Some(instance), + ); + bx.unreachable(); + } +} diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 2188eeae42686..7a9d78783c64f 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -2,6 +2,7 @@ use rustc_middle::mir; use rustc_middle::mir::NonDivergingIntrinsic; use rustc_session::config::OptLevel; +use super::pointers_to_check; use super::FunctionCx; use super::LocalRef; use crate::traits::*; @@ -10,6 +11,36 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { #[instrument(level = "debug", skip(self, bx))] pub fn codegen_statement(&mut self, bx: &mut Bx, statement: &mir::Statement<'tcx>) { self.set_debug_loc(bx, statement.source_info); + + let required_align_of = |local| { + // Since Deref projections must come first and only once, the pointer for an indirect place + // is the Local that the Place is based on. + let pointer_ty = self.mir.local_decls[local].ty; + let pointer_ty = self.monomorphize(pointer_ty); + + // We only want to check places based on unsafe pointers + if !pointer_ty.is_unsafe_ptr() { + return None; + } + + let pointee_ty = + pointer_ty.builtin_deref(true).expect("no builtin_deref for an unsafe pointer").ty; + let pointee_layout = bx.layout_of(pointee_ty); + + Some(pointee_layout.layout.align.abi.bytes()) + }; + + if bx.tcx().may_insert_alignment_checks() { + for (pointer, required_alignment) in pointers_to_check(statement, required_align_of) { + let pointer = mir::Operand::Copy(pointer.into()); + self.codegen_alignment_check( + bx, + pointer, + required_alignment, + statement.source_info, + ); + } + } match statement.kind { mir::StatementKind::Assign(box (ref place, ref rvalue)) => { if let Some(index) = place.as_local() { diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index dd835279df331..17dac60817071 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -567,12 +567,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, RemainderByZero(op) => RemainderByZero(eval_to_int(op)?), ResumedAfterReturn(coroutine_kind) => ResumedAfterReturn(*coroutine_kind), ResumedAfterPanic(coroutine_kind) => ResumedAfterPanic(*coroutine_kind), - MisalignedPointerDereference { ref required, ref found } => { - MisalignedPointerDereference { - required: eval_to_int(required)?, - found: eval_to_int(found)?, - } - } }; Err(ConstEvalErrKind::AssertFailure(err).into()) } diff --git a/compiler/rustc_middle/messages.ftl b/compiler/rustc_middle/messages.ftl index 27d555d7e26c7..9444f53ce706b 100644 --- a/compiler/rustc_middle/messages.ftl +++ b/compiler/rustc_middle/messages.ftl @@ -14,9 +14,6 @@ middle_assert_divide_by_zero = middle_assert_gen_resume_after_panic = `gen` fn or block cannot be further iterated on after it panicked -middle_assert_misaligned_ptr_deref = - misaligned pointer dereference: address must be a multiple of {$required} but is {$found} - middle_assert_op_overflow = attempt to compute `{$left} {$op} {$right}`, which would overflow diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index c78c225b0cdf8..18749f058a175 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -884,7 +884,6 @@ pub enum AssertKind { RemainderByZero(O), ResumedAfterReturn(CoroutineKind), ResumedAfterPanic(CoroutineKind), - MisalignedPointerDereference { required: O, found: O }, } #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)] diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs index f116347cc2bad..e984a092dcb19 100644 --- a/compiler/rustc_middle/src/mir/terminator.rs +++ b/compiler/rustc_middle/src/mir/terminator.rs @@ -190,7 +190,7 @@ impl AssertKind { LangItem::PanicGenFnNonePanic } - BoundsCheck { .. } | MisalignedPointerDereference { .. } => { + BoundsCheck { .. } => { bug!("Unexpected AssertKind") } } @@ -248,12 +248,6 @@ impl AssertKind { write!(f, "\"attempt to shift left by `{{}}`, which would overflow\", {r:?}") } Overflow(op, _, _) => bug!("{:?} cannot overflow", op), - MisalignedPointerDereference { required, found } => { - write!( - f, - "\"misaligned pointer dereference: address must be a multiple of {{}} but is {{}}\", {required:?}, {found:?}" - ) - } ResumedAfterReturn(CoroutineKind::Coroutine(_)) => { write!(f, "\"coroutine resumed after completion\"") } @@ -323,8 +317,6 @@ impl AssertKind { ResumedAfterPanic(CoroutineKind::Coroutine(_)) => { middle_assert_coroutine_resume_after_panic } - - MisalignedPointerDereference { .. } => middle_assert_misaligned_ptr_deref, } } @@ -357,10 +349,6 @@ impl AssertKind { add!("right", format!("{right:#?}")); } ResumedAfterReturn(_) | ResumedAfterPanic(_) => {} - MisalignedPointerDereference { required, found } => { - add!("required", format!("{required:#?}")); - add!("found", format!("{found:#?}")); - } } } } diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 4f7b2f7cbe48b..6d7a7f0718a58 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -625,10 +625,6 @@ macro_rules! make_mir_visitor { ResumedAfterReturn(_) | ResumedAfterPanic(_) => { // Nothing to visit } - MisalignedPointerDereference { required, found } => { - self.visit_operand(required, location); - self.visit_operand(found, location); - } } } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 75deffe695764..bc8ad903cd62e 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -2593,6 +2593,24 @@ impl<'tcx> TyCtxt<'tcx> { pub fn impl_polarity(self, def_id: impl IntoQueryParam) -> ty::ImplPolarity { self.impl_trait_header(def_id).map_or(ty::ImplPolarity::Positive, |h| h.polarity) } + + /// Whether a codegen backend may emit alignment checks for pointers when they are + /// read or written through. If this returns true, the backend might emit such checks. If this + /// returns false, the backend must not emit such checks. + pub fn may_insert_alignment_checks(self) -> bool { + // Alignment checks are panics. If for whatever reason we do not have a panic + // implementation, those new panics cause otherwise-valid code to not compile. + if self.lang_items().get(LangItem::PanicImpl).is_none() { + return false; + } + + // FIXME(#112480) MSVC and rustc disagree on minimum stack alignment on x86 Windows. + if self.sess.target.llvm_target == "i686-pc-windows-msvc" { + return false; + } + + self.sess.ub_checks() + } } /// Parameter attributes that can only be determined by examining the body of a function instead diff --git a/compiler/rustc_mir_transform/src/check_alignment.rs b/compiler/rustc_mir_transform/src/check_alignment.rs deleted file mode 100644 index 0af8872988754..0000000000000 --- a/compiler/rustc_mir_transform/src/check_alignment.rs +++ /dev/null @@ -1,245 +0,0 @@ -use rustc_hir::lang_items::LangItem; -use rustc_index::IndexVec; -use rustc_middle::mir::*; -use rustc_middle::mir::{ - interpret::Scalar, - visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}, -}; -use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt}; -use rustc_session::Session; - -pub struct CheckAlignment; - -impl<'tcx> MirPass<'tcx> for CheckAlignment { - fn is_enabled(&self, sess: &Session) -> bool { - // FIXME(#112480) MSVC and rustc disagree on minimum stack alignment on x86 Windows - if sess.target.llvm_target == "i686-pc-windows-msvc" { - return false; - } - sess.ub_checks() - } - - fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - // This pass emits new panics. If for whatever reason we do not have a panic - // implementation, running this pass may cause otherwise-valid code to not compile. - if tcx.lang_items().get(LangItem::PanicImpl).is_none() { - return; - } - - let basic_blocks = body.basic_blocks.as_mut(); - let local_decls = &mut body.local_decls; - let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); - - // This pass inserts new blocks. Each insertion changes the Location for all - // statements/blocks after. Iterating or visiting the MIR in order would require updating - // our current location after every insertion. By iterating backwards, we dodge this issue: - // The only Locations that an insertion changes have already been handled. - for block in (0..basic_blocks.len()).rev() { - let block = block.into(); - for statement_index in (0..basic_blocks[block].statements.len()).rev() { - let location = Location { block, statement_index }; - let statement = &basic_blocks[block].statements[statement_index]; - let source_info = statement.source_info; - - let mut finder = - PointerFinder { tcx, local_decls, param_env, pointers: Vec::new() }; - finder.visit_statement(statement, location); - - for (local, ty) in finder.pointers { - debug!("Inserting alignment check for {:?}", ty); - let new_block = split_block(basic_blocks, location); - insert_alignment_check( - tcx, - local_decls, - &mut basic_blocks[block], - local, - ty, - source_info, - new_block, - ); - } - } - } - } -} - -struct PointerFinder<'tcx, 'a> { - tcx: TyCtxt<'tcx>, - local_decls: &'a mut LocalDecls<'tcx>, - param_env: ParamEnv<'tcx>, - pointers: Vec<(Place<'tcx>, Ty<'tcx>)>, -} - -impl<'tcx, 'a> Visitor<'tcx> for PointerFinder<'tcx, 'a> { - fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) { - // We want to only check reads and writes to Places, so we specifically exclude - // Borrows and AddressOf. - match context { - PlaceContext::MutatingUse( - MutatingUseContext::Store - | MutatingUseContext::AsmOutput - | MutatingUseContext::Call - | MutatingUseContext::Yield - | MutatingUseContext::Drop, - ) => {} - PlaceContext::NonMutatingUse( - NonMutatingUseContext::Copy | NonMutatingUseContext::Move, - ) => {} - _ => { - return; - } - } - - if !place.is_indirect() { - return; - } - - // Since Deref projections must come first and only once, the pointer for an indirect place - // is the Local that the Place is based on. - let pointer = Place::from(place.local); - let pointer_ty = self.local_decls[place.local].ty; - - // We only want to check places based on unsafe pointers - if !pointer_ty.is_unsafe_ptr() { - trace!("Indirect, but not based on an unsafe ptr, not checking {:?}", place); - return; - } - - let pointee_ty = - pointer_ty.builtin_deref(true).expect("no builtin_deref for an unsafe pointer").ty; - // Ideally we'd support this in the future, but for now we are limited to sized types. - if !pointee_ty.is_sized(self.tcx, self.param_env) { - debug!("Unsafe pointer, but pointee is not known to be sized: {:?}", pointer_ty); - return; - } - - // Try to detect types we are sure have an alignment of 1 and skip the check - // We don't need to look for str and slices, we already rejected unsized types above - let element_ty = match pointee_ty.kind() { - ty::Array(ty, _) => *ty, - _ => pointee_ty, - }; - if [self.tcx.types.bool, self.tcx.types.i8, self.tcx.types.u8].contains(&element_ty) { - debug!("Trivially aligned place type: {:?}", pointee_ty); - return; - } - - // Ensure that this place is based on an aligned pointer. - self.pointers.push((pointer, pointee_ty)); - - self.super_place(place, context, location); - } -} - -fn split_block( - basic_blocks: &mut IndexVec>, - location: Location, -) -> BasicBlock { - let block_data = &mut basic_blocks[location.block]; - - // Drain every statement after this one and move the current terminator to a new basic block - let new_block = BasicBlockData { - statements: block_data.statements.split_off(location.statement_index), - terminator: block_data.terminator.take(), - is_cleanup: block_data.is_cleanup, - }; - - basic_blocks.push(new_block) -} - -fn insert_alignment_check<'tcx>( - tcx: TyCtxt<'tcx>, - local_decls: &mut IndexVec>, - block_data: &mut BasicBlockData<'tcx>, - pointer: Place<'tcx>, - pointee_ty: Ty<'tcx>, - source_info: SourceInfo, - new_block: BasicBlock, -) { - // Cast the pointer to a *const () - let const_raw_ptr = Ty::new_imm_ptr(tcx, tcx.types.unit); - let rvalue = Rvalue::Cast(CastKind::PtrToPtr, Operand::Copy(pointer), const_raw_ptr); - let thin_ptr = local_decls.push(LocalDecl::with_source_info(const_raw_ptr, source_info)).into(); - block_data - .statements - .push(Statement { source_info, kind: StatementKind::Assign(Box::new((thin_ptr, rvalue))) }); - - // Transmute the pointer to a usize (equivalent to `ptr.addr()`) - let rvalue = Rvalue::Cast(CastKind::Transmute, Operand::Copy(thin_ptr), tcx.types.usize); - let addr = local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into(); - block_data - .statements - .push(Statement { source_info, kind: StatementKind::Assign(Box::new((addr, rvalue))) }); - - // Get the alignment of the pointee - let alignment = - local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into(); - let rvalue = Rvalue::NullaryOp(NullOp::AlignOf, pointee_ty); - block_data.statements.push(Statement { - source_info, - kind: StatementKind::Assign(Box::new((alignment, rvalue))), - }); - - // Subtract 1 from the alignment to get the alignment mask - let alignment_mask = - local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into(); - let one = Operand::Constant(Box::new(ConstOperand { - span: source_info.span, - user_ty: None, - const_: Const::Val(ConstValue::Scalar(Scalar::from_target_usize(1, &tcx)), tcx.types.usize), - })); - block_data.statements.push(Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - alignment_mask, - Rvalue::BinaryOp(BinOp::Sub, Box::new((Operand::Copy(alignment), one))), - ))), - }); - - // BitAnd the alignment mask with the pointer - let alignment_bits = - local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into(); - block_data.statements.push(Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - alignment_bits, - Rvalue::BinaryOp( - BinOp::BitAnd, - Box::new((Operand::Copy(addr), Operand::Copy(alignment_mask))), - ), - ))), - }); - - // Check if the alignment bits are all zero - let is_ok = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into(); - let zero = Operand::Constant(Box::new(ConstOperand { - span: source_info.span, - user_ty: None, - const_: Const::Val(ConstValue::Scalar(Scalar::from_target_usize(0, &tcx)), tcx.types.usize), - })); - block_data.statements.push(Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - is_ok, - Rvalue::BinaryOp(BinOp::Eq, Box::new((Operand::Copy(alignment_bits), zero.clone()))), - ))), - }); - - // Set this block's terminator to our assert, continuing to new_block if we pass - block_data.terminator = Some(Terminator { - source_info, - kind: TerminatorKind::Assert { - cond: Operand::Copy(is_ok), - expected: true, - target: new_block, - msg: Box::new(AssertKind::MisalignedPointerDereference { - required: Operand::Copy(alignment), - found: Operand::Copy(addr), - }), - // This calls panic_misaligned_pointer_dereference, which is #[rustc_nounwind]. - // We never want to insert an unwind into unsafe code, because unwinding could - // make a failing UB check turn into much worse UB when we start unwinding. - unwind: UnwindAction::Unreachable, - }, - }); -} diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index e477c068229ff..ca9e075cedbae 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -104,7 +104,6 @@ mod reveal_all; mod shim; mod ssa; // This pass is public to allow external drivers to perform MIR cleanup -mod check_alignment; pub mod simplify; mod simplify_branches; mod simplify_comparison_integral; @@ -558,8 +557,6 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { // Before doing anything, remember which items are being mentioned so that the set of items // visited does not depend on the optimization level. &mentioned_items::MentionedItems, - // Add some UB checks before any UB gets optimized away. - &check_alignment::CheckAlignment, // Before inlining: trim down MIR with passes to reduce inlining work. // Has to be done before inlining, otherwise actual call will be almost always inlined. diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 0c35f9838ed3f..6db6d55106c49 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -975,9 +975,6 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { mir::AssertKind::BoundsCheck { .. } => { push_mono_lang_item(self, LangItem::PanicBoundsCheck); } - mir::AssertKind::MisalignedPointerDereference { .. } => { - push_mono_lang_item(self, LangItem::PanicMisalignedPointerDereference); - } _ => { push_mono_lang_item(self, msg.panic_function()); } diff --git a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs index c9f6661259022..8c3b3b4f6842a 100644 --- a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs +++ b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs @@ -457,12 +457,6 @@ impl<'tcx> Stable<'tcx> for mir::AssertMessage<'tcx> { AssertKind::ResumedAfterPanic(coroutine) => { stable_mir::mir::AssertMessage::ResumedAfterPanic(coroutine.stable(tables)) } - AssertKind::MisalignedPointerDereference { required, found } => { - stable_mir::mir::AssertMessage::MisalignedPointerDereference { - required: required.stable(tables), - found: found.stable(tables), - } - } } } } diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 593b1868f26fa..5b92832390af1 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -239,14 +239,21 @@ pub enum UnwindAction { #[derive(Clone, Debug, Eq, PartialEq)] pub enum AssertMessage { - BoundsCheck { len: Operand, index: Operand }, + BoundsCheck { + len: Operand, + index: Operand, + }, Overflow(BinOp, Operand, Operand), OverflowNeg(Operand), DivisionByZero(Operand), RemainderByZero(Operand), ResumedAfterReturn(CoroutineKind), ResumedAfterPanic(CoroutineKind), - MisalignedPointerDereference { required: Operand, found: Operand }, + #[deprecated(note = "Alignment checks are now implemented in codegen, not MIR")] + MisalignedPointerDereference { + required: Operand, + found: Operand, + }, } impl AssertMessage { @@ -299,6 +306,7 @@ impl AssertMessage { )) => Ok("`gen fn` should just keep returning `AssertMessage::None` after panicking"), AssertMessage::BoundsCheck { .. } => Ok("index out of bounds"), + #[allow(deprecated)] AssertMessage::MisalignedPointerDereference { .. } => { Ok("misaligned pointer dereference") } diff --git a/compiler/stable_mir/src/mir/pretty.rs b/compiler/stable_mir/src/mir/pretty.rs index 4ac4833add715..b1975e9d52448 100644 --- a/compiler/stable_mir/src/mir/pretty.rs +++ b/compiler/stable_mir/src/mir/pretty.rs @@ -288,6 +288,7 @@ fn pretty_assert_message(writer: &mut W, msg: &AssertMessage) -> io::R "\"attempt to calculate the remainder of `{{}}` with a divisor of zero\", {pretty_op}" ) } + #[allow(deprecated)] AssertMessage::MisalignedPointerDereference { required, found } => { let pretty_required = pretty_operand(required); let pretty_found = pretty_operand(found); diff --git a/compiler/stable_mir/src/mir/visit.rs b/compiler/stable_mir/src/mir/visit.rs index 24296e9e8778b..420eedf64f5c8 100644 --- a/compiler/stable_mir/src/mir/visit.rs +++ b/compiler/stable_mir/src/mir/visit.rs @@ -429,6 +429,7 @@ pub trait MirVisitor { } AssertMessage::ResumedAfterReturn(_) | AssertMessage::ResumedAfterPanic(_) => { //nothing to visit } + #[allow(deprecated)] AssertMessage::MisalignedPointerDereference { required, found } => { self.visit_operand(required, location); self.visit_operand(found, location); diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index cbb0a7d61db05..a517e1f19aad6 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -272,7 +272,7 @@ fn panic_bounds_check(index: usize, len: usize) -> ! { #[cfg_attr(feature = "panic_immediate_abort", inline)] #[track_caller] #[lang = "panic_misaligned_pointer_dereference"] // needed by codegen for panic on misaligned pointer deref -#[rustc_nounwind] // `CheckAlignment` MIR pass requires this function to never unwind +#[rustc_nounwind] // codegen requires this function to never unwind fn panic_misaligned_pointer_dereference(required: usize, found: usize) -> ! { if cfg!(feature = "panic_immediate_abort") { super::intrinsics::abort() diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 7821aa9efd4c1..f2392ca6fd4a6 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -144,7 +144,6 @@ pub const MIRI_DEFAULT_ARGS: &[&str] = &[ "-Zmir-emit-retag", "-Zmir-keep-place-mention", "-Zmir-opt-level=0", - "-Zmir-enable-passes=-CheckAlignment", // Deduplicating diagnostics means we miss events when tracking what happens during an // execution. Let's not do that. "-Zdeduplicate-diagnostics=no", diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index bb31ef733cf94..70a0a844591e5 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -233,27 +233,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { StackPopCleanup::Goto { ret: None, unwind }, )?; } - MisalignedPointerDereference { required, found } => { - // Forward to `panic_misaligned_pointer_dereference` lang item. - - // First arg: required. - let required = this.read_scalar(&this.eval_operand(required, None)?)?; - // Second arg: found. - let found = this.read_scalar(&this.eval_operand(found, None)?)?; - - // Call the lang item. - let panic_misaligned_pointer_dereference = - this.tcx.lang_items().panic_misaligned_pointer_dereference_fn().unwrap(); - let panic_misaligned_pointer_dereference = - ty::Instance::mono(this.tcx.tcx, panic_misaligned_pointer_dereference); - this.call_function( - panic_misaligned_pointer_dereference, - Abi::Rust, - &[required.into(), found.into()], - None, - StackPopCleanup::Goto { ret: None, unwind }, - )?; - } _ => { // Call the lang item associated with this message. diff --git a/tests/debuginfo/simple-struct.rs b/tests/debuginfo/simple-struct.rs index 968a5c63e89a3..2ca8e5e973fbf 100644 --- a/tests/debuginfo/simple-struct.rs +++ b/tests/debuginfo/simple-struct.rs @@ -1,7 +1,7 @@ //@ min-lldb-version: 310 //@ ignore-gdb // Test temporarily ignored due to debuginfo tests being disabled, see PR 47155 -//@ compile-flags: -g -Zmir-enable-passes=-CheckAlignment +//@ compile-flags: -g // === GDB TESTS ===================================================================================