mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove dead unwinds before drop elaboration

Browse Source 
As a part of drop elaboration, we identify dead unwinds, i.e., unwind
edges on a drop terminators which are known to be unreachable, because
there is no need to drop anything.

Previously, the data flow framework was informed about the dead unwinds,
and it assumed those edges are absent from MIR. Unfortunately, the data
flow framework wasn't consistent in maintaining this assumption.

In particular, if a block was reachable only through a dead unwind edge,
its state was propagated to other blocks still. This became an issue in
the context of change removes DropAndReplace terminator, since it
introduces initialization into cleanup blocks.

To avoid this issue, remove unreachable unwind edges before the drop
elaboration, and elaborate only blocks that remain reachable.
This commit is contained in:
Tomasz Miąsko 2023-01-05 10:15:33 +01:00
parent 00eec854f1
commit 23e3840014
3 changed files with 44 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
compiler/rustc_mir_dataflow/src/framework/direction.rs
View File

@ -1,4 +1,3 @@
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::{self, BasicBlock, Location, SwitchTargets};
use rustc_middle::ty::TyCtxt;
use std::ops::RangeInclusive;
@ -54,7 +53,6 @@ fn join_state_into_successors_of<'tcx, A>(
analysis: &A,
tcx: TyCtxt<'tcx>,
body: &mir::Body<'tcx>,
dead_unwinds: Option<&BitSet<BasicBlock>>,
exit_state: &mut A::Domain,
block: (BasicBlock, &'_ mir::BasicBlockData<'tcx>),
propagate: impl FnMut(BasicBlock, &A::Domain),
@ -221,7 +219,6 @@ fn join_state_into_successors_of<'tcx, A>(
analysis: &A,
_tcx: TyCtxt<'tcx>,
body: &mir::Body<'tcx>,
dead_unwinds: Option<&BitSet<BasicBlock>>,
exit_state: &mut A::Domain,
(bb, _bb_data): (BasicBlock, &'_ mir::BasicBlockData<'tcx>),
mut propagate: impl FnMut(BasicBlock, &A::Domain),
@ -278,20 +275,6 @@ fn join_state_into_successors_of<'tcx, A>(
}
}
// Ignore dead unwinds.
mir::TerminatorKind::Call { cleanup: Some(unwind), .. }
| mir::TerminatorKind::Assert { cleanup: Some(unwind), .. }
| mir::TerminatorKind::Drop { unwind: Some(unwind), .. }
| mir::TerminatorKind::DropAndReplace { unwind: Some(unwind), .. }
| mir::TerminatorKind::FalseUnwind { unwind: Some(unwind), .. }
| mir::TerminatorKind::InlineAsm { cleanup: Some(unwind), .. }
if unwind == bb =>
{
if dead_unwinds.map_or(true, |dead| !dead.contains(pred)) {
propagate(pred, exit_state);
}
}
_ => propagate(pred, exit_state),
}
}
@ -304,7 +287,6 @@ struct BackwardSwitchIntEdgeEffectsApplier<'a, 'tcx, D, F> {
exit_state: &'a mut D,
bb: BasicBlock,
propagate: &'a mut F,
effects_applied: bool,
}
@ -484,7 +466,6 @@ fn join_state_into_successors_of<'tcx, A>(
analysis: &A,
_tcx: TyCtxt<'tcx>,
_body: &mir::Body<'tcx>,
dead_unwinds: Option<&BitSet<BasicBlock>>,
exit_state: &mut A::Domain,
(bb, bb_data): (BasicBlock, &'_ mir::BasicBlockData<'tcx>),
mut propagate: impl FnMut(BasicBlock, &A::Domain),
@ -502,9 +483,7 @@ fn join_state_into_successors_of<'tcx, A>(
| DropAndReplace { target, unwind, value: _, place: _ }
| FalseUnwind { real_target: target, unwind } => {
if let Some(unwind) = unwind {
if dead_unwinds.map_or(true, |dead| !dead.contains(bb)) {
propagate(unwind, exit_state);
}
propagate(unwind, exit_state);
}
propagate(target, exit_state);
@ -534,9 +513,7 @@ fn join_state_into_successors_of<'tcx, A>(
fn_span: _,
} => {
if let Some(unwind) = cleanup {
if dead_unwinds.map_or(true, |dead| !dead.contains(bb)) {
propagate(unwind, exit_state);
}
propagate(unwind, exit_state);
}
if let Some(target) = target {
@ -560,9 +537,7 @@ fn join_state_into_successors_of<'tcx, A>(
cleanup,
} => {
if let Some(unwind) = cleanup {
if dead_unwinds.map_or(true, |dead| !dead.contains(bb)) {
propagate(unwind, exit_state);
}
propagate(unwind, exit_state);
}
if let Some(target) = destination {

32
compiler/rustc_mir_dataflow/src/framework/engine.rs
View File

@ -12,7 +12,6 @@
use rustc_data_structures::work_queue::WorkQueue;
use rustc_graphviz as dot;
use rustc_hir::def_id::DefId;
use rustc_index::bit_set::BitSet;
use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::mir::{self, traversal, BasicBlock};
use rustc_middle::mir::{create_dump_file, dump_enabled};
@ -78,7 +77,6 @@ pub struct Engine<'a, 'tcx, A>
{
tcx: TyCtxt<'tcx>,
body: &'a mir::Body<'tcx>,
dead_unwinds: Option<&'a BitSet<BasicBlock>>,
entry_sets: IndexVec<BasicBlock, A::Domain>,
pass_name: Option<&'static str>,
analysis: A,
@ -154,25 +152,7 @@ fn new(
bug!("`initialize_start_block` is not yet supported for backward dataflow analyses");
}
Engine {
analysis,
tcx,
body,
dead_unwinds: None,
pass_name: None,
entry_sets,
apply_trans_for_block,
}
}
/// Signals that we do not want dataflow state to propagate across unwind edges for these
/// `BasicBlock`s.
///
/// You must take care that `dead_unwinds` does not contain a `BasicBlock` that *can* actually
/// unwind during execution. Otherwise, your dataflow results will not be correct.
pub fn dead_unwinds(mut self, dead_unwinds: &'a BitSet<BasicBlock>) -> Self {
self.dead_unwinds = Some(dead_unwinds);
self
Engine { analysis, tcx, body, pass_name: None, entry_sets, apply_trans_for_block }
}
/// Adds an identifier to the graphviz output for this particular run of a dataflow analysis.
@ -190,14 +170,7 @@ pub fn iterate_to_fixpoint(self) -> Results<'tcx, A>
A::Domain: DebugWithContext<A>,
{
let Engine {
analysis,
body,
dead_unwinds,
mut entry_sets,
tcx,
apply_trans_for_block,
pass_name,
..
analysis, body, mut entry_sets, tcx, apply_trans_for_block, pass_name, ..
} = self;
let mut dirty_queue: WorkQueue<BasicBlock> = WorkQueue::with_none(body.basic_blocks.len());
@ -236,7 +209,6 @@ pub fn iterate_to_fixpoint(self) -> Results<'tcx, A>
&analysis,
tcx,
body,
dead_unwinds,
&mut state,
(bb, bb_data),
|target: BasicBlock, state: &A::Domain| {

57
compiler/rustc_mir_transform/src/elaborate_drops.rs
View File

@ -67,13 +67,11 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
};
let un_derefer = UnDerefer { tcx: tcx, derefer_sidetable: side_table };
let elaborate_patch = {
let body = &*body;
let env = MoveDataParamEnv { move_data, param_env };
let dead_unwinds = find_dead_unwinds(tcx, body, &env, &un_derefer);
remove_dead_unwinds(tcx, body, &env, &un_derefer);
let inits = MaybeInitializedPlaces::new(tcx, body, &env)
.into_engine(tcx, body)
.dead_unwinds(&dead_unwinds)
.pass_name("elaborate_drops")
.iterate_to_fixpoint()
.into_results_cursor(body);
@ -81,11 +79,12 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let uninits = MaybeUninitializedPlaces::new(tcx, body, &env)
.mark_inactive_variants_as_uninit()
.into_engine(tcx, body)
.dead_unwinds(&dead_unwinds)
.pass_name("elaborate_drops")
.iterate_to_fixpoint()
.into_results_cursor(body);
let reachable = traversal::reachable_as_bitset(body);
ElaborateDropsCtxt {
tcx,
body,
@ -94,6 +93,7 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
drop_flags: Default::default(),
patch: MirPatch::new(body),
un_derefer: un_derefer,
reachable,
}
.elaborate()
};
@ -102,22 +102,21 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
}
}
/// Returns the set of basic blocks whose unwind edges are known
/// to not be reachable, because they are `drop` terminators
/// Removes unwind edges which are known to be unreachable, because they are in `drop` terminators
/// that can't drop anything.
fn find_dead_unwinds<'tcx>(
fn remove_dead_unwinds<'tcx>(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
body: &mut Body<'tcx>,
env: &MoveDataParamEnv<'tcx>,
und: &UnDerefer<'tcx>,
) -> BitSet<BasicBlock> {
debug!("find_dead_unwinds({:?})", body.span);
) {
debug!("remove_dead_unwinds({:?})", body.span);
// We only need to do this pass once, because unwind edges can only
// reach cleanup blocks, which can't have unwind edges themselves.
let mut dead_unwinds = BitSet::new_empty(body.basic_blocks.len());
let mut dead_unwinds = Vec::new();
let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, &env)
.into_engine(tcx, body)
.pass_name("find_dead_unwinds")
.pass_name("remove_dead_unwinds")
.iterate_to_fixpoint()
.into_results_cursor(body);
for (bb, bb_data) in body.basic_blocks.iter_enumerated() {
@ -129,16 +128,16 @@ fn find_dead_unwinds<'tcx>(
_ => continue,
};
debug!("find_dead_unwinds @ {:?}: {:?}", bb, bb_data);
debug!("remove_dead_unwinds @ {:?}: {:?}", bb, bb_data);
let LookupResult::Exact(path) = env.move_data.rev_lookup.find(place.as_ref()) else {
debug!("find_dead_unwinds: has parent; skipping");
debug!("remove_dead_unwinds: has parent; skipping");
continue;
};
flow_inits.seek_before_primary_effect(body.terminator_loc(bb));
debug!(
"find_dead_unwinds @ {:?}: path({:?})={:?}; init_data={:?}",
"remove_dead_unwinds @ {:?}: path({:?})={:?}; init_data={:?}",
bb,
place,
path,
@ -150,13 +149,22 @@ fn find_dead_unwinds<'tcx>(
maybe_live |= flow_inits.contains(child);
});
debug!("find_dead_unwinds @ {:?}: maybe_live={}", bb, maybe_live);
debug!("remove_dead_unwinds @ {:?}: maybe_live={}", bb, maybe_live);
if !maybe_live {
dead_unwinds.insert(bb);
dead_unwinds.push(bb);
}
}
dead_unwinds
if dead_unwinds.is_empty() {
return;
}
let basic_blocks = body.basic_blocks.as_mut();
for &bb in dead_unwinds.iter() {
if let Some(unwind) = basic_blocks[bb].terminator_mut().unwind_mut() {
*unwind = None;
}
}
}
struct InitializationData<'mir, 'tcx> {
@ -290,6 +298,7 @@ struct ElaborateDropsCtxt<'a, 'tcx> {
drop_flags: FxHashMap<MovePathIndex, Local>,
patch: MirPatch<'tcx>,
un_derefer: UnDerefer<'tcx>,
reachable: BitSet<BasicBlock>,
}
impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
@ -329,6 +338,9 @@ fn elaborate(mut self) -> MirPatch<'tcx> {
fn collect_drop_flags(&mut self) {
for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
let terminator = data.terminator();
let place = match terminator.kind {
TerminatorKind::Drop { ref place, .. }
@ -384,6 +396,9 @@ fn collect_drop_flags(&mut self) {
fn elaborate_drops(&mut self) {
for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
let loc = Location { block: bb, statement_index: data.statements.len() };
let terminator = data.terminator();
@ -541,6 +556,9 @@ fn drop_flags_on_init(&mut self) {
fn drop_flags_for_fn_rets(&mut self) {
for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
if let TerminatorKind::Call {
destination, target: Some(tgt), cleanup: Some(_), ..
} = data.terminator().kind
@ -576,6 +594,9 @@ fn drop_flags_for_locs(&mut self) {
// clobbered before they are read.
for (bb, data) in self.body.basic_blocks.iter_enumerated() {
if !self.reachable.contains(bb) {
continue;
}
debug!("drop_flags_for_locs({:?})", data);
for i in 0..(data.statements.len() + 1) {
debug!("drop_flag_for_locs: stmt {}", i);