From 25aa86da17a0a96767eb1bf30198973d5f2fd702 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 22 Aug 2017 15:54:57 +0200 Subject: [PATCH 1/4] Fix issue #43481: emit the EndRegion *before* StorageDeads for a scope. (The idea is that the StorageDead marks the point where the memory can be deallocated, and the EndRegion is marking where borrows of that memory can no longer legally exist.) --- src/librustc_mir/build/scope.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index c08620c1e4104..bac884b4d01e9 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -352,6 +352,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } let scope = self.scopes.pop().unwrap(); assert_eq!(scope.region_scope, region_scope.0); + + self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope); unpack!(block = build_scope_drops(&mut self.cfg, &scope, &self.scopes, @@ -359,7 +361,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.arg_count, false)); - self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope); block.unit() } @@ -406,15 +407,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { scope.cached_exits.insert((target, region_scope.0), b); b }; + + // End all regions for scopes out of which we are breaking. + self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope); + unpack!(block = build_scope_drops(&mut self.cfg, scope, rest, block, self.arg_count, false)); - - // End all regions for scopes out of which we are breaking. - self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope); } } let scope = &self.scopes[len - scope_count]; From ab46142bd902f697f70305499e99a172caa419a5 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 28 Aug 2017 11:40:37 +0200 Subject: [PATCH 2/4] Update mir-opt tests to reflect change to EndRegion emission order. Driveby fix to end_region_9.rs; it was missing END marker and was therefore always passing (regardless of output correctness). --- src/test/mir-opt/end_region_1.rs | 2 +- src/test/mir-opt/end_region_2.rs | 6 +++--- src/test/mir-opt/end_region_3.rs | 6 +++--- src/test/mir-opt/end_region_4.rs | 4 ++-- src/test/mir-opt/end_region_5.rs | 2 +- src/test/mir-opt/end_region_6.rs | 4 ++-- src/test/mir-opt/end_region_7.rs | 2 +- src/test/mir-opt/end_region_8.rs | 2 +- src/test/mir-opt/end_region_9.rs | 7 ++++--- src/test/mir-opt/validate_1.rs | 2 +- src/test/mir-opt/validate_5.rs | 2 +- 11 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/test/mir-opt/end_region_1.rs b/src/test/mir-opt/end_region_1.rs index 9c8cfc0ab208c..1941d1bc7be1b 100644 --- a/src/test/mir-opt/end_region_1.rs +++ b/src/test/mir-opt/end_region_1.rs @@ -30,8 +30,8 @@ fn main() { // StorageLive(_2); // _2 = &'10_1rs _1; // _0 = (); -// StorageDead(_2); // EndRegion('10_1rs); +// StorageDead(_2); // StorageDead(_1); // return; // } diff --git a/src/test/mir-opt/end_region_2.rs b/src/test/mir-opt/end_region_2.rs index 6a4e209a14b7f..d8dd4aeadf495 100644 --- a/src/test/mir-opt/end_region_2.rs +++ b/src/test/mir-opt/end_region_2.rs @@ -46,8 +46,8 @@ fn main() { // bb2: { // _0 = (); // StorageDead(_5); -// StorageDead(_3); // EndRegion('23_1rs); +// StorageDead(_3); // StorageDead(_2); // return; // } @@ -56,10 +56,10 @@ fn main() { // StorageLive(_7); // _7 = &'23_3rs _2; // _1 = (); -// StorageDead(_7); // EndRegion('23_3rs); -// StorageDead(_3); +// StorageDead(_7); // EndRegion('23_1rs); +// StorageDead(_3); // StorageDead(_2); // goto -> bb1; // } diff --git a/src/test/mir-opt/end_region_3.rs b/src/test/mir-opt/end_region_3.rs index cb1d45e37c834..e404af838cef4 100644 --- a/src/test/mir-opt/end_region_3.rs +++ b/src/test/mir-opt/end_region_3.rs @@ -49,8 +49,8 @@ fn main() { // bb2: { // _0 = (); // StorageDead(_5); -// StorageDead(_3); // EndRegion('26_1rs); +// StorageDead(_3); // StorageDead(_1); // return; // } @@ -60,10 +60,10 @@ fn main() { // StorageLive(_7); // _7 = &'26_3rs _1; // _2 = (); -// StorageDead(_7); // EndRegion('26_3rs); -// StorageDead(_3); +// StorageDead(_7); // EndRegion('26_1rs); +// StorageDead(_3); // goto -> bb1; // } // END rustc.node4.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_4.rs b/src/test/mir-opt/end_region_4.rs index b714b6bc55b24..d51c627d14b23 100644 --- a/src/test/mir-opt/end_region_4.rs +++ b/src/test/mir-opt/end_region_4.rs @@ -53,10 +53,10 @@ fn foo(i: i32) { // StorageLive(_6); // _6 = &'26_4rs _2; // _0 = (); -// StorageDead(_6); // EndRegion('26_4rs); -// StorageDead(_3); +// StorageDead(_6); // EndRegion('26_2rs); +// StorageDead(_3); // StorageDead(_2); // drop(_1) -> bb4; // } diff --git a/src/test/mir-opt/end_region_5.rs b/src/test/mir-opt/end_region_5.rs index a4c53675b9c3a..6299ec3815cd1 100644 --- a/src/test/mir-opt/end_region_5.rs +++ b/src/test/mir-opt/end_region_5.rs @@ -44,8 +44,8 @@ fn foo(f: F) where F: FnOnce() -> i32 { // _2 = const foo(_3) -> [return: bb1, unwind: bb3]; // } // bb1: { -// StorageDead(_3); // EndRegion('14s); +// StorageDead(_3); // _0 = (); // drop(_1) -> bb4; // } diff --git a/src/test/mir-opt/end_region_6.rs b/src/test/mir-opt/end_region_6.rs index daa94769430df..13ab3e4f2dd2a 100644 --- a/src/test/mir-opt/end_region_6.rs +++ b/src/test/mir-opt/end_region_6.rs @@ -44,8 +44,8 @@ fn foo(f: F) where F: FnOnce() -> i32 { // _2 = const foo(_3) -> [return: bb1, unwind: bb3]; // } // bb1: { -// StorageDead(_3); // EndRegion('19s); +// StorageDead(_3); // _0 = (); // drop(_1) -> bb4; // } @@ -75,8 +75,8 @@ fn foo(f: F) where F: FnOnce() -> i32 { // _3 = ((*_2).0: i32); // _0 = _3; // StorageDead(_3); -// StorageDead(_2); // EndRegion('15_0rs); +// StorageDead(_2); // return; // } // END rustc.node22.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_7.rs b/src/test/mir-opt/end_region_7.rs index 9feacd04fe0fa..826d3749167d3 100644 --- a/src/test/mir-opt/end_region_7.rs +++ b/src/test/mir-opt/end_region_7.rs @@ -84,8 +84,8 @@ fn foo(f: F) where F: FnOnce() -> i32 { // _3 = ((*_2).0: i32); // _0 = _3; // StorageDead(_3); -// StorageDead(_2); // EndRegion('15_0rs); +// StorageDead(_2); // drop(_1) -> bb1; // } // bb1: { diff --git a/src/test/mir-opt/end_region_8.rs b/src/test/mir-opt/end_region_8.rs index 30dda7a406260..6438484fcfae4 100644 --- a/src/test/mir-opt/end_region_8.rs +++ b/src/test/mir-opt/end_region_8.rs @@ -50,8 +50,8 @@ fn foo(f: F) where F: FnOnce() -> i32 { // bb1: { // StorageDead(_4); // _0 = (); -// StorageDead(_2); // EndRegion('21_1rs); +// StorageDead(_2); // drop(_1) -> bb4; // } // bb2: { diff --git a/src/test/mir-opt/end_region_9.rs b/src/test/mir-opt/end_region_9.rs index bbfdc0e77a8d6..59d5d934391e9 100644 --- a/src/test/mir-opt/end_region_9.rs +++ b/src/test/mir-opt/end_region_9.rs @@ -42,7 +42,7 @@ fn main() { // let mut _0: (); // let mut _1: bool; // let _2: i32; -// let mut _4: &'13_0rs i32; +// let mut _4: &'33_0rs i32; // let mut _3: (); // let mut _5: !; // let mut _6: (); @@ -67,15 +67,15 @@ fn main() { // bb2: { // _0 = (); // StorageDead(_7); +// EndRegion('33_0rs); // StorageDead(_4); -// EndRegion('13_0rs); // StorageDead(_2); // StorageDead(_1); // return; // } // // bb3: { -// _4 = &'13_0rs _2; +// _4 = &'33_0rs _2; // _6 = (); // StorageDead(_7); // _1 = const true; @@ -83,3 +83,4 @@ fn main() { // goto -> bb1; // } // } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs index 842bc8374b6fb..ec044225b83e8 100644 --- a/src/test/mir-opt/validate_1.rs +++ b/src/test/mir-opt/validate_1.rs @@ -68,8 +68,8 @@ fn main() { // _5 = (*_4); // _0 = _5; // StorageDead(_5); -// StorageDead(_4); // EndRegion(ReScope(Remainder(BlockRemainder { block: ItemLocalId(22), first_statement_index: 0 }))); +// StorageDead(_4); // StorageDead(_3); // return; // } diff --git a/src/test/mir-opt/validate_5.rs b/src/test/mir-opt/validate_5.rs index 4bebc74ae460b..ff0c781d1e3bb 100644 --- a/src/test/mir-opt/validate_5.rs +++ b/src/test/mir-opt/validate_5.rs @@ -54,8 +54,8 @@ fn main() { // _5 = &ReErased mut (*_3); // Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(9)))]); // _4 = _5 as *mut i32 (Misc); -// StorageDead(_5); // EndRegion(ReScope(Node(ItemLocalId(9)))); +// StorageDead(_5); // Validate(Release, [_0: bool, _4: *mut i32]); // _0 = const write_42(_4) -> bb1; // } From 88080bd56a62cac418d245482f461bdcfe8e01ab Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 28 Aug 2017 13:39:06 +0200 Subject: [PATCH 3/4] Unit test for proper EndRegion emission on a cyclic reference. --- src/test/mir-opt/end_region_cyclic.rs | 132 ++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 src/test/mir-opt/end_region_cyclic.rs diff --git a/src/test/mir-opt/end_region_cyclic.rs b/src/test/mir-opt/end_region_cyclic.rs new file mode 100644 index 0000000000000..8f9dd79cd7542 --- /dev/null +++ b/src/test/mir-opt/end_region_cyclic.rs @@ -0,0 +1,132 @@ +// Copyright 2017 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. + +// compile-flags: -Z identify_regions -Z span_free_formats -Z emit-end-regions +// ignore-tidy-linelength + +// This test models a scenario with a cyclic reference. Rust obviously +// needs to handle such cases. +// +// The interesting part about this test is that such case shows that +// one cannot generally force all references to be dead before you hit +// their EndRegion; at least, not without breaking the more important +// property that all borrowed storage locations have their regions +// ended strictly before their StorageDeads. (This test was inspired +// by discussion on Issue #43481.) + +use std::cell::Cell; + +struct S<'a> { + r: Cell>>, +} + +fn main() { + loop { + let x = S { r: Cell::new(None) }; + x.r.set(Some(&x)); + if query() { break; } + x.r.set(Some(&x)); + } +} + +fn query() -> bool { true } + +// END RUST SOURCE +// START rustc.node16.SimplifyCfg-qualify-consts.after.mir +// fn main() -> () { +// let mut _0: (); +// scope 1 { +// let _2: S<'35_0rs>; +// } +// let mut _1: (); +// let mut _3: std::cell::Cell>>; +// let mut _4: std::option::Option<&'35_0rs S<'35_0rs>>; +// let mut _5: (); +// let mut _6: &'16s std::cell::Cell>>; +// let mut _7: std::option::Option<&'35_0rs S<'35_0rs>>; +// let mut _8: &'35_0rs S<'35_0rs>; +// let mut _9: &'35_0rs S<'35_0rs>; +// let mut _10: (); +// let mut _11: bool; +// let mut _12: !; +// let mut _13: (); +// let mut _14: &'33s std::cell::Cell>>; +// let mut _15: std::option::Option<&'35_0rs S<'35_0rs>>; +// let mut _16: &'35_0rs S<'35_0rs>; +// let mut _17: &'35_0rs S<'35_0rs>; +// bb0: { +// goto -> bb1; +// } +// bb1: { +// StorageLive(_2); +// StorageLive(_3); +// StorageLive(_4); +// _4 = std::option::Option<&'35_0rs S<'35_0rs>>::None; +// _3 = const >::new(_4) -> bb2; +// } +// bb2: { +// StorageDead(_4); +// _2 = S<'35_0rs> { r: _3 }; +// StorageDead(_3); +// StorageLive(_6); +// _6 = &'16s (_2.0: std::cell::Cell>>); +// StorageLive(_7); +// StorageLive(_8); +// StorageLive(_9); +// _9 = &'35_0rs _2; +// _8 = &'35_0rs (*_9); +// _7 = std::option::Option<&'35_0rs S<'35_0rs>>::Some(_8,); +// StorageDead(_8); +// _5 = const >::set(_6, _7) -> bb3; +// } +// bb3: { +// EndRegion('16s); +// StorageDead(_7); +// StorageDead(_6); +// StorageDead(_9); +// StorageLive(_11); +// _11 = const query() -> bb4; +// } +// bb4: { +// switchInt(_11) -> [0u8: bb6, otherwise: bb5]; +// } +// bb5: { +// _0 = (); +// StorageDead(_11); +// EndRegion('35_0rs); +// StorageDead(_2); +// return; +// } +// bb6: { +// _10 = (); +// StorageDead(_11); +// StorageLive(_14); +// _14 = &'33s (_2.0: std::cell::Cell>>); +// StorageLive(_15); +// StorageLive(_16); +// StorageLive(_17); +// _17 = &'35_0rs _2; +// _16 = &'35_0rs (*_17); +// _15 = std::option::Option<&'35_0rs S<'35_0rs>>::Some(_16,); +// StorageDead(_16); +// _13 = const >::set(_14, _15) -> bb7; +// } +// bb7: { +// EndRegion('33s); +// StorageDead(_15); +// StorageDead(_14); +// StorageDead(_17); +// _1 = (); +// EndRegion('35_0rs); +// StorageDead(_2); +// goto -> bb1; +// } +// } +// END rustc.node16.SimplifyCfg-qualify-consts.after.mir From 5fa0b661e02756d8158057d8fd9b0ca1f1f755fe Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 31 Aug 2017 13:32:52 +0200 Subject: [PATCH 4/4] Test case illustrating some destruction code extent stuff. --- .../end_region_destruction_extents_1.rs | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 src/test/mir-opt/end_region_destruction_extents_1.rs diff --git a/src/test/mir-opt/end_region_destruction_extents_1.rs b/src/test/mir-opt/end_region_destruction_extents_1.rs new file mode 100644 index 0000000000000..1f9ad988acc0b --- /dev/null +++ b/src/test/mir-opt/end_region_destruction_extents_1.rs @@ -0,0 +1,161 @@ +// Copyright 2017 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. + +// compile-flags: -Z identify_regions -Z span_free_formats -Z emit-end-regions +// ignore-tidy-linelength + +// A scenario with significant destruction code extents (which have +// suffix "dce" in current `-Z identify_regions` rendering). + +#![feature(generic_param_attrs)] +#![feature(dropck_eyepatch)] + +fn main() { + // Since the second param to `D1` is may_dangle, it is legal for + // the region of that parameter to end before the drop code for D1 + // is executed. + (D1(&S1("ex1"), &S1("dang1"))).0; +} + +#[derive(Debug)] +struct S1(&'static str); + +#[derive(Debug)] +struct D1<'a, 'b>(&'a S1, &'b S1); + +// The `#[may_dangle]` means that references of type `&'b _` may be +// invalid during the execution of this destructor; i.e. in this case +// the destructor code is not allowed to read or write `*self.1`, while +// it can read/write `*self.0`. +unsafe impl<'a, #[may_dangle] 'b> Drop for D1<'a, 'b> { + fn drop(&mut self) { + println!("D1({:?}, _)", self.0); + } +} + +// Notes on the MIR output below: +// +// 1. The `EndRegion('10s)` is allowed to precede the `drop(_3)` +// solely because of the #[may_dangle] mentioned above. +// +// 2. Regarding the occurrence of `EndRegion('12ds)` *after* `StorageDead(_6)` +// (where we have borrows `&'12ds _6`): Eventually: +// +// i. this code should be rejected (by mir-borrowck), or +// +// ii. the MIR code generation should be changed so that the +// EndRegion('12ds)` precedes `StorageDead(_6)` in the +// control-flow. (Note: arielb1 views drop+storagedead as one +// unit, and does not see this option as a useful avenue to +// explore.), or +// +// iii. the presence of EndRegion should be made irrelevant by a +// transformation encoding the effects of rvalue-promotion. +// This may be the simplest and most-likely option; note in +// particular that `StorageDead(_6)` goes away below in +// rustc.node4.QualifyAndPromoteConstants.after.mir + +// END RUST SOURCE + +// START rustc.node4.QualifyAndPromoteConstants.before.mir +// fn main() -> () { +// let mut _0: (); +// let mut _1: &'12ds S1; +// let mut _2: &'12ds S1; +// let mut _3: D1<'12ds, '10s>; +// let mut _4: &'12ds S1; +// let mut _5: &'12ds S1; +// let mut _6: S1; +// let mut _7: &'10s S1; +// let mut _8: &'10s S1; +// let mut _9: S1; +// +// bb0: { +// StorageLive(_2); +// StorageLive(_3); +// StorageLive(_4); +// StorageLive(_5); +// StorageLive(_6); +// _6 = S1::{{constructor}}(const "ex1",); +// _5 = &'12ds _6; +// _4 = &'12ds (*_5); +// StorageLive(_7); +// StorageLive(_8); +// StorageLive(_9); +// _9 = S1::{{constructor}}(const "dang1",); +// _8 = &'10s _9; +// _7 = &'10s (*_8); +// _3 = D1<'12ds, '10s>::{{constructor}}(_4, _7); +// EndRegion('10s); +// StorageDead(_7); +// StorageDead(_4); +// _2 = (_3.0: &'12ds S1); +// _1 = _2; +// StorageDead(_2); +// drop(_3) -> bb1; +// } +// +// bb1: { +// StorageDead(_3); +// StorageDead(_8); +// StorageDead(_9); +// StorageDead(_5); +// StorageDead(_6); +// EndRegion('12ds); +// _0 = (); +// return; +// } +// } +// END rustc.node4.QualifyAndPromoteConstants.before.mir + +// START rustc.node4.QualifyAndPromoteConstants.after.mir +// fn main() -> () { +// let mut _0: (); +// let mut _1: &'12ds S1; +// let mut _2: &'12ds S1; +// let mut _3: D1<'12ds, '10s>; +// let mut _4: &'12ds S1; +// let mut _5: &'12ds S1; +// let mut _6: S1; +// let mut _7: &'10s S1; +// let mut _8: &'10s S1; +// let mut _9: S1; +// +// bb0: { +// StorageLive(_2); +// StorageLive(_3); +// StorageLive(_4); +// StorageLive(_5); +// _5 = promoted1; +// _4 = &'12ds (*_5); +// StorageLive(_7); +// StorageLive(_8); +// _8 = promoted0; +// _7 = &'10s (*_8); +// _3 = D1<'12ds, '10s>::{{constructor}}(_4, _7); +// EndRegion('10s); +// StorageDead(_7); +// StorageDead(_4); +// _2 = (_3.0: &'12ds S1); +// _1 = _2; +// StorageDead(_2); +// drop(_3) -> bb1; +// } +// +// bb1: { +// StorageDead(_3); +// StorageDead(_8); +// StorageDead(_5); +// EndRegion('12ds); +// _0 = (); +// return; +// } +// } +// END rustc.node4.QualifyAndPromoteConstants.after.mir