From 173dc0faa8c91dae93369fb7b35598fab1232490 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 31 Jul 2015 03:10:13 +0200 Subject: [PATCH 1/3] Reinitialize the dropflag hint in binding occurrences. Such bindings can occur in loops, and thus the binding can be executed after a previous move cleared the flag, thus necessitating the flag be reset to `DTOR_NEEDED_HINT`. Fix #27401. --- src/librustc_trans/trans/_match.rs | 49 ++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/src/librustc_trans/trans/_match.rs b/src/librustc_trans/trans/_match.rs index 7d7fd111fe94c..f99409b857fff 100644 --- a/src/librustc_trans/trans/_match.rs +++ b/src/librustc_trans/trans/_match.rs @@ -1023,6 +1023,25 @@ fn insert_lllocals<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, bcx } +// Sets each dropflag hint (if any) for bindings to `dropflag_hint_val`. +fn set_lllocals_hints<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, + bindings_map: &BindingsMap<'tcx>, + drop_flag_hint_value: u8) + -> Block<'blk, 'tcx> { + let lllocals = bcx.fcx.lllocals.borrow(); + for (&ident, &binding_info) in bindings_map { + let datum = lllocals.get(&binding_info.id).unwrap(); + if let Some(hint) = datum.kind.dropflag_hint(bcx) { + let hint_value = drop_flag_hint_value as usize; + debug!("set_lllocals_hints store hint_value={} for hint={:?} ident={}", + hint_value, hint, ident); + Store(bcx, C_u8(bcx.fcx.ccx, hint_value), hint.to_value().value()); + } + } + + bcx +} + fn compile_guard<'a, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>, guard_expr: &ast::Expr, data: &ArmData<'p, 'blk, 'tcx>, @@ -1635,6 +1654,7 @@ fn trans_match_inner<'blk, 'tcx>(scope_cx: Block<'blk, 'tcx>, // insert bindings into the lllocals map and add cleanups let cs = fcx.push_custom_cleanup_scope(); bcx = insert_lllocals(bcx, &arm_data.bindings_map, Some(cleanup::CustomScope(cs))); + bcx = set_lllocals_hints(bcx, &arm_data.bindings_map, adt::DTOR_NEEDED_HINT); bcx = expr::trans_into(bcx, &*arm_data.arm.body, dest); bcx = fcx.pop_and_trans_custom_cleanup_scope(bcx, cs); arm_cxs.push(bcx); @@ -1666,16 +1686,10 @@ pub fn store_local<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, bcx = mk_binding_alloca( bcx, p_id, path1.node.name, scope, (), "_match::store_local::create_dummy_locals", + // Dummy-locals start out uninitialized, so set their + // drop-flag hints (if any) to "moved." + adt::DTOR_MOVED_HINT, |(), bcx, Datum { val: llval, ty, kind }| { - // Dummy-locals start out uninitialized, so set their - // drop-flag hints (if any) to "moved." - if let Some(hint) = kind.dropflag_hint(bcx) { - let moved_hint = adt::DTOR_MOVED_HINT as usize; - debug!("store moved_hint={} for hint={:?}, uninitialized dummy", - moved_hint, hint); - Store(bcx, C_u8(bcx.fcx.ccx, moved_hint), hint.to_value().value()); - } - if kind.drop_flag_info.must_zero() { // if no drop-flag hint, or the hint requires // we maintain the embedded drop-flag, then @@ -1707,6 +1721,9 @@ pub fn store_local<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, return mk_binding_alloca( bcx, pat.id, ident.name, var_scope, (), "_match::store_local", + // Issue #27401: `let x = expr;` means drop + // flag hint needs to be (re-)initialized. + adt::DTOR_NEEDED_HINT, |(), bcx, Datum { val: v, .. }| expr::trans_into(bcx, &**init_expr, expr::SaveIn(v))); } @@ -1735,6 +1752,7 @@ fn mk_binding_alloca<'blk, 'tcx, A, F>(bcx: Block<'blk, 'tcx>, cleanup_scope: cleanup::ScopeId, arg: A, caller_name: &'static str, + drop_flag_hint_value: u8, populate: F) -> Block<'blk, 'tcx> where F: FnOnce(A, Block<'blk, 'tcx>, Datum<'tcx, Lvalue>) -> Block<'blk, 'tcx>, @@ -1749,6 +1767,15 @@ fn mk_binding_alloca<'blk, 'tcx, A, F>(bcx: Block<'blk, 'tcx>, // Subtle: be sure that we *populate* the memory *before* // we schedule the cleanup. let bcx = populate(arg, bcx, datum); + + // Set the drop-flag hint, if any, to the value provided by context + if let Some(hint) = datum.kind.dropflag_hint(bcx) { + let hint_value = drop_flag_hint_value as usize; + debug!("mk_binding_alloca store hint_value={} for hint={:?}, name={}", + hint_value, hint, name); + Store(bcx, C_u8(bcx.fcx.ccx, hint_value), hint.to_value().value()); + } + bcx.fcx.schedule_lifetime_end(cleanup_scope, llval); bcx.fcx.schedule_drop_mem(cleanup_scope, llval, var_ty, lvalue.dropflag_hint(bcx)); @@ -1799,6 +1826,10 @@ pub fn bind_irrefutable_pat<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, bcx = mk_binding_alloca( bcx, pat.id, path1.node.name, cleanup_scope, (), "_match::bind_irrefutable_pat", + // Issue #27401: `match ... { PAT[x] => ... }` + // means drop flag hint for `x` needs to be + // (re-)initialized. + adt::DTOR_NEEDED_HINT, |(), bcx, Datum { val: llval, ty, kind: _ }| { match pat_binding_mode { ast::BindByValue(_) => { From 0f314f9f082a26e2ed3ba98111456defbf2dee49 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 31 Jul 2015 03:10:29 +0200 Subject: [PATCH 2/3] Regression tests for Issue #27401. --- src/test/run-pass/issue-27401-let-init.rs | 39 ++++++++++++++++++ src/test/run-pass/issue-27401-match-bind.rs | 45 +++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 src/test/run-pass/issue-27401-let-init.rs create mode 100644 src/test/run-pass/issue-27401-match-bind.rs diff --git a/src/test/run-pass/issue-27401-let-init.rs b/src/test/run-pass/issue-27401-let-init.rs new file mode 100644 index 0000000000000..dd549b68f6567 --- /dev/null +++ b/src/test/run-pass/issue-27401-let-init.rs @@ -0,0 +1,39 @@ +// Copyright 2015 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. + +// The stack-local drop flag associated with binding `let a = ...;` +// (that occurred in a loop) were failing to (re-)initialize the drop +// flag after it had been set to "moved" during an earlier iteration +// of the loop. +// +// This is a regression test to ensure we don't let that behavior +// creep back in. +// +// See also issue-27401-match-bind.rs + +struct A<'a>(&'a mut i32); + +impl<'a> Drop for A<'a> { + fn drop(&mut self) { + *self.0 += 1; + } +} + +fn main() { + let mut cnt = 0; + for i in 0..2 { + let a = A(&mut cnt); + if i == 1 { + break + } + drop(a); + } + assert_eq!(cnt, 2); +} diff --git a/src/test/run-pass/issue-27401-match-bind.rs b/src/test/run-pass/issue-27401-match-bind.rs new file mode 100644 index 0000000000000..601bd09da9f5c --- /dev/null +++ b/src/test/run-pass/issue-27401-match-bind.rs @@ -0,0 +1,45 @@ +// Copyright 2015 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. + +// The stack-local drop flag for binding in match-arm `(_, a) => ...` +// (that occurred in a loop) were failing to (re-)initialize the drop +// flag after it had been set to "moved" during an earlier iteration +// of the loop. +// +// This is a regression test to ensure we don't let that behavior +// creep back in. +// +// See also issue-27401-let-init.rs + +use std::cell::Cell; + +struct A<'a>(&'a Cell); + +impl<'a> Drop for A<'a> { + fn drop(&mut self) { + let old_val = self.0.get(); + self.0.set(old_val + 1); + } +} + +fn main() { + let cnt = Cell::new(0); + for i in 0..2 { + match (A(&cnt), A(&cnt)) { + (_, aaah) => { + if i == 1 { + break + } + drop(aaah); + } + } + } + assert_eq!(cnt.get(), 4); +} From c681d307608136dd0631431936f04c39bb3dea71 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 31 Jul 2015 04:28:41 +0200 Subject: [PATCH 3/3] placate `make tidy`. --- src/librustc_trans/trans/_match.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_trans/trans/_match.rs b/src/librustc_trans/trans/_match.rs index f99409b857fff..a8f19032f6558 100644 --- a/src/librustc_trans/trans/_match.rs +++ b/src/librustc_trans/trans/_match.rs @@ -1031,7 +1031,7 @@ fn set_lllocals_hints<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let lllocals = bcx.fcx.lllocals.borrow(); for (&ident, &binding_info) in bindings_map { let datum = lllocals.get(&binding_info.id).unwrap(); - if let Some(hint) = datum.kind.dropflag_hint(bcx) { + if let Some(hint) = datum.kind.dropflag_hint(bcx) { let hint_value = drop_flag_hint_value as usize; debug!("set_lllocals_hints store hint_value={} for hint={:?} ident={}", hint_value, hint, ident);