diff options
Diffstat (limited to 'compiler/rustc_mir_transform/src')
31 files changed, 825 insertions, 685 deletions
diff --git a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs index 87ae2b71654..8716fd1c098 100644 --- a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs +++ b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs @@ -99,10 +99,13 @@ fn add_move_for_packed_drop<'tcx>( patch.add_statement(loc, StatementKind::StorageLive(temp)); patch.add_assign(loc, Place::from(temp), Rvalue::Use(Operand::Move(*place))); - patch.patch_terminator(loc.block, TerminatorKind::Drop { - place: Place::from(temp), - target: storage_dead_block, - unwind, - replace, - }); + patch.patch_terminator( + loc.block, + TerminatorKind::Drop { + place: Place::from(temp), + target: storage_dead_block, + unwind, + replace, + }, + ); } diff --git a/compiler/rustc_mir_transform/src/add_retag.rs b/compiler/rustc_mir_transform/src/add_retag.rs index 1fc788a2dad..e5a28d1b66c 100644 --- a/compiler/rustc_mir_transform/src/add_retag.rs +++ b/compiler/rustc_mir_transform/src/add_retag.rs @@ -111,10 +111,13 @@ impl<'tcx> crate::MirPass<'tcx> for AddRetag { .collect::<Vec<_>>(); // Now we go over the returns we collected to retag the return values. for (source_info, dest_place, dest_block) in returns { - basic_blocks[dest_block].statements.insert(0, Statement { - source_info, - kind: StatementKind::Retag(RetagKind::Default, Box::new(dest_place)), - }); + basic_blocks[dest_block].statements.insert( + 0, + Statement { + source_info, + kind: StatementKind::Retag(RetagKind::Default, Box::new(dest_place)), + }, + ); } // PART 3 @@ -169,10 +172,13 @@ impl<'tcx> crate::MirPass<'tcx> for AddRetag { }; // Insert a retag after the statement. let source_info = block_data.statements[i].source_info; - block_data.statements.insert(i + 1, Statement { - source_info, - kind: StatementKind::Retag(retag_kind, Box::new(place)), - }); + block_data.statements.insert( + i + 1, + Statement { + source_info, + kind: StatementKind::Retag(retag_kind, Box::new(place)), + }, + ); } } } diff --git a/compiler/rustc_mir_transform/src/check_alignment.rs b/compiler/rustc_mir_transform/src/check_alignment.rs index d7e22c12394..b70cca14840 100644 --- a/compiler/rustc_mir_transform/src/check_alignment.rs +++ b/compiler/rustc_mir_transform/src/check_alignment.rs @@ -1,11 +1,11 @@ -use rustc_hir::lang_items::LangItem; use rustc_index::IndexVec; use rustc_middle::mir::interpret::Scalar; -use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; +use rustc_middle::mir::visit::PlaceContext; use rustc_middle::mir::*; -use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::ty::{Ty, TyCtxt}; use rustc_session::Session; -use tracing::{debug, trace}; + +use crate::check_pointers::{BorrowCheckMode, PointerCheck, check_pointers}; pub(super) struct CheckAlignment; @@ -19,46 +19,19 @@ impl<'tcx> crate::MirPass<'tcx> for CheckAlignment { } 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 typing_env = body.typing_env(tcx); - let basic_blocks = body.basic_blocks.as_mut(); - let local_decls = &mut body.local_decls; - - // 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, typing_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, - ); - } - } - } + // Skip trivially aligned place types. + let excluded_pointees = [tcx.types.bool, tcx.types.i8, tcx.types.u8]; + + // We have to exclude borrows here: in `&x.field`, the exact + // requirement is that the final reference must be aligned, but + // `check_pointers` would check that `x` is aligned, which would be wrong. + check_pointers( + tcx, + body, + &excluded_pointees, + insert_alignment_check, + BorrowCheckMode::ExcludeBorrows, + ); } fn is_required(&self) -> bool { @@ -66,119 +39,34 @@ impl<'tcx> crate::MirPass<'tcx> for CheckAlignment { } } -struct PointerFinder<'a, 'tcx> { - tcx: TyCtxt<'tcx>, - local_decls: &'a mut LocalDecls<'tcx>, - typing_env: ty::TypingEnv<'tcx>, - pointers: Vec<(Place<'tcx>, Ty<'tcx>)>, -} - -impl<'a, 'tcx> Visitor<'tcx> for PointerFinder<'a, 'tcx> { - 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 - // Borrow and RawBorrow. - 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"); - // 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.typing_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<BasicBlock, BasicBlockData<'_>>, - 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) -} - +/// Inserts the actual alignment check's logic. Returns a +/// [AssertKind::MisalignedPointerDereference] on failure. fn insert_alignment_check<'tcx>( tcx: TyCtxt<'tcx>, - local_decls: &mut IndexVec<Local, LocalDecl<'tcx>>, - block_data: &mut BasicBlockData<'tcx>, pointer: Place<'tcx>, pointee_ty: Ty<'tcx>, + _context: PlaceContext, + local_decls: &mut IndexVec<Local, LocalDecl<'tcx>>, + stmts: &mut Vec<Statement<'tcx>>, source_info: SourceInfo, - new_block: BasicBlock, -) { - // Cast the pointer to a *const () +) -> PointerCheck<'tcx> { + // 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 + stmts .push(Statement { source_info, kind: StatementKind::Assign(Box::new((thin_ptr, rvalue))) }); - // Transmute the pointer to a usize (equivalent to `ptr.addr()`) + // 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))) }); + stmts.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 { + stmts.push(Statement { source_info, kind: StatementKind::Assign(Box::new((alignment, rvalue))), }); @@ -191,7 +79,7 @@ fn insert_alignment_check<'tcx>( user_ty: None, const_: Const::Val(ConstValue::Scalar(Scalar::from_target_usize(1, &tcx)), tcx.types.usize), })); - block_data.statements.push(Statement { + stmts.push(Statement { source_info, kind: StatementKind::Assign(Box::new(( alignment_mask, @@ -202,7 +90,7 @@ fn insert_alignment_check<'tcx>( // 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 { + stmts.push(Statement { source_info, kind: StatementKind::Assign(Box::new(( alignment_bits, @@ -220,7 +108,7 @@ fn insert_alignment_check<'tcx>( user_ty: None, const_: Const::Val(ConstValue::Scalar(Scalar::from_target_usize(0, &tcx)), tcx.types.usize), })); - block_data.statements.push(Statement { + stmts.push(Statement { source_info, kind: StatementKind::Assign(Box::new(( is_ok, @@ -228,21 +116,13 @@ fn insert_alignment_check<'tcx>( ))), }); - // 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, - }, - }); + // Emit a check that asserts on the alignment and otherwise triggers a + // AssertKind::MisalignedPointerDereference. + PointerCheck { + cond: Operand::Copy(is_ok), + assert_kind: Box::new(AssertKind::MisalignedPointerDereference { + required: Operand::Copy(alignment), + found: Operand::Copy(addr), + }), + } } diff --git a/compiler/rustc_mir_transform/src/check_call_recursion.rs b/compiler/rustc_mir_transform/src/check_call_recursion.rs index 51fd3c6512e..e49723a6c39 100644 --- a/compiler/rustc_mir_transform/src/check_call_recursion.rs +++ b/compiler/rustc_mir_transform/src/check_call_recursion.rs @@ -83,10 +83,12 @@ fn check_recursion<'tcx>( let sp = tcx.def_span(def_id); let hir_id = tcx.local_def_id_to_hir_id(def_id); - tcx.emit_node_span_lint(UNCONDITIONAL_RECURSION, hir_id, sp, UnconditionalRecursion { - span: sp, - call_sites: vis.reachable_recursive_calls, - }); + tcx.emit_node_span_lint( + UNCONDITIONAL_RECURSION, + hir_id, + sp, + UnconditionalRecursion { span: sp, call_sites: vis.reachable_recursive_calls }, + ); } } diff --git a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs index 7968b666dff..3affe4abbfa 100644 --- a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs +++ b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs @@ -133,7 +133,7 @@ impl<'tcx> Visitor<'tcx> for ConstMutationChecker<'_, 'tcx> { // the `self` parameter of a method call (as the terminator of our current // BasicBlock). If so, we emit a more specific lint. let method_did = self.target_local.and_then(|target_local| { - rustc_middle::util::find_self_call(self.tcx, self.body, target_local, loc.block) + find_self_call(self.tcx, self.body, target_local, loc.block) }); let lint_loc = if method_did.is_some() { self.body.terminator_loc(loc.block) } else { loc }; diff --git a/compiler/rustc_mir_transform/src/check_null.rs b/compiler/rustc_mir_transform/src/check_null.rs new file mode 100644 index 00000000000..543e1845e65 --- /dev/null +++ b/compiler/rustc_mir_transform/src/check_null.rs @@ -0,0 +1,133 @@ +use rustc_index::IndexVec; +use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext}; +use rustc_middle::mir::*; +use rustc_middle::ty::{Ty, TyCtxt}; +use rustc_session::Session; + +use crate::check_pointers::{BorrowCheckMode, PointerCheck, check_pointers}; + +pub(super) struct CheckNull; + +impl<'tcx> crate::MirPass<'tcx> for CheckNull { + fn is_enabled(&self, sess: &Session) -> bool { + sess.ub_checks() + } + + fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + check_pointers(tcx, body, &[], insert_null_check, BorrowCheckMode::IncludeBorrows); + } + + fn is_required(&self) -> bool { + true + } +} + +fn insert_null_check<'tcx>( + tcx: TyCtxt<'tcx>, + pointer: Place<'tcx>, + pointee_ty: Ty<'tcx>, + context: PlaceContext, + local_decls: &mut IndexVec<Local, LocalDecl<'tcx>>, + stmts: &mut Vec<Statement<'tcx>>, + source_info: SourceInfo, +) -> PointerCheck<'tcx> { + // 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(); + stmts + .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(); + stmts.push(Statement { source_info, kind: StatementKind::Assign(Box::new((addr, rvalue))) }); + + let zero = Operand::Constant(Box::new(ConstOperand { + span: source_info.span, + user_ty: None, + const_: Const::Val(ConstValue::from_target_usize(0, &tcx), tcx.types.usize), + })); + + let pointee_should_be_checked = match context { + // Borrows pointing to "null" are UB even if the pointee is a ZST. + PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) + | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => { + // Pointer should be checked unconditionally. + Operand::Constant(Box::new(ConstOperand { + span: source_info.span, + user_ty: None, + const_: Const::Val(ConstValue::from_bool(true), tcx.types.bool), + })) + } + // Other usages of null pointers only are UB if the pointee is not a ZST. + _ => { + let rvalue = Rvalue::NullaryOp(NullOp::SizeOf, pointee_ty); + let sizeof_pointee = + local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into(); + stmts.push(Statement { + source_info, + kind: StatementKind::Assign(Box::new((sizeof_pointee, rvalue))), + }); + + // Check that the pointee is not a ZST. + let is_pointee_not_zst = + local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into(); + stmts.push(Statement { + source_info, + kind: StatementKind::Assign(Box::new(( + is_pointee_not_zst, + Rvalue::BinaryOp( + BinOp::Ne, + Box::new((Operand::Copy(sizeof_pointee), zero.clone())), + ), + ))), + }); + + // Pointer needs to be checked only if pointee is not a ZST. + Operand::Copy(is_pointee_not_zst) + } + }; + + // Check whether the pointer is null. + let is_null = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into(); + stmts.push(Statement { + source_info, + kind: StatementKind::Assign(Box::new(( + is_null, + Rvalue::BinaryOp(BinOp::Eq, Box::new((Operand::Copy(addr), zero))), + ))), + }); + + // We want to throw an exception if the pointer is null and the pointee is not unconditionally + // allowed (which for all non-borrow place uses, is when the pointee is ZST). + let should_throw_exception = + local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into(); + stmts.push(Statement { + source_info, + kind: StatementKind::Assign(Box::new(( + should_throw_exception, + Rvalue::BinaryOp( + BinOp::BitAnd, + Box::new((Operand::Copy(is_null), pointee_should_be_checked)), + ), + ))), + }); + + // The final condition whether this pointer usage is ok or not. + let is_ok = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into(); + stmts.push(Statement { + source_info, + kind: StatementKind::Assign(Box::new(( + is_ok, + Rvalue::UnaryOp(UnOp::Not, Operand::Copy(should_throw_exception)), + ))), + }); + + // Emit a PointerCheck that asserts on the condition and otherwise triggers + // a AssertKind::NullPointerDereference. + PointerCheck { + cond: Operand::Copy(is_ok), + assert_kind: Box::new(AssertKind::NullPointerDereference), + } +} diff --git a/compiler/rustc_mir_transform/src/check_pointers.rs b/compiler/rustc_mir_transform/src/check_pointers.rs new file mode 100644 index 00000000000..ccaa83fd9e2 --- /dev/null +++ b/compiler/rustc_mir_transform/src/check_pointers.rs @@ -0,0 +1,236 @@ +use rustc_hir::lang_items::LangItem; +use rustc_index::IndexVec; +use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; +use rustc_middle::mir::*; +use rustc_middle::ty::{self, Ty, TyCtxt}; +use tracing::{debug, trace}; + +/// Details of a pointer check, the condition on which we decide whether to +/// fail the assert and an [AssertKind] that defines the behavior on failure. +pub(crate) struct PointerCheck<'tcx> { + pub(crate) cond: Operand<'tcx>, + pub(crate) assert_kind: Box<AssertKind<Operand<'tcx>>>, +} + +/// Indicates whether we insert the checks for borrow places of a raw pointer. +/// Concretely places with [MutatingUseContext::Borrow] or +/// [NonMutatingUseContext::SharedBorrow]. +#[derive(Copy, Clone)] +pub(crate) enum BorrowCheckMode { + IncludeBorrows, + ExcludeBorrows, +} + +/// Utility for adding a check for read/write on every sized, raw pointer. +/// +/// Visits every read/write access to a [Sized], raw pointer and inserts a +/// new basic block directly before the pointer access. (Read/write accesses +/// are determined by the `PlaceContext` of the MIR visitor.) Then calls +/// `on_finding` to insert the actual logic for a pointer check (e.g. check for +/// alignment). A check can choose to be inserted for (mutable) borrows of +/// raw pointers via the `borrow_check_mode` parameter. +/// +/// This utility takes care of the right order of blocks, the only thing a +/// caller must do in `on_finding` is: +/// - Append [Statement]s to `stmts`. +/// - Append [LocalDecl]s to `local_decls`. +/// - Return a [PointerCheck] that contains the condition and an [AssertKind]. +/// The AssertKind must be a panic with `#[rustc_nounwind]`. The condition +/// should always return the boolean `is_ok`, so evaluate to true in case of +/// success and fail the check otherwise. +/// This utility will insert a terminator block that asserts on the condition +/// and panics on failure. +pub(crate) fn check_pointers<'tcx, F>( + tcx: TyCtxt<'tcx>, + body: &mut Body<'tcx>, + excluded_pointees: &[Ty<'tcx>], + on_finding: F, + borrow_check_mode: BorrowCheckMode, +) where + F: Fn( + /* tcx: */ TyCtxt<'tcx>, + /* pointer: */ Place<'tcx>, + /* pointee_ty: */ Ty<'tcx>, + /* context: */ PlaceContext, + /* local_decls: */ &mut IndexVec<Local, LocalDecl<'tcx>>, + /* stmts: */ &mut Vec<Statement<'tcx>>, + /* source_info: */ SourceInfo, + ) -> PointerCheck<'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 typing_env = body.typing_env(tcx); + let basic_blocks = body.basic_blocks.as_mut(); + let local_decls = &mut body.local_decls; + + // This operation 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::new( + tcx, + local_decls, + typing_env, + excluded_pointees, + borrow_check_mode, + ); + finder.visit_statement(statement, location); + + for (local, ty, context) in finder.into_found_pointers() { + debug!("Inserting check for {:?}", ty); + let new_block = split_block(basic_blocks, location); + + // Invoke `on_finding` which appends to `local_decls` and the + // blocks statements. It returns information about the assert + // we're performing in the Terminator. + let block_data = &mut basic_blocks[block]; + let pointer_check = on_finding( + tcx, + local, + ty, + context, + local_decls, + &mut block_data.statements, + source_info, + ); + block_data.terminator = Some(Terminator { + source_info, + kind: TerminatorKind::Assert { + cond: pointer_check.cond, + expected: true, + target: new_block, + msg: pointer_check.assert_kind, + // This calls a panic function associated with the pointer check, 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, + }, + }); + } + } + } +} + +struct PointerFinder<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + local_decls: &'a mut LocalDecls<'tcx>, + typing_env: ty::TypingEnv<'tcx>, + pointers: Vec<(Place<'tcx>, Ty<'tcx>, PlaceContext)>, + excluded_pointees: &'a [Ty<'tcx>], + borrow_check_mode: BorrowCheckMode, +} + +impl<'a, 'tcx> PointerFinder<'a, 'tcx> { + fn new( + tcx: TyCtxt<'tcx>, + local_decls: &'a mut LocalDecls<'tcx>, + typing_env: ty::TypingEnv<'tcx>, + excluded_pointees: &'a [Ty<'tcx>], + borrow_check_mode: BorrowCheckMode, + ) -> Self { + PointerFinder { + tcx, + local_decls, + typing_env, + excluded_pointees, + pointers: Vec::new(), + borrow_check_mode, + } + } + + fn into_found_pointers(self) -> Vec<(Place<'tcx>, Ty<'tcx>, PlaceContext)> { + self.pointers + } + + /// Whether or not we should visit a [Place] with [PlaceContext]. + /// + /// We generally only visit Reads/Writes to a place and only Borrows if + /// requested. + fn should_visit_place(&self, context: PlaceContext) -> bool { + match context { + PlaceContext::MutatingUse( + MutatingUseContext::Store + | MutatingUseContext::Call + | MutatingUseContext::Yield + | MutatingUseContext::Drop, + ) => true, + PlaceContext::NonMutatingUse( + NonMutatingUseContext::Copy | NonMutatingUseContext::Move, + ) => true, + PlaceContext::MutatingUse(MutatingUseContext::Borrow) + | PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) => { + matches!(self.borrow_check_mode, BorrowCheckMode::IncludeBorrows) + } + _ => false, + } + } +} + +impl<'a, 'tcx> Visitor<'tcx> for PointerFinder<'a, 'tcx> { + fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) { + if !self.should_visit_place(context) || !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 raw pointers + if !pointer_ty.is_unsafe_ptr() { + trace!("Indirect, but not based on an raw ptr, not checking {:?}", place); + return; + } + + let pointee_ty = + pointer_ty.builtin_deref(true).expect("no builtin_deref for an raw pointer"); + // 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.typing_env) { + trace!("Raw pointer, but pointee is not known to be sized: {:?}", pointer_ty); + return; + } + + // We don't need to look for slices, we already rejected unsized types above. + let element_ty = match pointee_ty.kind() { + ty::Array(ty, _) => *ty, + _ => pointee_ty, + }; + if self.excluded_pointees.contains(&element_ty) { + trace!("Skipping pointer for type: {:?}", pointee_ty); + return; + } + + self.pointers.push((pointer, pointee_ty, context)); + + self.super_place(place, context, location); + } +} + +fn split_block( + basic_blocks: &mut IndexVec<BasicBlock, BasicBlockData<'_>>, + 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) +} diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 3b75be58e43..a9bdbeb9cb8 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -1044,11 +1044,14 @@ fn insert_switch<'tcx>( let switch = TerminatorKind::SwitchInt { discr: Operand::Move(discr), targets: switch_targets }; let source_info = SourceInfo::outermost(body.span); - body.basic_blocks_mut().raw.insert(0, BasicBlockData { - statements: vec![assign], - terminator: Some(Terminator { source_info, kind: switch }), - is_cleanup: false, - }); + body.basic_blocks_mut().raw.insert( + 0, + BasicBlockData { + statements: vec![assign], + terminator: Some(Terminator { source_info, kind: switch }), + is_cleanup: false, + }, + ); let blocks = body.basic_blocks_mut().iter_mut(); @@ -1594,13 +1597,16 @@ impl<'tcx> crate::MirPass<'tcx> for StateTransform { // (which is now a generator interior). let source_info = SourceInfo::outermost(body.span); let stmts = &mut body.basic_blocks_mut()[START_BLOCK].statements; - stmts.insert(0, Statement { - source_info, - kind: StatementKind::Assign(Box::new(( - old_resume_local.into(), - Rvalue::Use(Operand::Move(resume_local.into())), - ))), - }); + stmts.insert( + 0, + Statement { + source_info, + kind: StatementKind::Assign(Box::new(( + old_resume_local.into(), + Rvalue::Use(Operand::Move(resume_local.into())), + ))), + }, + ); let always_live_locals = always_storage_live_locals(body); @@ -1839,12 +1845,17 @@ fn check_suspend_tys<'tcx>(tcx: TyCtxt<'tcx>, layout: &CoroutineLayout<'tcx>, bo continue; }; - check_must_not_suspend_ty(tcx, decl.ty, hir_id, SuspendCheckData { - source_span: decl.source_info.span, - yield_span: yield_source_info.span, - plural_len: 1, - ..Default::default() - }); + check_must_not_suspend_ty( + tcx, + decl.ty, + hir_id, + SuspendCheckData { + source_span: decl.source_info.span, + yield_span: yield_source_info.span, + plural_len: 1, + ..Default::default() + }, + ); } } } @@ -1883,13 +1894,17 @@ fn check_must_not_suspend_ty<'tcx>( ty::Adt(_, args) if ty.is_box() => { let boxed_ty = args.type_at(0); let allocator_ty = args.type_at(1); - check_must_not_suspend_ty(tcx, boxed_ty, hir_id, SuspendCheckData { - descr_pre: &format!("{}boxed ", data.descr_pre), - ..data - }) || check_must_not_suspend_ty(tcx, allocator_ty, hir_id, SuspendCheckData { - descr_pre: &format!("{}allocator ", data.descr_pre), - ..data - }) + check_must_not_suspend_ty( + tcx, + boxed_ty, + hir_id, + SuspendCheckData { descr_pre: &format!("{}boxed ", data.descr_pre), ..data }, + ) || check_must_not_suspend_ty( + tcx, + allocator_ty, + hir_id, + SuspendCheckData { descr_pre: &format!("{}allocator ", data.descr_pre), ..data }, + ) } ty::Adt(def, _) => check_must_not_suspend_def(tcx, def.did(), hir_id, data), // FIXME: support adding the attribute to TAITs @@ -1902,10 +1917,12 @@ fn check_must_not_suspend_ty<'tcx>( { let def_id = poly_trait_predicate.trait_ref.def_id; let descr_pre = &format!("{}implementer{} of ", data.descr_pre, plural_suffix); - if check_must_not_suspend_def(tcx, def_id, hir_id, SuspendCheckData { - descr_pre, - ..data - }) { + if check_must_not_suspend_def( + tcx, + def_id, + hir_id, + SuspendCheckData { descr_pre, ..data }, + ) { has_emitted = true; break; } @@ -1919,10 +1936,12 @@ fn check_must_not_suspend_ty<'tcx>( if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder() { let def_id = trait_ref.def_id; let descr_post = &format!(" trait object{}{}", plural_suffix, data.descr_post); - if check_must_not_suspend_def(tcx, def_id, hir_id, SuspendCheckData { - descr_post, - ..data - }) { + if check_must_not_suspend_def( + tcx, + def_id, + hir_id, + SuspendCheckData { descr_post, ..data }, + ) { has_emitted = true; break; } @@ -1934,10 +1953,12 @@ fn check_must_not_suspend_ty<'tcx>( let mut has_emitted = false; for (i, ty) in fields.iter().enumerate() { let descr_post = &format!(" in tuple element {i}"); - if check_must_not_suspend_ty(tcx, ty, hir_id, SuspendCheckData { - descr_post, - ..data - }) { + if check_must_not_suspend_ty( + tcx, + ty, + hir_id, + SuspendCheckData { descr_post, ..data }, + ) { has_emitted = true; } } @@ -1945,12 +1966,17 @@ fn check_must_not_suspend_ty<'tcx>( } ty::Array(ty, len) => { let descr_pre = &format!("{}array{} of ", data.descr_pre, plural_suffix); - check_must_not_suspend_ty(tcx, ty, hir_id, SuspendCheckData { - descr_pre, - // FIXME(must_not_suspend): This is wrong. We should handle printing unevaluated consts. - plural_len: len.try_to_target_usize(tcx).unwrap_or(0) as usize + 1, - ..data - }) + check_must_not_suspend_ty( + tcx, + ty, + hir_id, + SuspendCheckData { + descr_pre, + // FIXME(must_not_suspend): This is wrong. We should handle printing unevaluated consts. + plural_len: len.try_to_target_usize(tcx).unwrap_or(0) as usize + 1, + ..data + }, + ) } // If drop tracking is enabled, we want to look through references, since the referent // may not be considered live across the await point. diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index 9f5bb015fa3..0f5fcb0d8eb 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -138,10 +138,10 @@ pub(crate) fn coroutine_by_move_body_def_id<'tcx>( // If the parent capture is by-ref, then we need to apply an additional // deref before applying any further projections to this place. if parent_capture.is_by_ref() { - child_precise_captures.insert(0, Projection { - ty: parent_capture.place.ty(), - kind: ProjectionKind::Deref, - }); + child_precise_captures.insert( + 0, + Projection { ty: parent_capture.place.ty(), kind: ProjectionKind::Deref }, + ); } // If the child capture is by-ref, then we need to apply a "ref" // projection (i.e. `&`) at the end. But wait! We don't have that diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index adb99a75a9e..6f9984d5d0a 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -2,7 +2,6 @@ use std::cmp::Ordering; use either::Either; use itertools::Itertools; -use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_data_structures::graph::DirectedGraph; use rustc_index::IndexVec; @@ -11,31 +10,35 @@ use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId, use crate::coverage::counters::balanced_flow::BalancedFlowGraph; use crate::coverage::counters::node_flow::{ - CounterTerm, NodeCounters, make_node_counters, node_flow_data_for_balanced_graph, + CounterTerm, NodeCounters, NodeFlowData, node_flow_data_for_balanced_graph, }; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; mod balanced_flow; -mod node_flow; +pub(crate) mod node_flow; mod union_find; -/// Ensures that each BCB node needing a counter has one, by creating physical -/// counters or counter expressions for nodes as required. -pub(super) fn make_bcb_counters( - graph: &CoverageGraph, - bcb_needs_counter: &DenseBitSet<BasicCoverageBlock>, -) -> CoverageCounters { +/// Struct containing the results of [`prepare_bcb_counters_data`]. +pub(crate) struct BcbCountersData { + pub(crate) node_flow_data: NodeFlowData<BasicCoverageBlock>, + pub(crate) priority_list: Vec<BasicCoverageBlock>, +} + +/// Analyzes the coverage graph to create intermediate data structures that +/// will later be used (during codegen) to create physical counters or counter +/// expressions for each BCB node that needs one. +pub(crate) fn prepare_bcb_counters_data(graph: &CoverageGraph) -> BcbCountersData { // Create the derived graphs that are necessary for subsequent steps. let balanced_graph = BalancedFlowGraph::for_graph(graph, |n| !graph[n].is_out_summable); let node_flow_data = node_flow_data_for_balanced_graph(&balanced_graph); - // Use those graphs to determine which nodes get physical counters, and how - // to compute the execution counts of other nodes from those counters. + // Also create a "priority list" of coverage graph nodes, to help determine + // which ones get physical counters or counter expressions. This needs to + // be done now, because the later parts of the counter-creation process + // won't have access to the original coverage graph. let priority_list = make_node_flow_priority_list(graph, balanced_graph); - let node_counters = make_node_counters(&node_flow_data, &priority_list); - // Convert the counters into a form suitable for embedding into MIR. - transcribe_counters(&node_counters, bcb_needs_counter) + BcbCountersData { node_flow_data, priority_list } } /// Arranges the nodes in `balanced_graph` into a list, such that earlier nodes @@ -74,31 +77,33 @@ fn make_node_flow_priority_list( } // Converts node counters into a form suitable for embedding into MIR. -fn transcribe_counters( +pub(crate) fn transcribe_counters( old: &NodeCounters<BasicCoverageBlock>, bcb_needs_counter: &DenseBitSet<BasicCoverageBlock>, + bcbs_seen: &DenseBitSet<BasicCoverageBlock>, ) -> CoverageCounters { let mut new = CoverageCounters::with_num_bcbs(bcb_needs_counter.domain_size()); for bcb in bcb_needs_counter.iter() { + if !bcbs_seen.contains(bcb) { + // This BCB's code was removed by MIR opts, so its counter is always zero. + new.set_node_counter(bcb, CovTerm::Zero); + continue; + } + // Our counter-creation algorithm doesn't guarantee that a node's list // of terms starts or ends with a positive term, so partition the // counters into "positive" and "negative" lists for easier handling. - let (mut pos, mut neg): (Vec<_>, Vec<_>) = - old.counter_terms[bcb].iter().partition_map(|&CounterTerm { node, op }| match op { + let (mut pos, mut neg): (Vec<_>, Vec<_>) = old.counter_terms[bcb] + .iter() + // Filter out any BCBs that were removed by MIR opts; + // this treats them as having an execution count of 0. + .filter(|term| bcbs_seen.contains(term.node)) + .partition_map(|&CounterTerm { node, op }| match op { Op::Add => Either::Left(node), Op::Subtract => Either::Right(node), }); - if pos.is_empty() { - // If we somehow end up with no positive terms, fall back to - // creating a physical counter. There's no known way for this - // to happen, but we can avoid an ICE if it does. - debug_assert!(false, "{bcb:?} has no positive counter terms"); - pos = vec![bcb]; - neg = vec![]; - } - // These intermediate sorts are not strictly necessary, but were helpful // in reducing churn when switching to the current counter-creation scheme. // They also help to slightly decrease the overall size of the expression @@ -116,7 +121,7 @@ fn transcribe_counters( pos.sort(); neg.sort(); - let pos_counter = new.make_sum(&pos).expect("`pos` should not be empty"); + let pos_counter = new.make_sum(&pos).unwrap_or(CovTerm::Zero); let new_counter = new.make_subtracted_sum(pos_counter, &neg); new.set_node_counter(bcb, new_counter); } @@ -129,15 +134,15 @@ fn transcribe_counters( pub(super) struct CoverageCounters { /// List of places where a counter-increment statement should be injected /// into MIR, each with its corresponding counter ID. - phys_counter_for_node: FxIndexMap<BasicCoverageBlock, CounterId>, - next_counter_id: CounterId, + pub(crate) phys_counter_for_node: FxIndexMap<BasicCoverageBlock, CounterId>, + pub(crate) next_counter_id: CounterId, /// Coverage counters/expressions that are associated with individual BCBs. - node_counters: IndexVec<BasicCoverageBlock, Option<CovTerm>>, + pub(crate) node_counters: IndexVec<BasicCoverageBlock, Option<CovTerm>>, /// Table of expression data, associating each expression ID with its /// corresponding operator (+ or -) and its LHS/RHS operands. - expressions: IndexVec<ExpressionId, Expression>, + pub(crate) expressions: IndexVec<ExpressionId, Expression>, /// Remember expressions that have already been created (or simplified), /// so that we don't create unnecessary duplicates. expressions_memo: FxHashMap<Expression, CovTerm>, @@ -188,12 +193,6 @@ impl CoverageCounters { self.make_expression(lhs, Op::Subtract, rhs_sum) } - pub(super) fn num_counters(&self) -> usize { - let num_counters = self.phys_counter_for_node.len(); - assert_eq!(num_counters, self.next_counter_id.as_usize()); - num_counters - } - fn set_node_counter(&mut self, bcb: BasicCoverageBlock, counter: CovTerm) -> CovTerm { let existing = self.node_counters[bcb].replace(counter); assert!( @@ -202,34 +201,4 @@ impl CoverageCounters { ); counter } - - pub(super) fn term_for_bcb(&self, bcb: BasicCoverageBlock) -> Option<CovTerm> { - self.node_counters[bcb] - } - - /// Returns an iterator over all the nodes in the coverage graph that - /// should have a counter-increment statement injected into MIR, along with - /// each site's corresponding counter ID. - pub(super) fn counter_increment_sites( - &self, - ) -> impl Iterator<Item = (CounterId, BasicCoverageBlock)> + Captures<'_> { - self.phys_counter_for_node.iter().map(|(&site, &id)| (id, site)) - } - - /// Returns an iterator over the subset of BCB nodes that have been associated - /// with a counter *expression*, along with the ID of that expression. - pub(super) fn bcb_nodes_with_coverage_expressions( - &self, - ) -> impl Iterator<Item = (BasicCoverageBlock, ExpressionId)> + Captures<'_> { - self.node_counters.iter_enumerated().filter_map(|(bcb, &counter)| match counter { - // Yield the BCB along with its associated expression ID. - Some(CovTerm::Expression(id)) => Some((bcb, id)), - // This BCB is associated with a counter or nothing, so skip it. - Some(CovTerm::Counter { .. } | CovTerm::Zero) | None => None, - }) - } - - pub(super) fn into_expressions(self) -> IndexVec<ExpressionId, Expression> { - self.expressions - } } diff --git a/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs b/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs index 9d80b3af42d..91ed54b8b59 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs @@ -9,6 +9,7 @@ use rustc_data_structures::graph; use rustc_index::bit_set::DenseBitSet; use rustc_index::{Idx, IndexSlice, IndexVec}; +pub(crate) use rustc_middle::mir::coverage::NodeFlowData; use rustc_middle::mir::coverage::Op; use crate::coverage::counters::union_find::UnionFind; @@ -16,30 +17,6 @@ use crate::coverage::counters::union_find::UnionFind; #[cfg(test)] mod tests; -/// Data representing a view of some underlying graph, in which each node's -/// successors have been merged into a single "supernode". -/// -/// The resulting supernodes have no obvious meaning on their own. -/// However, merging successor nodes means that a node's out-edges can all -/// be combined into a single out-edge, whose flow is the same as the flow -/// (execution count) of its corresponding node in the original graph. -/// -/// With all node flows now in the original graph now represented as edge flows -/// in the merged graph, it becomes possible to analyze the original node flows -/// using techniques for analyzing edge flows. -#[derive(Debug)] -pub(crate) struct NodeFlowData<Node: Idx> { - /// Maps each node to the supernode that contains it, indicated by some - /// arbitrary "root" node that is part of that supernode. - supernodes: IndexVec<Node, Node>, - /// For each node, stores the single supernode that all of its successors - /// have been merged into. - /// - /// (Note that each node in a supernode can potentially have a _different_ - /// successor supernode from its peers.) - succ_supernodes: IndexVec<Node, Node>, -} - /// Creates a "merged" view of an underlying graph. /// /// The given graph is assumed to have [“balanced flow”](balanced-flow), diff --git a/compiler/rustc_mir_transform/src/coverage/counters/node_flow/tests.rs b/compiler/rustc_mir_transform/src/coverage/counters/node_flow/tests.rs index b509a14514b..46c46c743c2 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters/node_flow/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters/node_flow/tests.rs @@ -20,29 +20,25 @@ fn make_graph<Node: Idx + Ord>(num_nodes: usize, edge_pairs: Vec<(Node, Node)>) /// (Knuth & Stevenson, 1973), but with 0-based node IDs. #[test] fn example_driver() { - let graph = make_graph::<u32>(5, vec![ - (0, 1), - (0, 3), - (1, 0), - (1, 2), - (2, 1), - (2, 4), - (3, 3), - (3, 4), - (4, 0), - ]); + let graph = make_graph::<u32>( + 5, + vec![(0, 1), (0, 3), (1, 0), (1, 2), (2, 1), (2, 4), (3, 3), (3, 4), (4, 0)], + ); let node_flow_data = node_flow_data(&graph); let counters = make_node_counters(&node_flow_data, &[3, 1, 2, 0, 4]); - assert_eq!(format_counter_expressions(&counters), &[ - // (comment to force vertical formatting for clarity) - "[0]: +c0", - "[1]: +c0 +c2 -c4", - "[2]: +c2", - "[3]: +c3", - "[4]: +c4", - ]); + assert_eq!( + format_counter_expressions(&counters), + &[ + // (comment to force vertical formatting for clarity) + "[0]: +c0", + "[1]: +c0 +c2 -c4", + "[2]: +c2", + "[3]: +c3", + "[4]: +c4", + ] + ); } fn format_counter_expressions<Node: Idx>(counters: &NodeCounters<Node>) -> Vec<String> { diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 392b54c8d81..6d84248ddfb 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -8,6 +8,7 @@ use rustc_data_structures::graph::dominators::Dominators; use rustc_data_structures::graph::{self, DirectedGraph, StartNode}; use rustc_index::IndexVec; use rustc_index::bit_set::DenseBitSet; +pub(crate) use rustc_middle::mir::coverage::{BasicCoverageBlock, START_BCB}; use rustc_middle::mir::{self, BasicBlock, Terminator, TerminatorKind}; use tracing::debug; @@ -269,15 +270,6 @@ impl graph::Predecessors for CoverageGraph { } } -rustc_index::newtype_index! { - /// A node in the control-flow graph of CoverageGraph. - #[orderable] - #[debug_format = "bcb{}"] - pub(crate) struct BasicCoverageBlock { - const START_BCB = 0; - } -} - /// `BasicCoverageBlockData` holds the data indexed by a `BasicCoverageBlock`. /// /// A `BasicCoverageBlock` (BCB) represents the maximal-length sequence of MIR `BasicBlock`s without diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs index 8d0d92dc367..d83c0d40a7e 100644 --- a/compiler/rustc_mir_transform/src/coverage/mappings.rs +++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs @@ -1,9 +1,7 @@ use std::collections::BTreeSet; use rustc_data_structures::fx::FxIndexMap; -use rustc_data_structures::graph::DirectedGraph; use rustc_index::IndexVec; -use rustc_index::bit_set::DenseBitSet; use rustc_middle::mir::coverage::{ BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageInfoHi, CoverageKind, }; @@ -63,10 +61,6 @@ const MCDC_MAX_BITMAP_SIZE: usize = i32::MAX as usize; #[derive(Default)] pub(super) struct ExtractedMappings { - /// Store our own copy of [`CoverageGraph::num_nodes`], so that we don't - /// need access to the whole graph when allocating per-BCB data. This is - /// only public so that other code can still use exhaustive destructuring. - pub(super) num_bcbs: usize, pub(super) code_mappings: Vec<CodeMapping>, pub(super) branch_pairs: Vec<BranchPair>, pub(super) mcdc_bitmap_bits: usize, @@ -118,7 +112,6 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( ); ExtractedMappings { - num_bcbs: graph.num_nodes(), code_mappings, branch_pairs, mcdc_bitmap_bits, @@ -127,60 +120,6 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( } } -impl ExtractedMappings { - pub(super) fn all_bcbs_with_counter_mappings(&self) -> DenseBitSet<BasicCoverageBlock> { - // Fully destructure self to make sure we don't miss any fields that have mappings. - let Self { - num_bcbs, - code_mappings, - branch_pairs, - mcdc_bitmap_bits: _, - mcdc_degraded_branches, - mcdc_mappings, - } = self; - - // Identify which BCBs have one or more mappings. - let mut bcbs_with_counter_mappings = DenseBitSet::new_empty(*num_bcbs); - let mut insert = |bcb| { - bcbs_with_counter_mappings.insert(bcb); - }; - - for &CodeMapping { span: _, bcb } in code_mappings { - insert(bcb); - } - for &BranchPair { true_bcb, false_bcb, .. } in branch_pairs { - insert(true_bcb); - insert(false_bcb); - } - for &MCDCBranch { true_bcb, false_bcb, .. } in mcdc_degraded_branches - .iter() - .chain(mcdc_mappings.iter().map(|(_, branches)| branches.into_iter()).flatten()) - { - insert(true_bcb); - insert(false_bcb); - } - - // MC/DC decisions refer to BCBs, but don't require those BCBs to have counters. - if bcbs_with_counter_mappings.is_empty() { - debug_assert!( - mcdc_mappings.is_empty(), - "A function with no counter mappings shouldn't have any decisions: {mcdc_mappings:?}", - ); - } - - bcbs_with_counter_mappings - } - - /// Returns the set of BCBs that have one or more `Code` mappings. - pub(super) fn bcbs_with_ordinary_code_mappings(&self) -> DenseBitSet<BasicCoverageBlock> { - let mut bcbs = DenseBitSet::new_empty(self.num_bcbs); - for &CodeMapping { span: _, bcb } in &self.code_mappings { - bcbs.insert(bcb); - } - bcbs - } -} - fn resolve_block_markers( coverage_info_hi: &CoverageInfoHi, mir_body: &mir::Body<'_>, diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 15487d05a30..e195681bc92 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -21,7 +21,7 @@ use rustc_span::Span; use rustc_span::def_id::LocalDefId; use tracing::{debug, debug_span, trace}; -use crate::coverage::counters::CoverageCounters; +use crate::coverage::counters::BcbCountersData; use crate::coverage::graph::CoverageGraph; use crate::coverage::mappings::ExtractedMappings; @@ -82,28 +82,21 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: let extracted_mappings = mappings::extract_all_mapping_info_from_mir(tcx, mir_body, &hir_info, &graph); - //////////////////////////////////////////////////// - // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure - // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock` - // and all `Expression` dependencies (operands) are also generated, for any other - // `BasicCoverageBlock`s not already associated with a coverage span. - let bcbs_with_counter_mappings = extracted_mappings.all_bcbs_with_counter_mappings(); - if bcbs_with_counter_mappings.is_empty() { - // No relevant spans were found in MIR, so skip instrumenting this function. - return; - } - - let coverage_counters = counters::make_bcb_counters(&graph, &bcbs_with_counter_mappings); - - let mappings = create_mappings(&extracted_mappings, &coverage_counters); + let mappings = create_mappings(&extracted_mappings); if mappings.is_empty() { // No spans could be converted into valid mappings, so skip this function. debug!("no spans could be converted into valid mappings; skipping"); return; } - inject_coverage_statements(mir_body, &graph, &extracted_mappings, &coverage_counters); + // Use the coverage graph to prepare intermediate data that will eventually + // be used to assign physical counters and counter expressions to points in + // the control-flow graph + let BcbCountersData { node_flow_data, priority_list } = + counters::prepare_bcb_counters_data(&graph); + // Inject coverage statements into MIR. + inject_coverage_statements(mir_body, &graph); inject_mcdc_statements(mir_body, &graph, &extracted_mappings); let mcdc_num_condition_bitmaps = extracted_mappings @@ -116,29 +109,25 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { function_source_hash: hir_info.function_source_hash, body_span: hir_info.body_span, - num_counters: coverage_counters.num_counters(), - mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits, - expressions: coverage_counters.into_expressions(), + + node_flow_data, + priority_list, + mappings, + + mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits, mcdc_num_condition_bitmaps, })); } -/// For each coverage span extracted from MIR, create a corresponding -/// mapping. +/// For each coverage span extracted from MIR, create a corresponding mapping. /// -/// Precondition: All BCBs corresponding to those spans have been given -/// coverage counters. -fn create_mappings( - extracted_mappings: &ExtractedMappings, - coverage_counters: &CoverageCounters, -) -> Vec<Mapping> { - let term_for_bcb = - |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); - +/// FIXME(Zalathar): This used to be where BCBs in the extracted mappings were +/// resolved to a `CovTerm`. But that is now handled elsewhere, so this +/// function can potentially be simplified even further. +fn create_mappings(extracted_mappings: &ExtractedMappings) -> Vec<Mapping> { // Fully destructure the mappings struct to make sure we don't miss any kinds. let ExtractedMappings { - num_bcbs: _, code_mappings, branch_pairs, mcdc_bitmap_bits: _, @@ -150,23 +139,18 @@ fn create_mappings( mappings.extend(code_mappings.iter().map( // Ordinary code mappings are the simplest kind. |&mappings::CodeMapping { span, bcb }| { - let kind = MappingKind::Code(term_for_bcb(bcb)); + let kind = MappingKind::Code { bcb }; Mapping { kind, span } }, )); mappings.extend(branch_pairs.iter().map( |&mappings::BranchPair { span, true_bcb, false_bcb }| { - let true_term = term_for_bcb(true_bcb); - let false_term = term_for_bcb(false_bcb); - let kind = MappingKind::Branch { true_term, false_term }; + let kind = MappingKind::Branch { true_bcb, false_bcb }; Mapping { kind, span } }, )); - let term_for_bcb = - |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); - // MCDC branch mappings are appended with their decisions in case decisions were ignored. mappings.extend(mcdc_degraded_branches.iter().map( |&mappings::MCDCBranch { @@ -176,11 +160,7 @@ fn create_mappings( condition_info: _, true_index: _, false_index: _, - }| { - let true_term = term_for_bcb(true_bcb); - let false_term = term_for_bcb(false_bcb); - Mapping { kind: MappingKind::Branch { true_term, false_term }, span } - }, + }| { Mapping { kind: MappingKind::Branch { true_bcb, false_bcb }, span } }, )); for (decision, branches) in mcdc_mappings { @@ -201,12 +181,10 @@ fn create_mappings( true_index: _, false_index: _, }| { - let true_term = term_for_bcb(true_bcb); - let false_term = term_for_bcb(false_bcb); Mapping { kind: MappingKind::MCDCBranch { - true_term, - false_term, + true_bcb, + false_bcb, mcdc_params: condition_info, }, span, @@ -227,41 +205,11 @@ fn create_mappings( mappings } -/// For each BCB node or BCB edge that has an associated coverage counter, -/// inject any necessary coverage statements into MIR. -fn inject_coverage_statements<'tcx>( - mir_body: &mut mir::Body<'tcx>, - graph: &CoverageGraph, - extracted_mappings: &ExtractedMappings, - coverage_counters: &CoverageCounters, -) { - // Inject counter-increment statements into MIR. - for (id, bcb) in coverage_counters.counter_increment_sites() { - let target_bb = graph[bcb].leader_bb(); - inject_statement(mir_body, CoverageKind::CounterIncrement { id }, target_bb); - } - - // For each counter expression that is directly associated with at least one - // span, we inject an "expression-used" statement, so that coverage codegen - // can check whether the injected statement survived MIR optimization. - // (BCB edges can't have spans, so we only need to process BCB nodes here.) - // - // We only do this for ordinary `Code` mappings, because branch and MC/DC - // mappings might have expressions that don't correspond to any single - // point in the control-flow graph. - // - // See the code in `rustc_codegen_llvm::coverageinfo::map_data` that deals - // with "expressions seen" and "zero terms". - let eligible_bcbs = extracted_mappings.bcbs_with_ordinary_code_mappings(); - for (bcb, expression_id) in coverage_counters - .bcb_nodes_with_coverage_expressions() - .filter(|&(bcb, _)| eligible_bcbs.contains(bcb)) - { - inject_statement( - mir_body, - CoverageKind::ExpressionUsed { id: expression_id }, - graph[bcb].leader_bb(), - ); +/// Inject any necessary coverage statements into MIR, so that they influence codegen. +fn inject_coverage_statements<'tcx>(mir_body: &mut mir::Body<'tcx>, graph: &CoverageGraph) { + for (bcb, data) in graph.iter_enumerated() { + let target_bb = data.leader_bb(); + inject_statement(mir_body, CoverageKind::VirtualCounter { bcb }, target_bb); } } diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 5e7b46182dc..cd89fbe772d 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -1,22 +1,20 @@ use rustc_data_structures::captures::Captures; use rustc_index::bit_set::DenseBitSet; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; -use rustc_middle::mir::coverage::{ - CounterId, CovTerm, CoverageIdsInfo, CoverageKind, Expression, ExpressionId, - FunctionCoverageInfo, MappingKind, Op, -}; +use rustc_middle::mir::coverage::{BasicCoverageBlock, CoverageIdsInfo, CoverageKind, MappingKind}; use rustc_middle::mir::{Body, Statement, StatementKind}; -use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::{self, TyCtxt}; use rustc_middle::util::Providers; use rustc_span::def_id::LocalDefId; use rustc_span::sym; use tracing::trace; +use crate::coverage::counters::node_flow::make_node_counters; +use crate::coverage::counters::{CoverageCounters, transcribe_counters}; + /// Registers query/hook implementations related to coverage. pub(crate) fn provide(providers: &mut Providers) { - providers.hooks.is_eligible_for_coverage = - |TyCtxtAt { tcx, .. }, def_id| is_eligible_for_coverage(tcx, def_id); + providers.hooks.is_eligible_for_coverage = is_eligible_for_coverage; providers.queries.coverage_attr_on = coverage_attr_on; providers.queries.coverage_ids_info = coverage_ids_info; } @@ -91,39 +89,57 @@ fn coverage_ids_info<'tcx>( let mir_body = tcx.instance_mir(instance_def); let fn_cov_info = mir_body.function_coverage_info.as_deref()?; - let mut counters_seen = DenseBitSet::new_empty(fn_cov_info.num_counters); - let mut expressions_seen = DenseBitSet::new_filled(fn_cov_info.expressions.len()); - - // For each expression ID that is directly used by one or more mappings, - // mark it as not-yet-seen. This indicates that we expect to see a - // corresponding `ExpressionUsed` statement during MIR traversal. - for mapping in fn_cov_info.mappings.iter() { - // Currently we only worry about ordinary code mappings. - // For branch and MC/DC mappings, expressions might not correspond - // to any particular point in the control-flow graph. - // (Keep this in sync with the injection of `ExpressionUsed` - // statements in the `InstrumentCoverage` MIR pass.) - if let MappingKind::Code(CovTerm::Expression(id)) = mapping.kind { - expressions_seen.remove(id); - } - } - + // Scan through the final MIR to see which BCBs survived MIR opts. + // Any BCB not in this set was optimized away. + let mut bcbs_seen = DenseBitSet::new_empty(fn_cov_info.priority_list.len()); for kind in all_coverage_in_mir_body(mir_body) { match *kind { - CoverageKind::CounterIncrement { id } => { - counters_seen.insert(id); - } - CoverageKind::ExpressionUsed { id } => { - expressions_seen.insert(id); + CoverageKind::VirtualCounter { bcb } => { + bcbs_seen.insert(bcb); } _ => {} } } - let zero_expressions = - identify_zero_expressions(fn_cov_info, &counters_seen, &expressions_seen); + // Determine the set of BCBs that are referred to by mappings, and therefore + // need a counter. Any node not in this set will only get a counter if it + // is part of the counter expression for a node that is in the set. + let mut bcb_needs_counter = + DenseBitSet::<BasicCoverageBlock>::new_empty(fn_cov_info.priority_list.len()); + for mapping in &fn_cov_info.mappings { + match mapping.kind { + MappingKind::Code { bcb } => { + bcb_needs_counter.insert(bcb); + } + MappingKind::Branch { true_bcb, false_bcb } => { + bcb_needs_counter.insert(true_bcb); + bcb_needs_counter.insert(false_bcb); + } + MappingKind::MCDCBranch { true_bcb, false_bcb, mcdc_params: _ } => { + bcb_needs_counter.insert(true_bcb); + bcb_needs_counter.insert(false_bcb); + } + MappingKind::MCDCDecision(_) => {} + } + } - Some(CoverageIdsInfo { counters_seen, zero_expressions }) + // FIXME(Zalathar): It should be possible to sort `priority_list[1..]` by + // `!bcbs_seen.contains(bcb)` to simplify the mappings even further, at the + // expense of some churn in the tests. When doing so, also consider removing + // the sorts in `transcribe_counters`. + let node_counters = make_node_counters(&fn_cov_info.node_flow_data, &fn_cov_info.priority_list); + let coverage_counters = transcribe_counters(&node_counters, &bcb_needs_counter, &bcbs_seen); + + let CoverageCounters { + phys_counter_for_node, next_counter_id, node_counters, expressions, .. + } = coverage_counters; + + Some(CoverageIdsInfo { + num_counters: next_counter_id.as_u32(), + phys_counter_for_node, + term_for_bcb: node_counters, + expressions, + }) } fn all_coverage_in_mir_body<'a, 'tcx>( @@ -141,94 +157,3 @@ fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { let scope_data = &body.source_scopes[statement.source_info.scope]; scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some() } - -/// Identify expressions that will always have a value of zero, and note their -/// IDs in a `DenseBitSet`. Mappings that refer to a zero expression can instead -/// become mappings to a constant zero value. -/// -/// This function mainly exists to preserve the simplifications that were -/// already being performed by the Rust-side expression renumbering, so that -/// the resulting coverage mappings don't get worse. -fn identify_zero_expressions( - fn_cov_info: &FunctionCoverageInfo, - counters_seen: &DenseBitSet<CounterId>, - expressions_seen: &DenseBitSet<ExpressionId>, -) -> DenseBitSet<ExpressionId> { - // The set of expressions that either were optimized out entirely, or - // have zero as both of their operands, and will therefore always have - // a value of zero. Other expressions that refer to these as operands - // can have those operands replaced with `CovTerm::Zero`. - let mut zero_expressions = DenseBitSet::new_empty(fn_cov_info.expressions.len()); - - // Simplify a copy of each expression based on lower-numbered expressions, - // and then update the set of always-zero expressions if necessary. - // (By construction, expressions can only refer to other expressions - // that have lower IDs, so one pass is sufficient.) - for (id, expression) in fn_cov_info.expressions.iter_enumerated() { - if !expressions_seen.contains(id) { - // If an expression was not seen, it must have been optimized away, - // so any operand that refers to it can be replaced with zero. - zero_expressions.insert(id); - continue; - } - - // We don't need to simplify the actual expression data in the - // expressions list; we can just simplify a temporary copy and then - // use that to update the set of always-zero expressions. - let Expression { mut lhs, op, mut rhs } = *expression; - - // If an expression has an operand that is also an expression, the - // operand's ID must be strictly lower. This is what lets us find - // all zero expressions in one pass. - let assert_operand_expression_is_lower = |operand_id: ExpressionId| { - assert!( - operand_id < id, - "Operand {operand_id:?} should be less than {id:?} in {expression:?}", - ) - }; - - // If an operand refers to a counter or expression that is always - // zero, then that operand can be replaced with `CovTerm::Zero`. - let maybe_set_operand_to_zero = |operand: &mut CovTerm| { - if let CovTerm::Expression(id) = *operand { - assert_operand_expression_is_lower(id); - } - - if is_zero_term(&counters_seen, &zero_expressions, *operand) { - *operand = CovTerm::Zero; - } - }; - maybe_set_operand_to_zero(&mut lhs); - maybe_set_operand_to_zero(&mut rhs); - - // Coverage counter values cannot be negative, so if an expression - // involves subtraction from zero, assume that its RHS must also be zero. - // (Do this after simplifications that could set the LHS to zero.) - if lhs == CovTerm::Zero && op == Op::Subtract { - rhs = CovTerm::Zero; - } - - // After the above simplifications, if both operands are zero, then - // we know that this expression is always zero too. - if lhs == CovTerm::Zero && rhs == CovTerm::Zero { - zero_expressions.insert(id); - } - } - - zero_expressions -} - -/// Returns `true` if the given term is known to have a value of zero, taking -/// into account knowledge of which counters are unused and which expressions -/// are always zero. -fn is_zero_term( - counters_seen: &DenseBitSet<CounterId>, - zero_expressions: &DenseBitSet<ExpressionId>, - term: CovTerm, -) -> bool { - match term { - CovTerm::Zero => true, - CovTerm::Counter(id) => !counters_seen.contains(id), - CovTerm::Expression(id) => zero_expressions.contains(id), - } -} diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 26ce743be36..73b68d7155c 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -137,8 +137,7 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> { // These coverage statements should not exist prior to coverage instrumentation. StatementKind::Coverage( - CoverageKind::CounterIncrement { .. } - | CoverageKind::ExpressionUsed { .. } + CoverageKind::VirtualCounter { .. } | CoverageKind::CondBitmapUpdate { .. } | CoverageKind::TestVectorBitmapUpdate { .. }, ) => bug!( diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index b2ee50de50a..3c0053c610d 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -129,15 +129,18 @@ impl<'tcx> MockBlocks<'tcx> { } fn call(&mut self, some_from_block: Option<BasicBlock>) -> BasicBlock { - self.add_block_from(some_from_block, TerminatorKind::Call { - func: Operand::Copy(self.dummy_place.clone()), - args: [].into(), - destination: self.dummy_place.clone(), - target: Some(TEMP_BLOCK), - unwind: UnwindAction::Continue, - call_source: CallSource::Misc, - fn_span: DUMMY_SP, - }) + self.add_block_from( + some_from_block, + TerminatorKind::Call { + func: Operand::Copy(self.dummy_place.clone()), + args: [].into(), + destination: self.dummy_place.clone(), + target: Some(TEMP_BLOCK), + unwind: UnwindAction::Continue, + call_source: CallSource::Misc, + fn_span: DUMMY_SP, + }, + ) } fn goto(&mut self, some_from_block: Option<BasicBlock>) -> BasicBlock { diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 8879e029346..90173da17f0 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -504,7 +504,8 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { | Rvalue::Cast(..) | Rvalue::BinaryOp(..) | Rvalue::Aggregate(..) - | Rvalue::ShallowInitBox(..) => { + | Rvalue::ShallowInitBox(..) + | Rvalue::WrapUnsafeBinder(..) => { // No modification is possible through these r-values. return ValueOrPlace::TOP; } diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 41de1b58b91..7395ad496db 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -575,6 +575,9 @@ impl WriteInfo { self.add_operand(op); } } + Rvalue::WrapUnsafeBinder(op, _) => { + self.add_operand(op); + } Rvalue::ThreadLocalRef(_) | Rvalue::NullaryOp(_, _) | Rvalue::Ref(_, _, _) diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index 3d560bdf75c..9a6a153c7ba 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -90,10 +90,12 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { let span = terminator.source_info.span; let foreign = fn_def_id.is_some(); - tcx.emit_node_span_lint(FFI_UNWIND_CALLS, lint_root, span, errors::FfiUnwindCall { + tcx.emit_node_span_lint( + FFI_UNWIND_CALLS, + lint_root, span, - foreign, - }); + errors::FfiUnwindCall { span, foreign }, + ); tainted = true; } diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 16e15fa12e0..1a2120ecd71 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -476,6 +476,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } ProjectionElem::OpaqueCast(ty) => ProjectionElem::OpaqueCast(ty), ProjectionElem::Subtype(ty) => ProjectionElem::Subtype(ty), + ProjectionElem::UnwrapUnsafeBinder(ty) => { + ProjectionElem::UnwrapUnsafeBinder(ty) + } // This should have been replaced by a `ConstantIndex` earlier. ProjectionElem::Index(_) => return None, }; @@ -542,6 +545,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { .offset_of_subfield(self.typing_env(), layout, fields.iter()) .bytes(), NullOp::UbChecks => return None, + NullOp::ContractChecks => return None, }; let usize_layout = self.ecx.layout_of(self.tcx.types.usize).unwrap(); let imm = ImmTy::from_uint(val, usize_layout); @@ -713,6 +717,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } ProjectionElem::OpaqueCast(ty) => ProjectionElem::OpaqueCast(ty), ProjectionElem::Subtype(ty) => ProjectionElem::Subtype(ty), + ProjectionElem::UnwrapUnsafeBinder(ty) => ProjectionElem::UnwrapUnsafeBinder(ty), }; Some(self.insert(Value::Projection(value, proj))) @@ -867,6 +872,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { self.simplify_place_projection(place, location); return self.new_pointer(*place, AddressKind::Address(mutbl)); } + Rvalue::WrapUnsafeBinder(ref mut op, _) => { + return self.simplify_operand(op, location); + } // Operations. Rvalue::Len(ref mut place) => return self.simplify_len(place, location), @@ -931,6 +939,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { ProjectionElem::Downcast(symbol, idx) => ProjectionElem::Downcast(symbol, idx), ProjectionElem::OpaqueCast(idx) => ProjectionElem::OpaqueCast(idx), ProjectionElem::Subtype(idx) => ProjectionElem::Subtype(idx), + ProjectionElem::UnwrapUnsafeBinder(ty) => ProjectionElem::UnwrapUnsafeBinder(ty), }) } @@ -1358,16 +1367,17 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { fn simplify_cast( &mut self, - kind: &mut CastKind, - operand: &mut Operand<'tcx>, + initial_kind: &mut CastKind, + initial_operand: &mut Operand<'tcx>, to: Ty<'tcx>, location: Location, ) -> Option<VnIndex> { use CastKind::*; use rustc_middle::ty::adjustment::PointerCoercion::*; - let mut from = operand.ty(self.local_decls, self.tcx); - let mut value = self.simplify_operand(operand, location)?; + let mut from = initial_operand.ty(self.local_decls, self.tcx); + let mut kind = *initial_kind; + let mut value = self.simplify_operand(initial_operand, location)?; if from == to { return Some(value); } @@ -1391,7 +1401,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { && to.is_unsafe_ptr() && self.pointers_have_same_metadata(from, to) { - *kind = PtrToPtr; + kind = PtrToPtr; was_updated_this_iteration = true; } @@ -1434,7 +1444,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { to: inner_to, } = *self.get(value) { - let new_kind = match (inner_kind, *kind) { + let new_kind = match (inner_kind, kind) { // Even if there's a narrowing cast in here that's fine, because // things like `*mut [i32] -> *mut i32 -> *const i32` and // `*mut [i32] -> *const [i32] -> *const i32` can skip the middle in MIR. @@ -1462,7 +1472,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { _ => None, }; if let Some(new_kind) = new_kind { - *kind = new_kind; + kind = new_kind; from = inner_from; value = inner_value; was_updated_this_iteration = true; @@ -1480,10 +1490,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } if was_ever_updated && let Some(op) = self.try_as_operand(value, location) { - *operand = op; + *initial_operand = op; + *initial_kind = kind; } - Some(self.insert(Value::Cast { kind: *kind, value, from, to })) + Some(self.insert(Value::Cast { kind, value, from, to })) } fn simplify_len(&mut self, place: &mut Place<'tcx>, location: Location) -> Option<VnIndex> { diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 95aeccfdda6..ab617e2ce6f 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -4,7 +4,7 @@ use std::iter; use std::ops::{Range, RangeFrom}; use rustc_abi::{ExternAbi, FieldIdx}; -use rustc_attr_parsing::InlineAttr; +use rustc_attr_parsing::{InlineAttr, OptimizeAttr}; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_index::Idx; @@ -770,6 +770,10 @@ fn check_codegen_attributes<'tcx, I: Inliner<'tcx>>( return Err("never inline attribute"); } + if let OptimizeAttr::DoNotOptimize = callee_attrs.optimize { + return Err("has DoNotOptimize attribute"); + } + // Reachability pass defines which functions are eligible for inlining. Generally inlining // other functions is incorrect because they could reference symbols that aren't exported. let is_generic = callsite.callee.args.non_erasable_generics().next().is_some(); @@ -1107,10 +1111,13 @@ fn new_call_temp<'tcx>( }); if let Some(block) = return_block { - caller_body[block].statements.insert(0, Statement { - source_info: callsite.source_info, - kind: StatementKind::StorageDead(local), - }); + caller_body[block].statements.insert( + 0, + Statement { + source_info: callsite.source_info, + kind: StatementKind::StorageDead(local), + }, + ); } local @@ -1250,6 +1257,8 @@ impl<'tcx> MutVisitor<'tcx> for Integrator<'_, 'tcx> { // replaced down below anyways). if !matches!(terminator.kind, TerminatorKind::Return) { self.super_terminator(terminator, loc); + } else { + self.visit_source_info(&mut terminator.source_info); } match terminator.kind { diff --git a/compiler/rustc_mir_transform/src/known_panics_lint.rs b/compiler/rustc_mir_transform/src/known_panics_lint.rs index f4ac5c6aa80..59de6ca84a7 100644 --- a/compiler/rustc_mir_transform/src/known_panics_lint.rs +++ b/compiler/rustc_mir_transform/src/known_panics_lint.rs @@ -296,11 +296,12 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let source_info = self.body.source_info(location); if let Some(lint_root) = self.lint_root(*source_info) { let span = source_info.span; - self.tcx.emit_node_span_lint(lint_kind.lint(), lint_root, span, AssertLint { + self.tcx.emit_node_span_lint( + lint_kind.lint(), + lint_root, span, - assert_kind, - lint_kind, - }); + AssertLint { span, assert_kind, lint_kind }, + ); } } @@ -444,7 +445,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { | Rvalue::Cast(..) | Rvalue::ShallowInitBox(..) | Rvalue::Discriminant(..) - | Rvalue::NullaryOp(..) => {} + | Rvalue::NullaryOp(..) + | Rvalue::WrapUnsafeBinder(..) => {} } // FIXME we need to revisit this for #67176 @@ -546,7 +548,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let val: Value<'_> = match *rvalue { ThreadLocalRef(_) => return None, - Use(ref operand) => self.eval_operand(operand)?.into(), + Use(ref operand) | WrapUnsafeBinder(ref operand, _) => { + self.eval_operand(operand)?.into() + } CopyForDeref(place) => self.eval_place(place)?.into(), @@ -626,6 +630,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { .offset_of_subfield(self.typing_env, op_layout, fields.iter()) .bytes(), NullOp::UbChecks => return None, + NullOp::ContractChecks => return None, }; ImmTy::from_scalar(Scalar::from_target_usize(val, self), layout).into() } diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 2dc55e3614e..b572f6ca0b3 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -33,6 +33,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt}; use rustc_middle::util::Providers; use rustc_middle::{bug, query, span_bug}; +use rustc_mir_build::builder::build_mir; use rustc_span::source_map::Spanned; use rustc_span::{DUMMY_SP, sym}; use tracing::debug; @@ -44,6 +45,7 @@ use std::sync::LazyLock; use pass_manager::{self as pm, Lint, MirLint, MirPass, WithMinOptLevel}; +mod check_pointers; mod cost_checker; mod cross_crate_inline; mod deduce_param_attrs; @@ -118,6 +120,7 @@ declare_passes! { mod check_call_recursion : CheckCallRecursion, CheckDropRecursion; mod check_alignment : CheckAlignment; mod check_const_item_mutation : CheckConstItemMutation; + mod check_null : CheckNull; mod check_packed_ref : CheckPackedRef; mod check_undefined_transmutes : CheckUndefinedTransmutes; // This pass is public to allow external drivers to perform MIR cleanup @@ -368,7 +371,7 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def: LocalDefId) -> ConstQualifs { } fn mir_built(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> { - let mut body = tcx.build_mir(def); + let mut body = build_mir(tcx, def); pass_manager::dump_mir_for_phase_change(tcx, &body); @@ -418,11 +421,11 @@ fn mir_promoted( }; // the `has_ffi_unwind_calls` query uses the raw mir, so make sure it is run. - tcx.ensure_with_value().has_ffi_unwind_calls(def); + tcx.ensure_done().has_ffi_unwind_calls(def); // the `by_move_body` query uses the raw mir, so make sure it is run. if tcx.needs_coroutine_by_move_body_def_id(def.to_def_id()) { - tcx.ensure_with_value().coroutine_by_move_body_def_id(def); + tcx.ensure_done().coroutine_by_move_body_def_id(def); } let mut body = tcx.mir_built(def).steal(); @@ -485,7 +488,7 @@ fn inner_mir_for_ctfe(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> { /// end up missing the source MIR due to stealing happening. fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> { if tcx.is_coroutine(def.to_def_id()) { - tcx.ensure_with_value().mir_coroutine_witnesses(def); + tcx.ensure_done().mir_coroutine_witnesses(def); } // We only need to borrowck non-synthetic MIR. @@ -498,7 +501,7 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> & if pm::should_run_pass(tcx, &inline::Inline, pm::Optimizations::Allowed) || inline::ForceInline::should_run_pass_for_callee(tcx, def.to_def_id()) { - tcx.ensure_with_value().mir_inliner_callees(ty::InstanceKind::Item(def.to_def_id())); + tcx.ensure_done().mir_inliner_callees(ty::InstanceKind::Item(def.to_def_id())); } } @@ -642,6 +645,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { &[ // Add some UB checks before any UB gets optimized away. &check_alignment::CheckAlignment, + &check_null::CheckNull, // 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. @@ -729,7 +733,7 @@ fn inner_optimized_mir(tcx: TyCtxt<'_>, did: LocalDefId) -> Body<'_> { // Run the `mir_for_ctfe` query, which depends on `mir_drops_elaborated_and_const_checked` // which we are going to steal below. Thus we need to run `mir_for_ctfe` first, so it // computes and caches its result. - Some(hir::ConstContext::ConstFn) => tcx.ensure_with_value().mir_for_ctfe(did), + Some(hir::ConstContext::ConstFn) => tcx.ensure_done().mir_for_ctfe(did), None => {} Some(other) => panic!("do not use `optimized_mir` for constants: {other:?}"), } @@ -768,7 +772,7 @@ fn promoted_mir(tcx: TyCtxt<'_>, def: LocalDefId) -> &IndexVec<Promoted, Body<'_ } if !tcx.is_synthetic_mir(def) { - tcx.ensure_with_value().mir_borrowck(def); + tcx.ensure_done().mir_borrowck(def); } let mut promoted = tcx.mir_promoted(def).1.steal(); diff --git a/compiler/rustc_mir_transform/src/lower_intrinsics.rs b/compiler/rustc_mir_transform/src/lower_intrinsics.rs index 9a9f66ed4fd..9c21bcfc0d2 100644 --- a/compiler/rustc_mir_transform/src/lower_intrinsics.rs +++ b/compiler/rustc_mir_transform/src/lower_intrinsics.rs @@ -34,6 +34,17 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics { }); terminator.kind = TerminatorKind::Goto { target }; } + sym::contract_checks => { + let target = target.unwrap(); + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::Assign(Box::new(( + *destination, + Rvalue::NullaryOp(NullOp::ContractChecks, tcx.types.bool), + ))), + }); + terminator.kind = TerminatorKind::Goto { target }; + } sym::forget => { let target = target.unwrap(); block.statements.push(Statement { diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index 6aa3343bb6e..c8d8dc147e9 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -293,7 +293,8 @@ impl<'tcx> Validator<'_, 'tcx> { // Recurse directly. ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subtype(_) - | ProjectionElem::Subslice { .. } => {} + | ProjectionElem::Subslice { .. } + | ProjectionElem::UnwrapUnsafeBinder(_) => {} // Never recurse. ProjectionElem::OpaqueCast(..) | ProjectionElem::Downcast(..) => { @@ -426,7 +427,9 @@ impl<'tcx> Validator<'_, 'tcx> { fn validate_rvalue(&mut self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> { match rvalue { - Rvalue::Use(operand) | Rvalue::Repeat(operand, _) => { + Rvalue::Use(operand) + | Rvalue::Repeat(operand, _) + | Rvalue::WrapUnsafeBinder(operand, _) => { self.validate_operand(operand)?; } Rvalue::CopyForDeref(place) => { @@ -454,6 +457,7 @@ impl<'tcx> Validator<'_, 'tcx> { NullOp::AlignOf => {} NullOp::OffsetOf(_) => {} NullOp::UbChecks => {} + NullOp::ContractChecks => {} }, Rvalue::ShallowInitBox(_, _) => return Err(Unpromotable), @@ -916,19 +920,23 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { self.extra_statements.push((loc, promoted_ref_statement)); ( - Rvalue::Ref(tcx.lifetimes.re_erased, *borrow_kind, Place { - local: mem::replace(&mut place.local, promoted_ref), - projection: List::empty(), - }), + Rvalue::Ref( + tcx.lifetimes.re_erased, + *borrow_kind, + Place { + local: mem::replace(&mut place.local, promoted_ref), + projection: List::empty(), + }, + ), promoted_operand, ) }; assert_eq!(self.new_block(), START_BLOCK); - self.visit_rvalue(&mut rvalue, Location { - block: START_BLOCK, - statement_index: usize::MAX, - }); + self.visit_rvalue( + &mut rvalue, + Location { block: START_BLOCK, statement_index: usize::MAX }, + ); let span = self.promoted.span; self.assign(RETURN_PLACE, rvalue, span); diff --git a/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs b/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs index 1d53440cf0b..94b1b4b1855 100644 --- a/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs +++ b/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs @@ -447,17 +447,19 @@ impl<'tcx> AsyncDestructorCtorShimBuilder<'tcx> { } fn combine_sync_surface(&mut self) -> Ty<'tcx> { - self.apply_combinator(1, LangItem::AsyncDropSurfaceDropInPlace, &[self - .self_ty - .unwrap() - .into()]) + self.apply_combinator( + 1, + LangItem::AsyncDropSurfaceDropInPlace, + &[self.self_ty.unwrap().into()], + ) } fn combine_deferred_drop_in_place(&mut self) -> Ty<'tcx> { - self.apply_combinator(1, LangItem::AsyncDropDeferredDropInPlace, &[self - .self_ty - .unwrap() - .into()]) + self.apply_combinator( + 1, + LangItem::AsyncDropDeferredDropInPlace, + &[self.self_ty.unwrap().into()], + ) } fn combine_fuse(&mut self, inner_future_ty: Ty<'tcx>) -> Ty<'tcx> { @@ -477,11 +479,11 @@ impl<'tcx> AsyncDestructorCtorShimBuilder<'tcx> { } fn combine_either(&mut self, other: Ty<'tcx>, matched: Ty<'tcx>) -> Ty<'tcx> { - self.apply_combinator(4, LangItem::AsyncDropEither, &[ - other.into(), - matched.into(), - self.self_ty.unwrap().into(), - ]) + self.apply_combinator( + 4, + LangItem::AsyncDropEither, + &[other.into(), matched.into(), self.self_ty.unwrap().into()], + ) } fn return_(mut self) -> Body<'tcx> { diff --git a/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs b/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs index c0f25c7ecfe..21bc51ecca1 100644 --- a/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs +++ b/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs @@ -113,10 +113,13 @@ impl<'tcx> crate::MirPass<'tcx> for SimplifyComparisonIntegral { // if we have StorageDeads to remove then make sure to insert them at the top of // each target for bb_idx in new_targets.all_targets() { - storage_deads_to_insert.push((*bb_idx, Statement { - source_info: terminator.source_info, - kind: StatementKind::StorageDead(opt.to_switch_on.local), - })); + storage_deads_to_insert.push(( + *bb_idx, + Statement { + source_info: terminator.source_info, + kind: StatementKind::StorageDead(opt.to_switch_on.local), + }, + )); } } diff --git a/compiler/rustc_mir_transform/src/ssa.rs b/compiler/rustc_mir_transform/src/ssa.rs index a24b3b2e80f..9f769077e77 100644 --- a/compiler/rustc_mir_transform/src/ssa.rs +++ b/compiler/rustc_mir_transform/src/ssa.rs @@ -159,10 +159,11 @@ impl SsaLocals { ) { for &local in &self.assignment_order { match self.assignments[local] { - Set1::One(DefLocation::Argument) => f(local, AssignedValue::Arg, Location { - block: START_BLOCK, - statement_index: 0, - }), + Set1::One(DefLocation::Argument) => f( + local, + AssignedValue::Arg, + Location { block: START_BLOCK, statement_index: 0 }, + ), Set1::One(DefLocation::Assignment(loc)) => { let bb = &mut basic_blocks[loc.block]; // `loc` must point to a direct assignment to `local`. diff --git a/compiler/rustc_mir_transform/src/validate.rs b/compiler/rustc_mir_transform/src/validate.rs index 5881264cba5..4ac3a268c9c 100644 --- a/compiler/rustc_mir_transform/src/validate.rs +++ b/compiler/rustc_mir_transform/src/validate.rs @@ -97,6 +97,12 @@ impl<'tcx> crate::MirPass<'tcx> for Validator { } } +/// This checker covers basic properties of the control-flow graph, (dis)allowed statements and terminators. +/// Everything checked here must be stable under substitution of generic parameters. In other words, +/// this is about the *structure* of the MIR, not the *contents*. +/// +/// Everything that depends on types, or otherwise can be affected by generic parameters, +/// must be checked in `TypeChecker`. struct CfgChecker<'a, 'tcx> { when: &'a str, body: &'a Body<'tcx>, @@ -807,6 +813,25 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ) } } + ProjectionElem::UnwrapUnsafeBinder(unwrapped_ty) => { + let binder_ty = place_ref.ty(&self.body.local_decls, self.tcx); + let ty::UnsafeBinder(binder_ty) = *binder_ty.ty.kind() else { + self.fail( + location, + format!("WrapUnsafeBinder does not produce a ty::UnsafeBinder"), + ); + return; + }; + let binder_inner_ty = self.tcx.instantiate_bound_regions_with_erased(*binder_ty); + if !self.mir_assign_valid_types(unwrapped_ty, binder_inner_ty) { + self.fail( + location, + format!( + "Cannot unwrap unsafe binder {binder_ty:?} into type {unwrapped_ty:?}" + ), + ); + } + } _ => {} } self.super_projection_elem(place_ref, elem, context, location); @@ -1360,8 +1385,29 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { Rvalue::Repeat(_, _) | Rvalue::ThreadLocalRef(_) | Rvalue::RawPtr(_, _) - | Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::UbChecks, _) + | Rvalue::NullaryOp( + NullOp::SizeOf | NullOp::AlignOf | NullOp::UbChecks | NullOp::ContractChecks, + _, + ) | Rvalue::Discriminant(_) => {} + + Rvalue::WrapUnsafeBinder(op, ty) => { + let unwrapped_ty = op.ty(self.body, self.tcx); + let ty::UnsafeBinder(binder_ty) = *ty.kind() else { + self.fail( + location, + format!("WrapUnsafeBinder does not produce a ty::UnsafeBinder"), + ); + return; + }; + let binder_inner_ty = self.tcx.instantiate_bound_regions_with_erased(*binder_ty); + if !self.mir_assign_valid_types(unwrapped_ty, binder_inner_ty) { + self.fail( + location, + format!("Cannot wrap {unwrapped_ty:?} into unsafe binder {binder_ty:?}"), + ); + } + } } self.super_rvalue(rvalue, location); } |