Skip to content

Commit

Permalink
Break out of the correct number of scopes in loops
Browse files Browse the repository at this point in the history
We were incorrectly breaking out of one too many drop scopes when
generating MIR for loops and breakable blocks, resulting in use after
free and associated borrow checker warnings.

This wasn't noticed because the scope that we're breaking out of twice
is only used for temporaries that are created for adjustments applied to
the loop. Since loops generally propagate coercions to the `break`
expressions, the only case we see this is when the type of the loop is a
smart pointer to a trait object.
  • Loading branch information
matthewjasper committed Jul 4, 2019
1 parent 848e0a2 commit 1b7ffe5
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 21 deletions.
12 changes: 7 additions & 5 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
};

// Step 5. Create everything else: the guards and the arms.
let match_scope = self.scopes.topmost();

let arm_end_blocks: Vec<_> = arm_candidates.into_iter().map(|(arm, mut candidates)| {
let arm_source_info = self.source_info(arm.span);
let region_scope = (arm.scope, arm_source_info);
self.in_scope(region_scope, arm.lint_level, |this| {
let arm_scope = (arm.scope, arm_source_info);
self.in_scope(arm_scope, arm.lint_level, |this| {
let body = this.hir.mirror(arm.body.clone());
let scope = this.declare_bindings(
None,
Expand All @@ -248,7 +250,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
arm.guard.clone(),
&fake_borrow_temps,
scrutinee_span,
region_scope,
match_scope,
);
} else {
arm_block = this.cfg.start_new_block();
Expand All @@ -259,7 +261,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
arm.guard.clone(),
&fake_borrow_temps,
scrutinee_span,
region_scope,
match_scope,
);
this.cfg.terminate(
binding_end,
Expand Down Expand Up @@ -1339,7 +1341,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
guard: Option<Guard<'tcx>>,
fake_borrows: &Vec<(&Place<'tcx>, Local)>,
scrutinee_span: Span,
region_scope: (region::Scope, SourceInfo),
region_scope: region::Scope,
) -> BasicBlock {
debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate);

Expand Down
21 changes: 13 additions & 8 deletions src/librustc_mir/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,18 @@ where
}

let arg_scope_s = (arg_scope, source_info);
unpack!(block = builder.in_scope(arg_scope_s, LintLevel::Inherited, |builder| {
builder.args_and_body(block, &arguments, arg_scope, &body.value)
}));
// `return_block` is called when we evaluate a `return` expression, so
// we just use `START_BLOCK` here.
unpack!(block = builder.in_breakable_scope(
None,
START_BLOCK,
Place::RETURN_PLACE,
|builder| {
builder.in_scope(arg_scope_s, LintLevel::Inherited, |builder| {
builder.args_and_body(block, &arguments, arg_scope, &body.value)
})
},
));
// Attribute epilogue to function's closing brace
let fn_end = span.shrink_to_hi();
let source_info = builder.source_info(fn_end);
Expand Down Expand Up @@ -860,11 +869,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}

let body = self.hir.mirror(ast_body);
// `return_block` is called when we evaluate a `return` expression, so
// we just use `START_BLOCK` here.
self.in_breakable_scope(None, START_BLOCK, Place::RETURN_PLACE, |this| {
this.into(&Place::RETURN_PLACE, block, body)
})
self.into(&Place::RETURN_PLACE, block, body)
}

fn get_unit_temp(&mut self) -> Place<'tcx> {
Expand Down
16 changes: 8 additions & 8 deletions src/librustc_mir/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,9 @@ impl<'tcx> Scopes<'tcx> {
}
}

fn num_scopes_to(&self, region_scope: (region::Scope, SourceInfo), span: Span) -> usize {
let scope_count = 1 + self.scopes.iter().rev()
.position(|scope| scope.region_scope == region_scope.0)
fn num_scopes_above(&self, region_scope: region::Scope, span: Span) -> usize {
let scope_count = self.scopes.iter().rev()
.position(|scope| scope.region_scope == region_scope)
.unwrap_or_else(|| {
span_bug!(span, "region_scope {:?} does not enclose", region_scope)
});
Expand All @@ -354,7 +354,7 @@ impl<'tcx> Scopes<'tcx> {

/// Returns the topmost active scope, which is known to be alive until
/// the next scope expression.
fn topmost(&self) -> region::Scope {
pub(super) fn topmost(&self) -> region::Scope {
self.scopes.last().expect("topmost_scope: no scopes present").region_scope
}

Expand Down Expand Up @@ -514,7 +514,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} else {
assert!(value.is_none(), "`return` and `break` should have a destination");
}
self.exit_scope(source_info.span, (region_scope, source_info), block, target_block);
self.exit_scope(source_info.span, region_scope, block, target_block);
self.cfg.start_new_block().unit()
}

Expand All @@ -523,12 +523,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// needed. See module comment for details.
pub fn exit_scope(&mut self,
span: Span,
region_scope: (region::Scope, SourceInfo),
region_scope: region::Scope,
mut block: BasicBlock,
target: BasicBlock) {
debug!("exit_scope(region_scope={:?}, block={:?}, target={:?})",
region_scope, block, target);
let scope_count = self.scopes.num_scopes_to(region_scope, span);
let scope_count = self.scopes.num_scopes_above(region_scope, span);

// If we are emitting a `drop` statement, we need to have the cached
// diverge cleanup pads ready in case that drop panics.
Expand All @@ -545,7 +545,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
continue;
}
let source_info = scope.source_info(span);
block = match scope.cached_exits.entry((target, region_scope.0)) {
block = match scope.cached_exits.entry((target, region_scope)) {
Entry::Occupied(e) => {
self.cfg.terminate(block, source_info,
TerminatorKind::Goto { target: *e.get() });
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/async-await/await-unsize.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Regression test for #62312

// check-pass
// edition:2018

#![feature(async_await)]

async fn make_boxed_object() -> Box<dyn Send> {
Box::new(()) as _
}

async fn await_object() {
let _ = make_boxed_object().await;
}

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/loops/loop-break-unsize.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Regression test for #62312
// check-pass

fn main() {
let _ = loop {
break Box::new(()) as Box<dyn Send>;
};
}

0 comments on commit 1b7ffe5

Please sign in to comment.