From 2b25c0cd815f032b2c0b34d7d2316fbb20eda8de Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 14 Apr 2020 18:05:47 +0200 Subject: [PATCH 1/3] Don't load the same allocation twice when reading a scalar pair from it --- src/librustc_mir/interpret/operand.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3741f31927e94..f5ae434d00cec 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -242,13 +242,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } }; + let alloc = self.memory.get_raw(ptr.alloc_id)?; + match mplace.layout.abi { Abi::Scalar(..) => { - let scalar = self.memory.get_raw(ptr.alloc_id)?.read_scalar( - self, - ptr, - mplace.layout.size, - )?; + let scalar = alloc.read_scalar(self, ptr, mplace.layout.size)?; Ok(Some(ImmTy { imm: scalar.into(), layout: mplace.layout })) } Abi::ScalarPair(ref a, ref b) => { @@ -261,8 +259,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let b_offset = a_size.align_to(b.align(self).abi); assert!(b_offset.bytes() > 0); // we later use the offset to tell apart the fields let b_ptr = ptr.offset(b_offset, self)?; - let a_val = self.memory.get_raw(ptr.alloc_id)?.read_scalar(self, a_ptr, a_size)?; - let b_val = self.memory.get_raw(ptr.alloc_id)?.read_scalar(self, b_ptr, b_size)?; + let a_val = alloc.read_scalar(self, a_ptr, a_size)?; + let b_val = alloc.read_scalar(self, b_ptr, b_size)?; Ok(Some(ImmTy { imm: Immediate::ScalarPair(a_val, b_val), layout: mplace.layout })) } _ => Ok(None), From af44cdf04fddf4d18efee49c0c683cb4bbce71fa Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 23 Apr 2020 12:53:05 +0200 Subject: [PATCH 2/3] Disallow statics initializing themselves --- src/librustc_mir/interpret/memory.rs | 11 +++++++++- src/test/ui/consts/recursive-zst-static.rs | 8 +++++-- .../ui/consts/recursive-zst-static.stderr | 21 +++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/consts/recursive-zst-static.stderr diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index bcad7855c3736..43b2f0e91102d 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -400,7 +400,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We can still be zero-sized in this branch, in which case we have to // return `None`. - if size.bytes() == 0 { None } else { Some(ptr) } + if size.bytes() == 0 { + // We may be reading from a static. + // In order to ensure that `static FOO: Type = FOO;` causes a cycle error + // instead of magically pulling *any* ZST value from the ether, we need to + // trigger a read here. + self.get_raw(ptr.alloc_id)?; + None + } else { + Some(ptr) + } } }) } diff --git a/src/test/ui/consts/recursive-zst-static.rs b/src/test/ui/consts/recursive-zst-static.rs index df7562bd9f5d2..768df58e1e32e 100644 --- a/src/test/ui/consts/recursive-zst-static.rs +++ b/src/test/ui/consts/recursive-zst-static.rs @@ -1,6 +1,10 @@ -// build-pass +// This test ensures that we do not allow ZST statics to initialize themselves without ever +// actually creating a value of that type. This is important, as the ZST may have private fields +// that users can reasonably expect to only get initialized by their own code. Thus unsafe code +// can depend on this fact and will thus do unsound things when it is violated. +// See https://github.com/rust-lang/rust/issues/71078 for more details. -static FOO: () = FOO; +static FOO: () = FOO; //~ cycle detected when const-evaluating `FOO` fn main() { FOO diff --git a/src/test/ui/consts/recursive-zst-static.stderr b/src/test/ui/consts/recursive-zst-static.stderr new file mode 100644 index 0000000000000..e21dcf691ab0a --- /dev/null +++ b/src/test/ui/consts/recursive-zst-static.stderr @@ -0,0 +1,21 @@ +error[E0391]: cycle detected when const-evaluating `FOO` + --> $DIR/recursive-zst-static.rs:7:18 + | +LL | static FOO: () = FOO; + | ^^^ + | +note: ...which requires const-evaluating `FOO`... + --> $DIR/recursive-zst-static.rs:7:1 + | +LL | static FOO: () = FOO; + | ^^^^^^^^^^^^^^^^^^^^^ + = note: ...which again requires const-evaluating `FOO`, completing the cycle +note: cycle used when const-evaluating + checking `FOO` + --> $DIR/recursive-zst-static.rs:7:1 + | +LL | static FOO: () = FOO; + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0391`. From e4ab4ee020479d9312560e29056313ee9836c6d2 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 23 Apr 2020 13:24:50 +0200 Subject: [PATCH 3/3] Update src/librustc_mir/interpret/memory.rs Co-Authored-By: Ralf Jung --- src/librustc_mir/interpret/memory.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 43b2f0e91102d..6f8b0c1445a16 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -404,7 +404,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We may be reading from a static. // In order to ensure that `static FOO: Type = FOO;` causes a cycle error // instead of magically pulling *any* ZST value from the ether, we need to - // trigger a read here. + // actually access the referenced allocation. The caller is likely + // to short-circuit on `None`, so we trigger the access here to + // make sure it happens. self.get_raw(ptr.alloc_id)?; None } else {