From 32d0dbd49a372b9d2e000587497de6ffca9e1cca Mon Sep 17 00:00:00 2001 From: Kevin Butler 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] ---- (Joint authorship by pcwalton and Ryman; thanks all!) --- src/librustc_borrowck/borrowck/check_loans.rs | 24 ++++++++- src/librustc_borrowck/borrowck/mod.rs | 12 +++++ .../compile-fail/borrowck-partial-reinit-1.rs | 49 +++++++++++++++++++ .../compile-fail/borrowck-partial-reinit-2.rs | 34 +++++++++++++ .../compile-fail/borrowck-partial-reinit-3.rs | 22 +++++++++ .../compile-fail/borrowck-partial-reinit-4.rs | 33 +++++++++++++ 6 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/borrowck-partial-reinit-1.rs create mode 100644 src/test/compile-fail/borrowck-partial-reinit-2.rs 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 91f1121deaab1..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); @@ -775,10 +795,12 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { mark_variable_as_used_mut(self, assignee_cmt); } } + 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 } diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 0bad55948822f..7c055bc3118b1 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<'tcx>) { + 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..1ee040a0705ae --- /dev/null +++ b/src/test/compile-fail/borrowck-partial-reinit-1.rs @@ -0,0 +1,49 @@ +// 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. +// +// 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, +} + +struct Test3(Option); + +impl Drop for Test { + fn drop(&mut self) { + println!("dropping!"); + } +} + +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 `t` + + let mut t = Test3(None); + let u = Test; + drop(t); + t.0 = Some(u); + //~^ ERROR partial reinitialization of uninitialized structure `t` +} + +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..0926ba6e43250 --- /dev/null +++ b/src/test/compile-fail/borrowck-partial-reinit-2.rs @@ -0,0 +1,34 @@ +// 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. +// +// 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: isize, + 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::new(t))}; + t.b = Some(Box::new(u)); + //~^ ERROR partial reinitialization of uninitialized structure `t` + println!("done"); +} + +fn main() { + stuff(); + println!("Hello, world!") +} + 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() +} From 6cc3b00d3f7b1e7512d85830bf62f2acc461237d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 12 Feb 2015 13:53:16 +0100 Subject: [PATCH 2/2] Add a couple FIXME notes inspired during my review. --- src/librustc_borrowck/borrowck/check_loans.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index d3f830c7e845b..a18e8b16e8bac 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -699,6 +699,11 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { lp: &Rc>) { debug!("check_if_path_is_moved(id={}, use_kind={:?}, lp={})", id, use_kind, lp.repr(self.bccx.tcx)); + + // FIXME (22079): if you find yourself tempted to cut and paste + // the body below and then specializing the error reporting, + // consider refactoring this instead! + let base_lp = owned_ptr_base_path_rc(lp); self.move_data.each_move_of(id, &base_lp, |the_move, moved_lp| { self.bccx.report_use_of_moved_value( @@ -751,6 +756,9 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { // In the case where the owner implements drop, then // the path must be initialized to prevent a case of // partial reinitialization + // + // FIXME (22079): could refactor via hypothetical + // generalized check_if_path_is_moved let loan_path = owned_ptr_base_path_rc(lp_base); self.move_data.each_move_of(id, &loan_path, |_, _| { self.bccx