Skip to content

Fix leaks from panics in drops #72152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/librustc_mir_build/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ use crate::build::ForGuard::OutsideGuard;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::hair::*;
use rustc_hir as hir;
use rustc_middle::middle::region;
use rustc_middle::mir::*;
use rustc_span::Span;

impl<'a, 'tcx> Builder<'a, 'tcx> {
crate fn ast_block(
&mut self,
destination: Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
ast_block: &'tcx hir::Block<'tcx>,
source_info: SourceInfo,
Expand All @@ -26,9 +28,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.in_opt_scope(opt_destruction_scope.map(|de| (de, source_info)), move |this| {
this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
if targeted_by_break {
this.in_breakable_scope(None, destination, span, |this| {
this.in_breakable_scope(None, destination, scope, span, |this| {
Some(this.ast_block_stmts(
destination,
scope,
block,
span,
stmts,
Expand All @@ -37,7 +40,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
))
})
} else {
this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
this.ast_block_stmts(destination, scope, block, span, stmts, expr, safety_mode)
}
})
})
Expand All @@ -46,6 +49,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn ast_block_stmts(
&mut self,
destination: Place<'tcx>,
scope: Option<region::Scope>,
mut block: BasicBlock,
span: Span,
stmts: Vec<StmtRef<'tcx>>,
Expand Down Expand Up @@ -181,7 +185,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
};
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored, span });

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

assert!(popped.map_or(false, |bf| bf.is_tail_expr()));
Expand Down
20 changes: 16 additions & 4 deletions src/librustc_mir_build/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use rustc_middle::mir::*;
use rustc_middle::ty::{self, Ty, UpvarSubsts};
use rustc_span::Span;

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 @@ -111,12 +113,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let box_ = Rvalue::NullaryOp(NullOp::Box, 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.into(this.hir.tcx().mk_place_deref(Place::from(result)), block, value)
block = this.into(
this.hir.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 = unpack!(block = this.as_operand(block, scope, source));
Expand Down Expand Up @@ -160,6 +169,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.map(|f| unpack!(block = this.as_operand(block, scope, f)))
.collect();

this.record_operands_moved(&fields);
block.and(Rvalue::Aggregate(box AggregateKind::Array(el_ty), fields))
}
ExprKind::Tuple { fields } => {
Expand All @@ -170,6 +180,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.map(|f| unpack!(block = this.as_operand(block, scope, f)))
.collect();

this.record_operands_moved(&fields);
block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields))
}
ExprKind::Closure { closure_id, substs, upvars, movability } => {
Expand Down Expand Up @@ -221,6 +232,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
UpvarSubsts::Closure(substs) => box AggregateKind::Closure(closure_id, substs),
};
this.record_operands_moved(&operands);
block.and(Rvalue::Aggregate(result, operands))
}
ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => {
Expand Down
6 changes: 1 addition & 5 deletions src/librustc_mir_build/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

unpack!(block = this.into(temp_place, block, expr));

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

block.and(temp)
}
Expand Down
108 changes: 75 additions & 33 deletions src/librustc_mir_build/build/expr/into.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,35 @@
//! See docs in build/expr/mod.rs

use crate::build::expr::category::{Category, RvalueFunc};
use crate::build::scope::DropKind;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::hair::*;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir as hir;
use rustc_middle::middle::region;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation};
use rustc_span::symbol::sym;

use rustc_target::spec::abi::Abi;

use std::slice;

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Compile `expr`, storing the result into `destination`, which
/// is assumed to be uninitialized.
/// If a `drop_scope` is provided, `destination` is scheduled to be dropped
/// in `scope` once it has been initialized.
crate fn into_expr(
&mut self,
destination: Place<'tcx>,
scope: Option<region::Scope>,
mut block: BasicBlock,
expr: Expr<'tcx>,
) -> BlockAnd<()> {
debug!("into_expr(destination={:?}, block={:?}, expr={:?})", destination, block, expr);
debug!(
"into_expr(destination={:?}, scope={:?}, block={:?}, expr={:?})",
destination, scope, block, expr
);

// since we frequently have to reference `self` from within a
// closure, where `self` would be shadowed, it's easier to
Expand All @@ -35,20 +44,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
_ => false,
};

let schedule_drop = move |this: &mut Self| {
if let Some(drop_scope) = scope {
let local =
destination.as_local().expect("cannot schedule drop of non-Local place");
this.schedule_drop(expr_span, drop_scope, local, DropKind::Value);
}
};

if !expr_is_block_or_scope {
this.block_context.push(BlockFrame::SubExpr);
}

let block_and = match expr.kind {
ExprKind::Scope { region_scope, lint_level, value } => {
let region_scope = (region_scope, source_info);
this.in_scope(region_scope, lint_level, |this| this.into(destination, block, value))
this.in_scope(region_scope, lint_level, |this| {
this.into(destination, scope, block, value)
})
}
ExprKind::Block { body: ast_block } => {
this.ast_block(destination, block, ast_block, source_info)
this.ast_block(destination, scope, block, ast_block, source_info)
}
ExprKind::Match { scrutinee, arms } => {
this.match_expr(destination, expr_span, block, scrutinee, arms)
this.match_expr(destination, scope, expr_span, block, scrutinee, arms)
}
ExprKind::NeverToAny { source } => {
let source = this.hir.mirror(source);
Expand All @@ -63,6 +82,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// This is an optimization. If the expression was a call then we already have an
// unreachable block. Don't bother to terminate it and create a new one.
schedule_drop(this);
if is_call {
block.unit()
} else {
Expand Down Expand Up @@ -138,26 +158,35 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Start the loop.
this.cfg.goto(block, source_info, loop_block);

this.in_breakable_scope(Some(loop_block), destination, expr_span, move |this| {
// conduct the test, if necessary
let body_block = this.cfg.start_new_block();
this.cfg.terminate(
loop_block,
source_info,
TerminatorKind::FalseUnwind { real_target: body_block, unwind: None },
);
this.diverge_from(loop_block);

// The “return” value of the loop body must always be an unit. We therefore
// introduce a unit temporary as the destination for the loop body.
let tmp = this.get_unit_temp();
// Execute the body, branching back to the test.
let body_block_end = unpack!(this.into(tmp, body_block, body));
this.cfg.goto(body_block_end, source_info, loop_block);

// Loops are only exited by `break` expressions.
None
})
this.in_breakable_scope(
Some(loop_block),
destination,
scope,
expr_span,
move |this| {
// conduct the test, if necessary
let body_block = this.cfg.start_new_block();
this.cfg.terminate(
loop_block,
source_info,
TerminatorKind::FalseUnwind { real_target: body_block, unwind: None },
);
this.diverge_from(loop_block);

// The “return” value of the loop body must always be an unit. We therefore
// introduce a unit temporary as the destination for the loop body.
let tmp = this.get_unit_temp();
// Execute the body, branching back to the test.
// We don't need to provide a drop scope because `tmp`
// has type `()`.
let body_block_end = unpack!(this.into(tmp, None, body_block, body));
this.cfg.goto(body_block_end, source_info, loop_block);
schedule_drop(this);

// Loops are only exited by `break` expressions.
None
},
)
}
ExprKind::Call { ty, fun, args, from_hir_call } => {
let intrinsic = match ty.kind {
Expand Down Expand Up @@ -185,13 +214,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let ptr_ty = ptr.ty;
// Create an *internal* temp for the pointer, so that unsafety
// checking won't complain about the raw pointer assignment.
let ptr_temp = this.local_decls.push(LocalDecl::with_source_info(
ptr_ty,
source_info,
).internal());
let ptr_temp = this
.local_decls
.push(LocalDecl::with_source_info(ptr_ty, source_info).internal());
let ptr_temp = Place::from(ptr_temp);
let block = unpack!(this.into(ptr_temp, block, ptr));
this.into(this.hir.tcx().mk_place_deref(ptr_temp), block, val)
// No need for a scope, ptr_temp doesn't need drop
let block = unpack!(this.into(ptr_temp, None, block, ptr));
// Maybe we should provide a scope here so that
// `move_val_init` wouldn't leak on panic even with an
// arbitrary `val` expression, but `schedule_drop`,
// borrowck and drop elaboration all prevent us from
// dropping `ptr_temp.deref()`.
this.into(this.hir.tcx().mk_place_deref(ptr_temp), None, block, val)
} else {
let args: Vec<_> = args
.into_iter()
Expand Down Expand Up @@ -221,10 +255,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
},
);
this.diverge_from(block);
schedule_drop(this);
success.unit()
}
}
ExprKind::Use { source } => this.into(destination, block, source),
ExprKind::Use { source } => this.into(destination, scope, block, source),
ExprKind::Borrow { arg, borrow_kind } => {
// We don't do this in `as_rvalue` because we use `as_place`
// for borrow expressions, so we cannot create an `RValue` that
Expand Down Expand Up @@ -266,7 +301,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

let field_names = this.hir.all_fields(adt_def, variant_index);

let fields = if let Some(FruInfo { base, field_types }) = base {
let fields: Vec<_> = if let Some(FruInfo { base, field_types }) = base {
let base = unpack!(block = this.as_place(block, base));

// MIR does not natively support FRU, so for each
Expand Down Expand Up @@ -301,12 +336,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
user_ty,
active_field_index,
);
this.record_operands_moved(&fields);
this.cfg.push_assign(
block,
source_info,
destination,
Rvalue::Aggregate(adt, fields),
);
schedule_drop(this);
block.unit()
}

Expand All @@ -332,6 +369,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let place = unpack!(block = this.as_place(block, expr));
let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
this.cfg.push_assign(block, source_info, destination, rvalue);
schedule_drop(this);
block.unit()
}
ExprKind::Index { .. } | ExprKind::Deref { .. } | ExprKind::Field { .. } => {
Expand All @@ -349,19 +387,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let place = unpack!(block = this.as_place(block, expr));
let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
this.cfg.push_assign(block, source_info, destination, rvalue);
schedule_drop(this);
block.unit()
}

ExprKind::Yield { value } => {
let scope = this.local_scope();
let value = unpack!(block = this.as_operand(block, scope, value));
let resume = this.cfg.start_new_block();
this.record_operands_moved(slice::from_ref(&value));
this.cfg.terminate(
block,
source_info,
TerminatorKind::Yield { value, resume, resume_arg: destination, drop: None },
);
this.generator_drop_cleanup(block);
schedule_drop(this);
resume.unit()
}

Expand Down Expand Up @@ -391,6 +432,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

let rvalue = unpack!(block = this.as_local_rvalue(block, expr));
this.cfg.push_assign(block, source_info, destination, rvalue);
schedule_drop(this);
block.unit()
}
};
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_mir_build/build/expr/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::hair::*;
use rustc_middle::middle::region;
use rustc_middle::mir::*;
use std::slice;

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Builds a block of MIR statements to evaluate the HAIR `expr`.
Expand Down Expand Up @@ -46,6 +47,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if this.hir.needs_drop(lhs.ty) {
let rhs = unpack!(block = this.as_local_operand(block, rhs));
let lhs = unpack!(block = this.as_place(block, lhs));
this.record_operands_moved(slice::from_ref(&rhs));
unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs));
} else {
let rhs = unpack!(block = this.as_local_rvalue(block, rhs));
Expand Down Expand Up @@ -150,8 +152,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
break;
}
}
this.block_context
.push(BlockFrame::TailExpr { tail_result_is_ignored: true, span: expr.span });
this.block_context.push(BlockFrame::TailExpr {
tail_result_is_ignored: true,
span: expr.span,
});
return Some(expr.span);
}
}
Expand Down
Loading