From 1b7ffe5300d65da7189db3f62d1cb236746e5665 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 4 Jul 2019 21:53:46 +0100 Subject: [PATCH] Break out of the correct number of scopes in loops 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. --- src/librustc_mir/build/matches/mod.rs | 12 +++++++----- src/librustc_mir/build/mod.rs | 21 +++++++++++++-------- src/librustc_mir/build/scope.rs | 16 ++++++++-------- src/test/ui/async-await/await-unsize.rs | 16 ++++++++++++++++ src/test/ui/loops/loop-break-unsize.rs | 8 ++++++++ 5 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/async-await/await-unsize.rs create mode 100644 src/test/ui/loops/loop-break-unsize.rs diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index f831f5105a468..75c64bb2644c1 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -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, @@ -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(); @@ -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, @@ -1339,7 +1341,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { guard: Option>, 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); diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index ad970de466cfd..a5f9d1f99ccd0 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -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); @@ -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> { diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 1b5fa1c9770f1..a74d5d7ab2de3 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -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) }); @@ -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 } @@ -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() } @@ -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. @@ -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() }); diff --git a/src/test/ui/async-await/await-unsize.rs b/src/test/ui/async-await/await-unsize.rs new file mode 100644 index 0000000000000..d5e21257f4f7d --- /dev/null +++ b/src/test/ui/async-await/await-unsize.rs @@ -0,0 +1,16 @@ +// Regression test for #62312 + +// check-pass +// edition:2018 + +#![feature(async_await)] + +async fn make_boxed_object() -> Box { + Box::new(()) as _ +} + +async fn await_object() { + let _ = make_boxed_object().await; +} + +fn main() {} diff --git a/src/test/ui/loops/loop-break-unsize.rs b/src/test/ui/loops/loop-break-unsize.rs new file mode 100644 index 0000000000000..974c63cea85e4 --- /dev/null +++ b/src/test/ui/loops/loop-break-unsize.rs @@ -0,0 +1,8 @@ +// Regression test for #62312 +// check-pass + +fn main() { + let _ = loop { + break Box::new(()) as Box; + }; +}