Skip to content

Commit

Permalink
Auto merge of #125923 - matthewjasper:no-return-leak, r=<try>
Browse files Browse the repository at this point in the history
Fix leaks from panics in destructors

Resurrects #78373.

This avoids the problem with #80949 by not unscheduling drops of function arguments until after the call (so they still get a drop terminator on the function unwind path).

Closes #47949

r? `@lcnr`
  • Loading branch information
bors committed Jul 15, 2024
2 parents 0da95bd + 3e0db6a commit b905bf4
Show file tree
Hide file tree
Showing 24 changed files with 778 additions and 492 deletions.
24 changes: 20 additions & 4 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::build::matches::{DeclareLetBindings, EmitStorageLive, ScheduleDrops};
use crate::build::scope::DropKind;
use crate::build::ForGuard::OutsideGuard;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use rustc_middle::middle::region::Scope;
Expand All @@ -12,6 +13,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub(crate) fn ast_block(
&mut self,
destination: Place<'tcx>,
scope: Option<Scope>,
block: BasicBlock,
ast_block: BlockId,
source_info: SourceInfo,
Expand All @@ -20,18 +22,27 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.thir[ast_block];
self.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
if targeted_by_break {
this.in_breakable_scope(None, destination, span, |this| {
Some(this.ast_block_stmts(destination, block, span, stmts, expr, region_scope))
this.in_breakable_scope(None, destination, scope, span, |this| {
Some(this.ast_block_stmts(
destination,
scope,
block,
span,
stmts,
expr,
region_scope,
))
})
} else {
this.ast_block_stmts(destination, block, span, stmts, expr, region_scope)
this.ast_block_stmts(destination, scope, block, span, stmts, expr, region_scope)
}
})
}

fn ast_block_stmts(
&mut self,
destination: Place<'tcx>,
scope: Option<Scope>,
mut block: BasicBlock,
span: Span,
stmts: &[StmtId],
Expand Down Expand Up @@ -169,6 +180,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
unpack!(
failure_block = this.ast_block(
dummy_place,
None,
failure_entry,
*else_block,
this.source_info(else_block_span),
Expand Down Expand Up @@ -333,7 +345,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.block_context
.push(BlockFrame::TailExpr { tail_result_is_ignored, span: expr.span });

unpack!(block = this.expr_into_dest(destination, block, expr_id));
unpack!(block = this.expr_into_dest(destination, scope, block, expr_id));
let popped = this.block_context.pop();

assert!(popped.is_some_and(|bf| bf.is_tail_expr()));
Expand All @@ -350,6 +362,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// We only want to assign an implicit `()` as the return value of the block if the
// block does not diverge. (Otherwise, we may try to assign a unit to a `!`-type.)
this.cfg.push_assign_unit(block, source_info, destination, this.tcx);
} else if let Some(destination_local) = destination.as_local()
&& let Some(scope) = scope
{
this.schedule_drop(span, scope, destination_local, DropKind::Value);
}
}
// Finally, we pop all the let scopes before exiting out from the scope of block
Expand Down
15 changes: 12 additions & 3 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ use rustc_middle::ty::{self, Ty, UpvarArgs};
use rustc_span::{Span, DUMMY_SP};
use tracing::debug;

use std::slice;

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Returns an rvalue suitable for use until the end of the current
/// scope expression.
Expand Down Expand Up @@ -184,15 +186,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let box_ = Rvalue::ShallowInitBox(Operand::Move(storage), value_ty);
this.cfg.push_assign(block, source_info, Place::from(result), box_);

// initialize the box contents:
// Initialize the box contents. No scope is needed since the
// `Box` is already scheduled to be dropped.
unpack!(
block = this.expr_into_dest(
this.tcx.mk_place_deref(Place::from(result)),
None,
block,
value,
)
);
block.and(Rvalue::Use(Operand::Move(Place::from(result))))
let result_operand = Operand::Move(Place::from(result));
this.record_operands_moved(slice::from_ref(&result_operand));
block.and(Rvalue::Use(result_operand))
}
ExprKind::Cast { source } => {
let source_expr = &this.thir[source];
Expand Down Expand Up @@ -360,6 +366,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
})
.collect();

this.record_operands_moved(&fields.raw);
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(el_ty)), fields))
}
ExprKind::Tuple { ref fields } => {
Expand All @@ -381,6 +388,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
})
.collect();

this.record_operands_moved(&fields.raw);
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields))
}
ExprKind::Closure(box ClosureExpr {
Expand Down Expand Up @@ -483,6 +491,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Box::new(AggregateKind::CoroutineClosure(closure_id.to_def_id(), args))
}
};
this.record_operands_moved(&operands.raw);
block.and(Rvalue::Aggregate(result, operands))
}
ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => {
Expand Down Expand Up @@ -738,7 +747,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.diverge_from(block);
block = success;
}
this.record_operands_moved(&[Spanned { node: value_operand, span: DUMMY_SP }]);
this.record_operands_moved(&[value_operand]);
}
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(elem_ty)), IndexVec::new()))
}
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_mir_build/src/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

unpack!(block = this.expr_into_dest(temp_place, block, expr_id));

if let Some(temp_lifetime) = temp_lifetime {
this.schedule_drop(expr_span, temp_lifetime, temp, DropKind::Value);
}
unpack!(block = this.expr_into_dest(temp_place, temp_lifetime, block, expr_id));

block.and(temp)
}
Expand Down
Loading

0 comments on commit b905bf4

Please sign in to comment.