From 07becc5f2438a2f628e8cb5d5a6128dd9ed2cc31 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 8 Sep 2017 20:22:50 -0400 Subject: [PATCH 1/3] Fix regression in promotion of rvalues referencing a static This commit makes librustc_passes::consts::CheckCrateVisitor properly mark expressions as promotable if they reference a static, as it's perfectly fine for one static to reference another. It fixes a regression that prevented a temporary rvalue from referencing a static if it was itself declared within a static. Prior to commit https://github.com/rust-lang/rust/commit/b8c05fe90bc, `region::ScopeTree` would only register a 'terminating scope' for function bodies. Thus, while rvalues in a static that referenced a static would be marked unpromotable, the lack of enclosing scope would cause mem_categorization::MemCategorizationContext::cat_rvalue_node to compute a 'temporary scope' of `ReStatic`. Since this had the same effect as explicitly selecting a scope of `ReStatic` due to the rvalue being marked by CheckCrateVisitor as promotable, no issue occurred. However, commit https://github.com/rust-lang/rust/commit/b8c05fe90bc made ScopeTree unconditionally register a 'terminating scope' Since mem_categorization would now compute a non-static 'temporary scope', the aforementioned rvalues would be erroneously marked as living for too short a time. By fixing the behavior of CheckCrateVisitor, this commit avoids changing mem_categorization's behavior, while ensuring that temporary values in statics are still allowed to reference other statics. Fixes issue #44373 --- src/librustc_passes/consts.rs | 23 ++++++++++++++++++++++- src/test/run-pass/issue-44373.rs | 18 ++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 src/test/run-pass/issue-44373.rs diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs index 6f2c448ceb6f9..f7581bb604fe2 100644 --- a/src/librustc_passes/consts.rs +++ b/src/librustc_passes/consts.rs @@ -327,7 +327,28 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node let def = v.tables.qpath_def(qpath, e.hir_id); match def { Def::VariantCtor(..) | Def::StructCtor(..) | - Def::Fn(..) | Def::Method(..) => {} + Def::Fn(..) | Def::Method(..) => {} + + // References to a static are inherently promotable, + // with the exception of "#[thread_loca]" statics. + // The latter may not outlive the current function + Def::Static(did, _) => { + let mut thread_local = false; + + for attr in &v.tcx.get_attrs(did)[..] { + if attr.check_name("thread_local") { + debug!("Static(id={:?}) is unpromotable \ + due to a #[thread_local] attribute", did); + v.promotable = false; + thread_local = true; + break; + } + } + + if !thread_local { + debug!("Allowing promotion of reference to Static(id={:?})", did); + } + } Def::Const(did) | Def::AssociatedConst(did) => { diff --git a/src/test/run-pass/issue-44373.rs b/src/test/run-pass/issue-44373.rs new file mode 100644 index 0000000000000..06627e2ad9341 --- /dev/null +++ b/src/test/run-pass/issue-44373.rs @@ -0,0 +1,18 @@ +// 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. + +struct Foo(bool); + +struct Container(&'static [&'static Foo]); + +static FOO: Foo = Foo(true); +static CONTAINER: Container = Container(&[&FOO]); + +fn main() {} From 813b323bdcece529a5cc7e5d69aa596253c484e6 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 9 Sep 2017 16:01:45 -0400 Subject: [PATCH 2/3] Don't promote references to statics that occur in non-static locations --- src/librustc_passes/consts.rs | 46 +++++++++++++++++++--------- src/test/compile-fail/issue-44373.rs | 16 ++++++++++ 2 files changed, 47 insertions(+), 15 deletions(-) create mode 100644 src/test/compile-fail/issue-44373.rs diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs index f7581bb604fe2..eb955ccf6bf70 100644 --- a/src/librustc_passes/consts.rs +++ b/src/librustc_passes/consts.rs @@ -56,6 +56,7 @@ use std::cmp::Ordering; struct CheckCrateVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, in_fn: bool, + in_static: bool, promotable: bool, mut_rvalue_borrows: NodeSet, param_env: ty::ParamEnv<'tcx>, @@ -128,10 +129,16 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckCrateVisitor<'a, 'tcx> { let outer_param_env = self.param_env; let outer_identity_substs = self.identity_substs; - self.in_fn = match MirSource::from_node(self.tcx, item_id) { - MirSource::Fn(_) => true, - _ => false + self.in_fn = false; + self.in_static = false; + + match MirSource::from_node(self.tcx, item_id) { + MirSource::Fn(_) => self.in_fn = true, + MirSource::Static(_, _) => self.in_static = true, + _ => {} }; + + self.tables = self.tcx.typeck_tables_of(item_def_id); self.param_env = self.tcx.param_env(item_def_id); self.identity_substs = Substs::identity_for_item(self.tcx, item_def_id); @@ -333,20 +340,28 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node // with the exception of "#[thread_loca]" statics. // The latter may not outlive the current function Def::Static(did, _) => { - let mut thread_local = false; - - for attr in &v.tcx.get_attrs(did)[..] { - if attr.check_name("thread_local") { - debug!("Static(id={:?}) is unpromotable \ - due to a #[thread_local] attribute", did); - v.promotable = false; - thread_local = true; - break; + + if v.in_static { + let mut thread_local = false; + + for attr in &v.tcx.get_attrs(did)[..] { + if attr.check_name("thread_local") { + debug!("Reference to Static(id={:?}) is unpromotable \ + due to a #[thread_local] attribute", did); + v.promotable = false; + thread_local = true; + break; + } } - } - if !thread_local { - debug!("Allowing promotion of reference to Static(id={:?})", did); + if !thread_local { + debug!("Allowing promotion of reference to Static(id={:?})", did); + } + } else { + debug!("Reference to Static(id={:?}) is unpromotable as it is not \ + referenced from a static", did); + v.promotable = false; + } } @@ -502,6 +517,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { tcx, tables: &ty::TypeckTables::empty(None), in_fn: false, + in_static: false, promotable: false, mut_rvalue_borrows: NodeSet(), param_env: ty::ParamEnv::empty(Reveal::UserFacing), diff --git a/src/test/compile-fail/issue-44373.rs b/src/test/compile-fail/issue-44373.rs new file mode 100644 index 0000000000000..d744ad11c5c72 --- /dev/null +++ b/src/test/compile-fail/issue-44373.rs @@ -0,0 +1,16 @@ +// 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. + + +static FOO: u32 = 50; + +fn main() { + let _val: &'static [&'static u32] = &[&FOO]; //~ ERROR borrowed value does not live long enough +} From fb540e3de45571ac85c610475031449993c19277 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 11 Sep 2017 10:51:28 -0400 Subject: [PATCH 3/3] Update comment to properly describe static promotion restrictions --- src/librustc_passes/consts.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs index eb955ccf6bf70..547d63fc3d4aa 100644 --- a/src/librustc_passes/consts.rs +++ b/src/librustc_passes/consts.rs @@ -336,9 +336,10 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node Def::VariantCtor(..) | Def::StructCtor(..) | Def::Fn(..) | Def::Method(..) => {} - // References to a static are inherently promotable, - // with the exception of "#[thread_loca]" statics. - // The latter may not outlive the current function + // References to a static that are themselves within a static + // are inherently promotable with the exception + // of "#[thread_loca]" statics, which may not + // outlive the current function Def::Static(did, _) => { if v.in_static {