From 326443105b897171424c6efde15198cd7a25bc61 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 27 Jan 2015 18:12:34 +0100 Subject: [PATCH 1/3] trans: When coercing to `Box` or `Box<[T]>`, leave datum in its original L-/R-value state. This fixes a subtle issue where temporaries were being allocated (but not necessarily initialized) to the (parent) terminating scope of a match expression; in particular, the code to zero out the temporary emitted by `datum.store_to` is only attached to the particular match-arm for that temporary, but when going down other arms of the match expression, the temporary may falsely appear to have been initialized, depending on what the stack held at that location, and thus may have its destructor erroneously run at the end of the terminating scope. Test cases to appear in a follow-up commit. Fix #20055 --- src/librustc_trans/trans/expr.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 1504c2a7c2d..3e60c0c541a 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -420,9 +420,15 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let tcx = bcx.tcx(); let datum_ty = datum.ty; - // Arrange cleanup - let lval = unpack_datum!(bcx, - datum.to_lvalue_datum(bcx, "unsize_unique_vec", expr.id)); + + debug!("unsize_unique_vec expr.id={} datum_ty={} len={}", + expr.id, datum_ty.repr(tcx), len); + + // We do not arrange cleanup ourselves; if we already are an + // L-value, then cleanup will have already been scheduled (and + // the `datum.store_to` call below will emit code to zero the + // drop flag when moving out of the L-value). If we are an R-value, + // then we do not need to schedule cleanup. let ll_len = C_uint(bcx.ccx(), len); let unit_ty = ty::sequence_element_type(tcx, ty::type_content(datum_ty)); @@ -433,7 +439,7 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let base = PointerCast(bcx, base, type_of::type_of(bcx.ccx(), datum_ty).ptr_to()); - bcx = lval.store_to(bcx, base); + bcx = datum.store_to(bcx, base); Store(bcx, ll_len, get_len(bcx, scratch.val)); DatumBlock::new(bcx, scratch.to_expr_datum()) @@ -455,13 +461,16 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, }; let result_ty = ty::mk_uniq(tcx, ty::unsize_ty(tcx, unboxed_ty, k, expr.span)); - let lval = unpack_datum!(bcx, - datum.to_lvalue_datum(bcx, "unsize_unique_expr", expr.id)); + // We do not arrange cleanup ourselves; if we already are an + // L-value, then cleanup will have already been scheduled (and + // the `datum.store_to` call below will emit code to zero the + // drop flag when moving out of the L-value). If we are an R-value, + // then we do not need to schedule cleanup. let scratch = rvalue_scratch_datum(bcx, result_ty, "__uniq_fat_ptr"); let llbox_ty = type_of::type_of(bcx.ccx(), datum_ty); let base = PointerCast(bcx, get_dataptr(bcx, scratch.val), llbox_ty.ptr_to()); - bcx = lval.store_to(bcx, base); + bcx = datum.store_to(bcx, base); let info = unsized_info(bcx, k, expr.id, unboxed_ty, |t| ty::mk_uniq(tcx, t)); Store(bcx, info, get_len(bcx, scratch.val)); From 5b66c6dfa4a6b258d83f8eaf566e53814c7f812e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 27 Jan 2015 18:20:43 +0100 Subject: [PATCH 2/3] Test cases for issue #20055. Note that I have not yet managed to expose any bug in `trans::expr::into_fat_ptr`; it would be good to try to do so (or show that the use of `.to_lvalue_datum` there is sound). --- src/test/run-pass/issue-20055-box-trait.rs | 40 +++++++++++++++++++ .../run-pass/issue-20055-box-unsized-array.rs | 33 +++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 src/test/run-pass/issue-20055-box-trait.rs create mode 100644 src/test/run-pass/issue-20055-box-unsized-array.rs diff --git a/src/test/run-pass/issue-20055-box-trait.rs b/src/test/run-pass/issue-20055-box-trait.rs new file mode 100644 index 00000000000..fca63f91ca8 --- /dev/null +++ b/src/test/run-pass/issue-20055-box-trait.rs @@ -0,0 +1,40 @@ +// 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. + +trait Boo { } + +impl Boo for [i8; 1] { } +impl Boo for [i8; 2] { } +impl Boo for [i8; 3] { } +impl Boo for [i8; 4] { } + +pub fn foo(box_1: fn () -> Box<[i8; 1]>, + box_2: fn () -> Box<[i8; 20]>, + box_3: fn () -> Box<[i8; 300]>, + box_4: fn () -> Box<[i8; 4000]>, + ) { + println!("Hello World 1"); + let _: Box<[i8]> = match 3 { + 1 => box_1(), + 2 => box_2(), + 3 => box_3(), + _ => box_4(), + }; + println!("Hello World 2"); +} + +pub fn main() { + fn box_1() -> Box<[i8; 1]> { Box::new( [1i8] ) } + fn box_2() -> Box<[i8; 20]> { Box::new( [1i8; 20] ) } + fn box_3() -> Box<[i8; 300]> { Box::new( [1i8; 300] ) } + fn box_4() -> Box<[i8; 4000]> { Box::new( [1i8; 4000] ) } + + foo(box_1, box_2, box_3, box_4); +} diff --git a/src/test/run-pass/issue-20055-box-unsized-array.rs b/src/test/run-pass/issue-20055-box-unsized-array.rs new file mode 100644 index 00000000000..2f3f737d1a7 --- /dev/null +++ b/src/test/run-pass/issue-20055-box-unsized-array.rs @@ -0,0 +1,33 @@ +// 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. + +pub fn foo(box_1: fn () -> Box<[i8; 1]>, + box_2: fn () -> Box<[i8; 20]>, + box_3: fn () -> Box<[i8; 300]>, + box_4: fn () -> Box<[i8; 4000]>, + ) { + println!("Hello World 1"); + let _: Box<[i8]> = match 3 { + 1 => box_1(), + 2 => box_2(), + 3 => box_3(), + _ => box_4(), + }; + println!("Hello World 2"); +} + +pub fn main() { + fn box_1() -> Box<[i8; 1]> { Box::new( [1i8] ) } + fn box_2() -> Box<[i8; 20]> { Box::new( [1i8; 20] ) } + fn box_3() -> Box<[i8; 300]> { Box::new( [1i8; 300] ) } + fn box_4() -> Box<[i8; 4000]> { Box::new( [1i8; 4000] ) } + + foo(box_1, box_2, box_3, box_4); +} From d85520202ab05f1d67da26e00905bf22c548b86f Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 28 Jan 2015 03:37:24 +0100 Subject: [PATCH 3/3] In unsize_unique_expr, do not convert scratch value to lvalue. Fix the issue-20055-box-trait.rs test to actually test `Box`. Fix #21695. --- src/librustc_trans/trans/expr.rs | 5 ---- src/test/run-pass/issue-20055-box-trait.rs | 24 ++++++++++++------- .../run-pass/issue-20055-box-unsized-array.rs | 5 ++++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 3e60c0c541a..cde98154004 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -475,11 +475,6 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let info = unsized_info(bcx, k, expr.id, unboxed_ty, |t| ty::mk_uniq(tcx, t)); Store(bcx, info, get_len(bcx, scratch.val)); - let scratch = unpack_datum!(bcx, - scratch.to_expr_datum().to_lvalue_datum(bcx, - "fresh_uniq_fat_ptr", - expr.id)); - DatumBlock::new(bcx, scratch.to_expr_datum()) } } diff --git a/src/test/run-pass/issue-20055-box-trait.rs b/src/test/run-pass/issue-20055-box-trait.rs index fca63f91ca8..836e78b5b51 100644 --- a/src/test/run-pass/issue-20055-box-trait.rs +++ b/src/test/run-pass/issue-20055-box-trait.rs @@ -8,6 +8,14 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// See Issues #20055 and #21695. + +// We are checking here that the temporaries `Box<[i8, k]>`, for `k` +// in 1, 2, 3, 4, that are induced by the match expression are +// properly handled, in that only *one* will be initialized by +// whichever arm is run, and subsequently dropped at the end of the +// statement surrounding the `match`. + trait Boo { } impl Boo for [i8; 1] { } @@ -16,12 +24,12 @@ impl Boo for [i8; 3] { } impl Boo for [i8; 4] { } pub fn foo(box_1: fn () -> Box<[i8; 1]>, - box_2: fn () -> Box<[i8; 20]>, - box_3: fn () -> Box<[i8; 300]>, - box_4: fn () -> Box<[i8; 4000]>, + box_2: fn () -> Box<[i8; 2]>, + box_3: fn () -> Box<[i8; 3]>, + box_4: fn () -> Box<[i8; 4]>, ) { println!("Hello World 1"); - let _: Box<[i8]> = match 3 { + let _: Box = match 3 { 1 => box_1(), 2 => box_2(), 3 => box_3(), @@ -31,10 +39,10 @@ pub fn foo(box_1: fn () -> Box<[i8; 1]>, } pub fn main() { - fn box_1() -> Box<[i8; 1]> { Box::new( [1i8] ) } - fn box_2() -> Box<[i8; 20]> { Box::new( [1i8; 20] ) } - fn box_3() -> Box<[i8; 300]> { Box::new( [1i8; 300] ) } - fn box_4() -> Box<[i8; 4000]> { Box::new( [1i8; 4000] ) } + fn box_1() -> Box<[i8; 1]> { Box::new( [1i8; 1] ) } + fn box_2() -> Box<[i8; 2]> { Box::new( [1i8; 2] ) } + fn box_3() -> Box<[i8; 3]> { Box::new( [1i8; 3] ) } + fn box_4() -> Box<[i8; 4]> { Box::new( [1i8; 4] ) } foo(box_1, box_2, box_3, box_4); } diff --git a/src/test/run-pass/issue-20055-box-unsized-array.rs b/src/test/run-pass/issue-20055-box-unsized-array.rs index 2f3f737d1a7..f751be6f13b 100644 --- a/src/test/run-pass/issue-20055-box-unsized-array.rs +++ b/src/test/run-pass/issue-20055-box-unsized-array.rs @@ -8,6 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Issue #2005: Check that boxed fixed-size arrays are properly +// accounted for (namely, only deallocated if they were actually +// created) when they appear as temporaries in unused arms of a match +// expression. + pub fn foo(box_1: fn () -> Box<[i8; 1]>, box_2: fn () -> Box<[i8; 20]>, box_3: fn () -> Box<[i8; 300]>,