From ddbe69a5b2f268042d7d68b0bb5d2cac0055519b Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Thu, 30 Jul 2020 18:33:34 +0800 Subject: [PATCH 1/5] Special treatment for dereferencing a borrow to a static definition --- src/librustc_mir/transform/promote_consts.rs | 28 ++++++++++++++++++-- src/test/ui/statics/static-promotion.rs | 15 +++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/statics/static-promotion.rs diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 59a8415ef96f0..5f62e19938b83 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -502,9 +502,33 @@ impl<'tcx> Validator<'_, 'tcx> { fn validate_place(&self, place: PlaceRef<'tcx>) -> Result<(), Unpromotable> { match place { PlaceRef { local, projection: [] } => self.validate_local(local), - PlaceRef { local: _, projection: [proj_base @ .., elem] } => { + PlaceRef { local, projection: [proj_base @ .., elem] } => { match *elem { - ProjectionElem::Deref | ProjectionElem::Downcast(..) => { + ProjectionElem::Deref => { + let mut not_promotable = true; + if let TempState::Defined { location, .. } = self.temps[local] { + let def_stmt = + self.body[location.block].statements.get(location.statement_index); + if let Some(Statement { + kind: + StatementKind::Assign(box (_, Rvalue::Use(Operand::Constant(c)))), + .. + }) = def_stmt + { + if let Some(did) = c.check_static_ptr(self.tcx) { + if let Some(hir::ConstContext::Static(..)) = self.const_kind { + if !self.tcx.is_thread_local_static(did) { + not_promotable = false; + } + } + } + } + } + if not_promotable { + return Err(Unpromotable); + } + } + ProjectionElem::Downcast(..) => { return Err(Unpromotable); } diff --git a/src/test/ui/statics/static-promotion.rs b/src/test/ui/statics/static-promotion.rs new file mode 100644 index 0000000000000..2d9237d11c929 --- /dev/null +++ b/src/test/ui/statics/static-promotion.rs @@ -0,0 +1,15 @@ +// run-pass + +struct A(&'static T); +struct B { + x: &'static T, +} +static C: A>> = { + A(&B { + x: &B { x: b"hi" as &[u8] }, + }) +}; + +fn main() { + assert_eq!(b"hi", C.0.x.x); +} From 3b2642ffa7eceb9d04113d34e8136bd4b4e429d5 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 31 Jul 2020 02:16:42 +0800 Subject: [PATCH 2/5] Add comments to explain the test case and the special treatment --- src/librustc_mir/transform/promote_consts.rs | 14 +++++++++++++- src/test/ui/statics/static-promotion.rs | 10 +++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 5f62e19938b83..688f48f32b120 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -506,6 +506,10 @@ impl<'tcx> Validator<'_, 'tcx> { match *elem { ProjectionElem::Deref => { let mut not_promotable = true; + // This is a special treatment for cases like *&STATIC where STATIC is a + // global static variable. + // This pattern is generated only when global static variables are directly + // accessed and is qualified for promotion safely. if let TempState::Defined { location, .. } = self.temps[local] { let def_stmt = self.body[location.block].statements.get(location.statement_index); @@ -517,7 +521,15 @@ impl<'tcx> Validator<'_, 'tcx> { { if let Some(did) = c.check_static_ptr(self.tcx) { if let Some(hir::ConstContext::Static(..)) = self.const_kind { - if !self.tcx.is_thread_local_static(did) { + // The `is_empty` predicate is introduced to exclude the case + // where the projection operations are [ .field, * ]. + // The reason is because promotion will be illegal if field + // accesses preceed the dereferencing. + // Discussion can be found at + // https://github.com/rust-lang/rust/pull/74945#discussion_r463063247 + // There may be opportunity for generalization, but this needs to be + // accounted for. + if proj_base.is_empty() && !self.tcx.is_thread_local_static(did) { not_promotable = false; } } diff --git a/src/test/ui/statics/static-promotion.rs b/src/test/ui/statics/static-promotion.rs index 2d9237d11c929..500af1e15e12e 100644 --- a/src/test/ui/statics/static-promotion.rs +++ b/src/test/ui/statics/static-promotion.rs @@ -1,4 +1,12 @@ -// run-pass +// check-pass + +// Use of global static variables in literal values should be allowed for +// promotion. +// This test is to demonstrate the issue raised in +// https://github.com/rust-lang/rust/issues/70584 + +// Literal values were previously promoted into local static values when +// other global static variables are used. struct A(&'static T); struct B { From db071746323029c634f246a50156192d16ab182e Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 31 Jul 2020 11:46:05 +0800 Subject: [PATCH 3/5] Remove a trailing space --- src/librustc_mir/transform/promote_consts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 688f48f32b120..ca79cb3cc0a9c 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -526,7 +526,7 @@ impl<'tcx> Validator<'_, 'tcx> { // The reason is because promotion will be illegal if field // accesses preceed the dereferencing. // Discussion can be found at - // https://github.com/rust-lang/rust/pull/74945#discussion_r463063247 + // https://github.com/rust-lang/rust/pull/74945#discussion_r463063247 // There may be opportunity for generalization, but this needs to be // accounted for. if proj_base.is_empty() && !self.tcx.is_thread_local_static(did) { From 4631579b000b006ed3eba7d3f80bbfece02579bc Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 31 Jul 2020 11:58:49 +0800 Subject: [PATCH 4/5] rustfmt --- src/librustc_mir/transform/promote_consts.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index ca79cb3cc0a9c..f68473480630b 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -529,7 +529,9 @@ impl<'tcx> Validator<'_, 'tcx> { // https://github.com/rust-lang/rust/pull/74945#discussion_r463063247 // There may be opportunity for generalization, but this needs to be // accounted for. - if proj_base.is_empty() && !self.tcx.is_thread_local_static(did) { + if proj_base.is_empty() + && !self.tcx.is_thread_local_static(did) + { not_promotable = false; } } From c5114549d74f6092517af6ea630ec5a26317ae93 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 31 Jul 2020 18:04:13 +0800 Subject: [PATCH 5/5] Add the proper tests --- src/test/ui/statics/static-promotion.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/test/ui/statics/static-promotion.rs b/src/test/ui/statics/static-promotion.rs index 500af1e15e12e..bd8910bdb3f3f 100644 --- a/src/test/ui/statics/static-promotion.rs +++ b/src/test/ui/statics/static-promotion.rs @@ -12,12 +12,23 @@ struct A(&'static T); struct B { x: &'static T, } +static STR: &'static [u8] = b"hi"; static C: A>> = { A(&B { - x: &B { x: b"hi" as &[u8] }, + x: &B { x: STR }, }) }; +pub struct Slice(&'static [i32]); + +static CONTENT: i32 = 42; +pub static CONTENT_MAP: Slice = Slice(&[CONTENT]); + +pub static FOO: (i32, i32) = (42, 43); +pub static CONTENT_MAP2: Slice = Slice(&[FOO.0]); + fn main() { assert_eq!(b"hi", C.0.x.x); + assert_eq!(&[42], CONTENT_MAP.0); + assert_eq!(&[42], CONTENT_MAP2.0); }