From fa761f06f8cc031adf64c51f244bdaeb1280ab32 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 14 Nov 2014 15:14:52 -0800 Subject: [PATCH 1/2] librustc: Forbid partial reinitialization of uninitialized structures or enumerations that implement the `Drop` trait. This breaks code like: struct Struct { f: String, g: String, } impl Drop for Struct { ... } fn main() { let x = Struct { ... }; drop(x); x.f = ...; } Change this code to not create partially-initialized structures. For example: struct Struct { f: String, g: String, } impl Drop for Struct { ... } fn main() { let x = Struct { ... }; drop(x); x = Struct { f: ..., g: ..., } } Closes #18571. [breaking-change] --- src/librustc_borrowck/borrowck/check_loans.rs | 50 ++++++++++++++++++- src/librustc_borrowck/borrowck/mod.rs | 12 +++++ .../compile-fail/borrowck-partial-reinit-1.rs | 37 ++++++++++++++ .../compile-fail/borrowck-partial-reinit-2.rs | 34 +++++++++++++ 4 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 src/test/compile-fail/borrowck-partial-reinit-1.rs create mode 100644 src/test/compile-fail/borrowck-partial-reinit-2.rs diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index 91f1121deaab1..f1fdd6606cac5 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -772,13 +772,26 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { if mode != euv::Init { check_for_assignment_to_borrowed_path( self, assignment_id, assignment_span, assignee_cmt.clone()); - mark_variable_as_used_mut(self, assignee_cmt); + mark_variable_as_used_mut(self, assignee_cmt.clone()); } } + + // Check for partial reinitialization of a fully uninitialized structure. + match assignee_cmt.cat { + mc::cat_interior(ref container_cmt, _) | mc::cat_downcast(ref container_cmt) => { + check_for_illegal_initialization(self, + assignment_id, + assignment_span, + container_cmt); + } + mc::cat_rvalue(_) | mc::cat_static_item | mc::cat_upvar(_) | mc::cat_local(_) | + mc::cat_deref(_, _, _) => {} + } return; } - // Initializations are OK. + // Initializations are OK if and only if they aren't partial + // reinitialization of a partially-uninitialized structure. if mode == euv::Init { return } @@ -949,6 +962,39 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { false }); } + + fn check_for_illegal_initialization(this: &CheckLoanCtxt, + assignment_id: ast::NodeId, + span: Span, + assignee_cmt: &mc::cmt) { + let local_id = match assignee_cmt.cat { + mc::cat_interior(ref container_cmt, _) | mc::cat_downcast(ref container_cmt) => { + return check_for_illegal_initialization(this, + assignment_id, + span, + container_cmt) + } + mc::cat_local(local_id) => local_id, + mc::cat_rvalue(_) | mc::cat_static_item | mc::cat_upvar(_) | + mc::cat_deref(..) => return, + }; + + let struct_id = match ty::get(assignee_cmt.ty).sty { + ty::ty_struct(def_id, _) => def_id, + _ => return, + }; + if !ty::has_dtor(this.tcx(), struct_id) { + return + } + + let loan_path = Rc::new(LpVar(local_id)); + this.move_data.each_move_of(assignment_id, &loan_path, |_, _| { + this.bccx.report_partial_reinitialization_of_uninitialized_structure( + span, + &*loan_path); + false + }); + } } pub fn report_illegal_mutation(&self, diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 0bad55948822f..0f59c2a0cdf4a 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -686,6 +686,18 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { } } + pub fn report_partial_reinitialization_of_uninitialized_structure( + &self, + span: Span, + lp: &LoanPath) { + self.tcx + .sess + .span_err(span, + (format!("partial reinitialization of uninitialized \ + structure `{}`", + self.loan_path_to_string(lp))).as_slice()); + } + pub fn report_reassigned_immutable_variable(&self, span: Span, lp: &LoanPath<'tcx>, diff --git a/src/test/compile-fail/borrowck-partial-reinit-1.rs b/src/test/compile-fail/borrowck-partial-reinit-1.rs new file mode 100644 index 0000000000000..7f47eb504fbfc --- /dev/null +++ b/src/test/compile-fail/borrowck-partial-reinit-1.rs @@ -0,0 +1,37 @@ +// Copyright 2014 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. + +struct Test; + +struct Test2 { + b: Option, +} + +impl Drop for Test { + fn drop(&mut self) { + println!("dropping!"); + } +} + +impl Drop for Test2 { + fn drop(&mut self) {} +} + +fn stuff() { + let mut t = Test2 { b: None }; + let u = Test; + drop(t); + t.b = Some(u); + //~^ ERROR partial reinitialization of uninitialized structure +} + +fn main() { + stuff() +} diff --git a/src/test/compile-fail/borrowck-partial-reinit-2.rs b/src/test/compile-fail/borrowck-partial-reinit-2.rs new file mode 100644 index 0000000000000..70995a024f1d4 --- /dev/null +++ b/src/test/compile-fail/borrowck-partial-reinit-2.rs @@ -0,0 +1,34 @@ +// Copyright 2014 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. + +struct Test { + a: int, + b: Option>, +} + +impl Drop for Test { + fn drop(&mut self) { + println!("Dropping {}", self.a); + } +} + +fn stuff() { + let mut t = Test { a: 1, b: None}; + let mut u = Test { a: 2, b: Some(box t)}; + t.b = Some(box u); + //~^ ERROR partial reinitialization of uninitialized structure + println!("done"); +} + +fn main() { + stuff(); + println!("Hello, world!") +} + From dda79e944d4109d5eb729aeea628ae160234c2ce Mon Sep 17 00:00:00 2001 From: Kevin Butler Date: Sun, 8 Feb 2015 04:29:09 +0000 Subject: [PATCH 2/2] Rebase and handle additional testcase. --- src/librustc_borrowck/borrowck/check_loans.rs | 66 ++++++------------- src/librustc_borrowck/borrowck/mod.rs | 2 +- .../compile-fail/borrowck-partial-reinit-1.rs | 16 ++++- .../compile-fail/borrowck-partial-reinit-2.rs | 10 +-- .../compile-fail/borrowck-partial-reinit-3.rs | 22 +++++++ .../compile-fail/borrowck-partial-reinit-4.rs | 33 ++++++++++ 6 files changed, 96 insertions(+), 53 deletions(-) create mode 100644 src/test/compile-fail/borrowck-partial-reinit-3.rs create mode 100644 src/test/compile-fail/borrowck-partial-reinit-4.rs diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index f1fdd6606cac5..d3f830c7e845b 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -745,6 +745,26 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { use_kind, lp_base); } LpExtend(ref lp_base, _, LpInterior(InteriorField(_))) => { + match lp_base.to_type().sty { + ty::ty_struct(def_id, _) | ty::ty_enum(def_id, _) => { + if ty::has_dtor(self.tcx(), def_id) { + // In the case where the owner implements drop, then + // the path must be initialized to prevent a case of + // partial reinitialization + let loan_path = owned_ptr_base_path_rc(lp_base); + self.move_data.each_move_of(id, &loan_path, |_, _| { + self.bccx + .report_partial_reinitialization_of_uninitialized_structure( + span, + &*loan_path); + false + }); + return; + } + }, + _ => {}, + } + // assigning to `P.f` is ok if assigning to `P` is ok self.check_if_assigned_path_is_moved(id, span, use_kind, lp_base); @@ -772,21 +792,10 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { if mode != euv::Init { check_for_assignment_to_borrowed_path( self, assignment_id, assignment_span, assignee_cmt.clone()); - mark_variable_as_used_mut(self, assignee_cmt.clone()); + mark_variable_as_used_mut(self, assignee_cmt); } } - // Check for partial reinitialization of a fully uninitialized structure. - match assignee_cmt.cat { - mc::cat_interior(ref container_cmt, _) | mc::cat_downcast(ref container_cmt) => { - check_for_illegal_initialization(self, - assignment_id, - assignment_span, - container_cmt); - } - mc::cat_rvalue(_) | mc::cat_static_item | mc::cat_upvar(_) | mc::cat_local(_) | - mc::cat_deref(_, _, _) => {} - } return; } @@ -962,39 +971,6 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { false }); } - - fn check_for_illegal_initialization(this: &CheckLoanCtxt, - assignment_id: ast::NodeId, - span: Span, - assignee_cmt: &mc::cmt) { - let local_id = match assignee_cmt.cat { - mc::cat_interior(ref container_cmt, _) | mc::cat_downcast(ref container_cmt) => { - return check_for_illegal_initialization(this, - assignment_id, - span, - container_cmt) - } - mc::cat_local(local_id) => local_id, - mc::cat_rvalue(_) | mc::cat_static_item | mc::cat_upvar(_) | - mc::cat_deref(..) => return, - }; - - let struct_id = match ty::get(assignee_cmt.ty).sty { - ty::ty_struct(def_id, _) => def_id, - _ => return, - }; - if !ty::has_dtor(this.tcx(), struct_id) { - return - } - - let loan_path = Rc::new(LpVar(local_id)); - this.move_data.each_move_of(assignment_id, &loan_path, |_, _| { - this.bccx.report_partial_reinitialization_of_uninitialized_structure( - span, - &*loan_path); - false - }); - } } pub fn report_illegal_mutation(&self, diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 0f59c2a0cdf4a..7c055bc3118b1 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -689,7 +689,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { pub fn report_partial_reinitialization_of_uninitialized_structure( &self, span: Span, - lp: &LoanPath) { + lp: &LoanPath<'tcx>) { self.tcx .sess .span_err(span, diff --git a/src/test/compile-fail/borrowck-partial-reinit-1.rs b/src/test/compile-fail/borrowck-partial-reinit-1.rs index 7f47eb504fbfc..1ee040a0705ae 100644 --- a/src/test/compile-fail/borrowck-partial-reinit-1.rs +++ b/src/test/compile-fail/borrowck-partial-reinit-1.rs @@ -1,4 +1,4 @@ -// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// Copyright 2014-2015 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -14,6 +14,8 @@ struct Test2 { b: Option, } +struct Test3(Option); + impl Drop for Test { fn drop(&mut self) { println!("dropping!"); @@ -24,12 +26,22 @@ impl Drop for Test2 { fn drop(&mut self) {} } +impl Drop for Test3 { + fn drop(&mut self) {} +} + fn stuff() { let mut t = Test2 { b: None }; let u = Test; drop(t); t.b = Some(u); - //~^ ERROR partial reinitialization of uninitialized structure + //~^ ERROR partial reinitialization of uninitialized structure `t` + + let mut t = Test3(None); + let u = Test; + drop(t); + t.0 = Some(u); + //~^ ERROR partial reinitialization of uninitialized structure `t` } fn main() { diff --git a/src/test/compile-fail/borrowck-partial-reinit-2.rs b/src/test/compile-fail/borrowck-partial-reinit-2.rs index 70995a024f1d4..0926ba6e43250 100644 --- a/src/test/compile-fail/borrowck-partial-reinit-2.rs +++ b/src/test/compile-fail/borrowck-partial-reinit-2.rs @@ -1,4 +1,4 @@ -// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// Copyright 2014-2015 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -9,7 +9,7 @@ // except according to those terms. struct Test { - a: int, + a: isize, b: Option>, } @@ -21,9 +21,9 @@ impl Drop for Test { fn stuff() { let mut t = Test { a: 1, b: None}; - let mut u = Test { a: 2, b: Some(box t)}; - t.b = Some(box u); - //~^ ERROR partial reinitialization of uninitialized structure + let mut u = Test { a: 2, b: Some(Box::new(t))}; + t.b = Some(Box::new(u)); + //~^ ERROR partial reinitialization of uninitialized structure `t` println!("done"); } diff --git a/src/test/compile-fail/borrowck-partial-reinit-3.rs b/src/test/compile-fail/borrowck-partial-reinit-3.rs new file mode 100644 index 0000000000000..0aa73892b8229 --- /dev/null +++ b/src/test/compile-fail/borrowck-partial-reinit-3.rs @@ -0,0 +1,22 @@ +// 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. +use std::mem; + +struct Test { f: usize } +impl Drop for Test { + fn drop(&mut self) {} +} + +fn main() { + let mut x = (Test { f: 2 }, Test { f: 4 }); + mem::drop(x.0); + x.0.f = 3; + //~^ ERROR partial reinitialization of uninitialized structure `x.0` +} diff --git a/src/test/compile-fail/borrowck-partial-reinit-4.rs b/src/test/compile-fail/borrowck-partial-reinit-4.rs new file mode 100644 index 0000000000000..774e04ecb298e --- /dev/null +++ b/src/test/compile-fail/borrowck-partial-reinit-4.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. + +struct Test; + +struct Test2(Option); + +impl Drop for Test { + fn drop(&mut self) { + println!("dropping!"); + } +} + +impl Drop for Test2 { + fn drop(&mut self) {} +} + +fn stuff() { + let mut x : (Test2, Test2); + (x.0).0 = Some(Test); + //~^ ERROR partial reinitialization of uninitialized structure `x.0` +} + +fn main() { + stuff() +}