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 Jun 4, 2024
2 parents 85f90a4 + 28ac9be commit 1c34390
Show file tree
Hide file tree
Showing 21 changed files with 711 additions and 486 deletions.
19 changes: 15 additions & 4 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,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 @@ -19,18 +20,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 @@ -168,6 +178,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 @@ -321,7 +332,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 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 @@ -183,15 +185,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 @@ -359,6 +365,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 @@ -380,6 +387,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 @@ -482,6 +490,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 @@ -737,7 +746,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 1c34390

Please sign in to comment.