From 99aa48de2903f2b2a1fdc056f8791b21672f3346 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 5 Jan 2017 01:19:54 +0200 Subject: [PATCH] fix promotion of MIR terminators promotion of MIR terminators used to try to promote the destination it is trying to promote, leading to stack overflow. Fixes #37991. --- src/librustc_mir/transform/promote_consts.rs | 144 +++++++++---------- src/librustc_mir/transform/qualify_consts.rs | 2 +- src/test/run-pass/issue-37991.rs | 20 +++ 3 files changed, 87 insertions(+), 79 deletions(-) create mode 100644 src/test/run-pass/issue-37991.rs diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 41698574e0f1f..2a4b2b515cc68 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -66,6 +66,7 @@ impl TempState { /// A "root candidate" for promotion, which will become the /// returned value in a promoted MIR, unless it's a subset /// of a larger candidate. +#[derive(Debug)] pub enum Candidate { /// Borrow of a constant temporary. Ref(Location), @@ -190,15 +191,12 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { /// promoted MIR, recursing through temps. fn promote_temp(&mut self, temp: Local) -> Local { let old_keep_original = self.keep_original; - let (bb, stmt_idx) = match self.temps[temp] { - TempState::Defined { - location: Location { block, statement_index }, - uses - } if uses > 0 => { + let loc = match self.temps[temp] { + TempState::Defined { location, uses } if uses > 0 => { if uses > 1 { self.keep_original = true; } - (block, statement_index) + location } state => { span_bug!(self.promoted.span, "{:?} not promotable: {:?}", @@ -209,91 +207,80 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { self.temps[temp] = TempState::PromotedOut; } - let no_stmts = self.source[bb].statements.len(); + let no_stmts = self.source[loc.block].statements.len(); + let new_temp = self.promoted.local_decls.push( + LocalDecl::new_temp(self.source.local_decls[temp].ty)); + + debug!("promote({:?} @ {:?}/{:?}, {:?})", + temp, loc, no_stmts, self.keep_original); // First, take the Rvalue or Call out of the source MIR, // or duplicate it, depending on keep_original. - let (mut rvalue, mut call) = (None, None); - let source_info = if stmt_idx < no_stmts { - let statement = &mut self.source[bb].statements[stmt_idx]; - let rhs = match statement.kind { - StatementKind::Assign(_, ref mut rhs) => rhs, - _ => { - span_bug!(statement.source_info.span, "{:?} is not an assignment", - statement); - } + if loc.statement_index < no_stmts { + let (mut rvalue, source_info) = { + let statement = &mut self.source[loc.block].statements[loc.statement_index]; + let rhs = match statement.kind { + StatementKind::Assign(_, ref mut rhs) => rhs, + _ => { + span_bug!(statement.source_info.span, "{:?} is not an assignment", + statement); + } + }; + + (if self.keep_original { + rhs.clone() + } else { + let unit = Rvalue::Aggregate(AggregateKind::Tuple, vec![]); + mem::replace(rhs, unit) + }, statement.source_info) }; - if self.keep_original { - rvalue = Some(rhs.clone()); - } else { - let unit = Rvalue::Aggregate(AggregateKind::Tuple, vec![]); - rvalue = Some(mem::replace(rhs, unit)); - } - statement.source_info - } else if self.keep_original { - let terminator = self.source[bb].terminator().clone(); - call = Some(terminator.kind); - terminator.source_info + + self.visit_rvalue(&mut rvalue, loc); + self.assign(new_temp, rvalue, source_info.span); } else { - let terminator = self.source[bb].terminator_mut(); - let target = match terminator.kind { - TerminatorKind::Call { - destination: ref mut dest @ Some(_), - ref mut cleanup, .. - } => { - // No cleanup necessary. - cleanup.take(); - - // We'll put a new destination in later. - dest.take().unwrap().1 - } - ref kind => { - span_bug!(terminator.source_info.span, "{:?} not promotable", kind); + let mut terminator = if self.keep_original { + self.source[loc.block].terminator().clone() + } else { + let terminator = self.source[loc.block].terminator_mut(); + let target = match terminator.kind { + TerminatorKind::Call { destination: Some((_, target)), .. } => target, + ref kind => { + span_bug!(terminator.source_info.span, "{:?} not promotable", kind); + } + }; + Terminator { + source_info: terminator.source_info, + kind: mem::replace(&mut terminator.kind, TerminatorKind::Goto { + target: target + }) } }; - call = Some(mem::replace(&mut terminator.kind, TerminatorKind::Goto { - target: target - })); - terminator.source_info - }; - // Then, recurse for components in the Rvalue or Call. - if stmt_idx < no_stmts { - self.visit_rvalue(rvalue.as_mut().unwrap(), Location { - block: bb, - statement_index: stmt_idx - }); - } else { - self.visit_terminator_kind(bb, call.as_mut().unwrap(), Location { - block: bb, - statement_index: no_stmts - }); - } - - let new_temp = self.promoted.local_decls.push( - LocalDecl::new_temp(self.source.local_decls[temp].ty)); - - // Inject the Rvalue or Call into the promoted MIR. - if stmt_idx < no_stmts { - self.assign(new_temp, rvalue.unwrap(), source_info.span); - } else { let last = self.promoted.basic_blocks().last().unwrap(); let new_target = self.new_block(); - let mut call = call.unwrap(); - match call { - TerminatorKind::Call { ref mut destination, ..} => { - *destination = Some((Lvalue::Local(new_temp), new_target)); + + terminator.kind = match terminator.kind { + TerminatorKind::Call { mut func, mut args, .. } => { + self.visit_operand(&mut func, loc); + for arg in &mut args { + self.visit_operand(arg, loc); + } + TerminatorKind::Call { + func: func, + args: args, + cleanup: None, + destination: Some((Lvalue::Local(new_temp), new_target)) + } } - _ => bug!() - } - let terminator = self.promoted[last].terminator_mut(); - terminator.source_info.span = source_info.span; - terminator.kind = call; - } + ref kind => { + span_bug!(terminator.source_info.span, "{:?} not promotable", kind); + } + }; - // Restore the old duplication state. - self.keep_original = old_keep_original; + *self.promoted[last].terminator_mut() = terminator; + }; + self.keep_original = old_keep_original; new_temp } @@ -355,6 +342,7 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, mut temps: IndexVec, candidates: Vec) { // Visit candidates in reverse, in case they're nested. + debug!("promote_candidates({:?})", candidates); for candidate in candidates.into_iter().rev() { let (span, ty) = match candidate { Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => { diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 893478a933182..d144651fb7db6 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -993,9 +993,9 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants { Entry::Vacant(entry) => { // Guard against `const` recursion. entry.insert(Qualif::RECURSIVE); + Mode::Const } } - Mode::Const } MirSource::Static(_, hir::MutImmutable) => Mode::Static, MirSource::Static(_, hir::MutMutable) => Mode::StaticMut, diff --git a/src/test/run-pass/issue-37991.rs b/src/test/run-pass/issue-37991.rs new file mode 100644 index 0000000000000..cacc653871ad5 --- /dev/null +++ b/src/test/run-pass/issue-37991.rs @@ -0,0 +1,20 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(const_fn)] + +const fn foo() -> i64 { + 3 +} + +fn main() { + let val = &(foo() % 2); + assert_eq!(*val, 1); +}