From 68c1e2fd484b430726881f822311b4e9aa9c044d Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Tue, 24 Jan 2023 18:48:05 +0100 Subject: [PATCH] Treat Drop as a rmw operation Previously, a Drop terminator was considered a move in MIR. This commit changes the behavior to only treat Drop as a mutable access to the dropped place. In order for this change to be correct, we need to guarantee that a) A dropped value won't be used again b) Places that appear in a drop won't be used again before a subsequent initialization. We can ensure this to be correct at MIR construction because Drop will only be emitted when a variable goes out of scope, thus having: (a) as there is no way of reaching the old value. drop-elaboration will also remove any uninitialized drop. (b) as the place can't be named following the end of the scope. However, the initialization status, previously tracked by moves, should also be tied to the execution of a Drop, hence the additional logic in the dataflow analyses. --- .../src/drop_flag_effects.rs | 13 ++++++++- .../src/move_paths/builder.rs | 7 ++--- .../rustc_mir_dataflow/src/value_analysis.rs | 6 ++-- .../src/elaborate_drops.rs | 29 +++++++++++++++++++ 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs index 3224e13f7af4b..0d466bbe56e01 100644 --- a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs +++ b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs @@ -1,5 +1,5 @@ use crate::elaborate_drops::DropFlagState; -use rustc_middle::mir::{self, Body, Location}; +use rustc_middle::mir::{self, Body, Location, Terminator, TerminatorKind}; use rustc_middle::ty::{self, TyCtxt}; use rustc_target::abi::VariantIdx; @@ -194,6 +194,17 @@ pub fn drop_flag_effects_for_location<'tcx, F>( on_all_children_bits(tcx, body, move_data, path, |mpi| callback(mpi, DropFlagState::Absent)) } + // Drop does not count as a move but we should still consider the variable uninitialized. + if let Some(Terminator { kind: TerminatorKind::Drop { place, .. }, .. }) = + body.stmt_at(loc).right() + { + if let LookupResult::Exact(mpi) = move_data.rev_lookup.find(place.as_ref()) { + on_all_children_bits(tcx, body, move_data, mpi, |mpi| { + callback(mpi, DropFlagState::Absent) + }) + } + } + debug!("drop_flag_effects: assignment for location({:?})", loc); for_location_inits(tcx, body, move_data, loc, |mpi| callback(mpi, DropFlagState::Present)); diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index f46fd118bde5d..f7f98d6e31cc6 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -375,7 +375,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { | TerminatorKind::Resume | TerminatorKind::Abort | TerminatorKind::GeneratorDrop - | TerminatorKind::Unreachable => {} + | TerminatorKind::Unreachable + | TerminatorKind::Drop { .. } => {} TerminatorKind::Assert { ref cond, .. } => { self.gather_operand(cond); @@ -390,10 +391,6 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { self.create_move_path(place); self.gather_init(place.as_ref(), InitKind::Deep); } - - TerminatorKind::Drop { place, target: _, unwind: _ } => { - self.gather_move(place); - } TerminatorKind::DropAndReplace { place, ref value, .. } => { self.create_move_path(place); self.gather_operand(value); diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 0522c657939f5..8c5cf0e557492 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -222,13 +222,13 @@ pub trait ValueAnalysis<'tcx> { self.super_terminator(terminator, state) } - fn super_terminator(&self, terminator: &Terminator<'tcx>, _state: &mut State) { + fn super_terminator(&self, terminator: &Terminator<'tcx>, state: &mut State) { match &terminator.kind { TerminatorKind::Call { .. } | TerminatorKind::InlineAsm { .. } => { // Effect is applied by `handle_call_return`. } - TerminatorKind::Drop { .. } => { - // We don't track dropped places. + TerminatorKind::Drop { place, .. } => { + state.flood_with(place.as_ref(), self.map(), Self::Value::bottom()); } TerminatorKind::DropAndReplace { .. } | TerminatorKind::Yield { .. } => { // They would have an effect, but are not allowed in this phase. diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 65f4956d23acd..c2ff8645635e0 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -18,6 +18,35 @@ use rustc_span::Span; use rustc_target::abi::VariantIdx; use std::fmt; +/// During MIR building, Drop and DropAndReplace terminators are inserted in every place where a drop may occur. +/// However, in this phase, the presence of these terminators does not guarantee that a destructor will run, +/// as the target of the drop may be uninitialized. +/// In general, the compiler cannot determine at compile time whether a destructor will run or not. +/// +/// At a high level, this pass refines Drop and DropAndReplace to only run the destructor if the +/// target is initialized. The way this is achievied is by inserting drop flags for every variable +/// that may be dropped, and then using those flags to determine whether a destructor should run. +/// This pass also removes DropAndReplace, replacing it with a Drop paired with an assign statement. +/// Once this is complete, Drop terminators in the MIR correspond to a call to the "drop glue" or +/// "drop shim" for the type of the dropped place. +/// +/// This pass relies on dropped places having an associated move path, which is then used to determine +/// the initialization status of the place and its descendants. +/// It's worth noting that a MIR containing a Drop without an associated move path is probably ill formed, +/// as it would allow running a destructor on a place behind a reference: +/// +/// ```text +// fn drop_term(t: &mut T) { +// mir!( +// { +// Drop(*t, exit) +// } +// exit = { +// Return() +// } +// ) +// } +/// ``` pub struct ElaborateDrops; impl<'tcx> MirPass<'tcx> for ElaborateDrops {