Skip to content

Commit

Permalink
Rollup merge of #71140 - oli-obk:static_cycle, r=RalfJung
Browse files Browse the repository at this point in the history
[breaking change] Disallow statics initializing themselves

fixes #71078

Self-initialization is unsound because it breaks privacy assumptions that unsafe code can make. In

```rust
pub mod foo {
    #[derive(Debug, Copy, Clone)]
    pub struct Foo {
        x: (),
    }
}

pub static FOO: foo::Foo = FOO;
```

unsafe could could expect that ony functions inside the `foo` module were able to create a value of type `Foo`.
  • Loading branch information
Dylan-DPC authored Apr 25, 2020
2 parents e51cbc8 + e4ab4ee commit b964451
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 10 deletions.
13 changes: 12 additions & 1 deletion src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,18 @@ 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
// 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 {
Some(ptr)
}
}
})
}
Expand Down
12 changes: 5 additions & 7 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,11 @@ impl<'mir, 'tcx: 'mir, 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) => {
Expand All @@ -267,8 +265,8 @@ impl<'mir, 'tcx: 'mir, 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),
Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/consts/recursive-zst-static.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/consts/recursive-zst-static.stderr
Original file line number Diff line number Diff line change
@@ -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`.

0 comments on commit b964451

Please sign in to comment.