Skip to content

Commit

Permalink
Auto merge of #51110 - alexreg:new-static-eval-rules, r=eddyb
Browse files Browse the repository at this point in the history
Loosened rules involving statics mentioning other statics

Before this PR, trying to mention a static in any way other than taking a reference to it caused a compile-time error. So, while

```rust
static A: u32 = 42;
static B: &u32 = &A;
```

compiles successfully,

```rust
static A: u32 = 42;
static B: u32 = A; // error
```

and

```rust
static A: u32 = 42;
static B: u32 = *&A; // error
```

are not possible to express in Rust. On the other hand, introducing an intermediate `const fn` can presently allow one to do just that:

```rust
static A: u32 = 42;
static B: u32 = foo(&A); // success!

const fn foo(a: &u32) -> u32 {
    *a
}
```

Preventing `const fn` from allowing to work around the ban on reading from statics would cripple `const fn` almost into uselessness.
Additionally, the limitation for reading from statics comes from the old const evaluator(s) and is not shared by `miri`.

This PR loosens the rules around use of statics to allow statics to evaluate other statics by value, allowing all of the above examples to compile and run successfully.
Reads from extern (foreign) statics are however still disallowed by miri, because there is no compile-time value to be read.

```rust
extern static A: u32;

static B: u32 = A; // error
```

This opens up a new avenue of potential issues, as a static can now not just refer to other statics or read from other statics, but even contain references that point into itself.
While it might seem like this could cause subtle bugs like allowing a static to be initialized by its own value, this is inherently impossible in miri.
Reading from a static causes the `const_eval` query for that static to be invoked. Calling the `const_eval` query for a static while already inside the `const_eval` query of said static will cause cycle errors.
It is not possible to accidentally create a bug in miri that would enable initializing a static with itself, because the memory of the static *does not exist* while being initialized.
The memory is not uninitialized, it is not there. Thus any change that would accidentally allow reading from a not yet initialized static would cause ICEs.

Tests have been modified according to the new rules, and new tests have been added for writing to `static mut`s within definitions of statics (which needs to fail), and incremental compilation with complex/interlinking static definitions.
Note that incremental compilation did not need to be adjusted, because all of this was already possible before with workarounds (like intermediate `const fn`s) and the encoding/decoding already supports all the possible cases.

r? @eddyb
  • Loading branch information
bors committed Jul 1, 2018
2 parents a7a60dc + 1c34227 commit c697a56
Show file tree
Hide file tree
Showing 25 changed files with 127 additions and 236 deletions.
1 change: 1 addition & 0 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
InvalidNullPointerUsage |
ReadPointerAsBytes |
ReadBytesAsPointer |
ReadForeignStatic |
InvalidPointerMath |
ReadUndefBytes |
DeadLocal |
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ pub enum EvalErrorKind<'tcx, O> {
InvalidNullPointerUsage,
ReadPointerAsBytes,
ReadBytesAsPointer,
ReadForeignStatic,
InvalidPointerMath,
ReadUndefBytes,
DeadLocal,
Expand Down Expand Up @@ -304,6 +305,8 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
"a raw memory access tried to access part of a pointer value as raw bytes",
ReadBytesAsPointer =>
"a memory access tried to interpret some bytes as a pointer",
ReadForeignStatic =>
"tried to read from foreign (extern) static",
InvalidPointerMath =>
"attempted to do invalid arithmetic on pointers that would leak base addresses, e.g. comparing pointers into different allocations",
ReadUndefBytes =>
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ impl<'a, 'tcx, O: Lift<'tcx>> Lift<'tcx> for interpret::EvalErrorKind<'a, O> {
InvalidNullPointerUsage => InvalidNullPointerUsage,
ReadPointerAsBytes => ReadPointerAsBytes,
ReadBytesAsPointer => ReadBytesAsPointer,
ReadForeignStatic => ReadForeignStatic,
InvalidPointerMath => InvalidPointerMath,
ReadUndefBytes => ReadUndefBytes,
DeadLocal => DeadLocal,
Expand Down
55 changes: 0 additions & 55 deletions src/librustc_mir/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1145,33 +1145,6 @@ fn main() {
```
"##,

E0394: r##"
A static was referred to by value by another static.
Erroneous code examples:
```compile_fail,E0394
static A: u32 = 0;
static B: u32 = A; // error: cannot refer to other statics by value, use the
// address-of operator or a constant instead
```
A static cannot be referred by value. To fix this issue, either use a
constant:
```
const A: u32 = 0; // `A` is now a constant
static B: u32 = A; // ok!
```
Or refer to `A` by reference:
```
static A: u32 = 0;
static B: &'static u32 = &A; // ok!
```
"##,

E0395: r##"
The value assigned to a constant scalar must be known at compile time,
which is not the case when comparing raw pointers.
Expand Down Expand Up @@ -1333,34 +1306,6 @@ Remember this solution is unsafe! You will have to ensure that accesses to the
cell are synchronized.
"##,

E0494: r##"
A reference of an interior static was assigned to another const/static.
Erroneous code example:
```compile_fail,E0494
struct Foo {
a: u32
}
static S : Foo = Foo { a : 0 };
static A : &'static u32 = &S.a;
// error: cannot refer to the interior of another static, use a
// constant instead
```
The "base" variable has to be a const if you want another static/const variable
to refer to one of its fields. Example:
```
struct Foo {
a: u32
}
const S : Foo = Foo { a : 0 };
static A : &'static u32 = &S.a; // ok!
```
"##,

E0499: r##"
A variable was borrowed as mutable more than once. Erroneous code example:
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ impl<'mir, 'tcx> super::Machine<'mir, 'tcx> for CompileTimeEvaluator {
Ok(None)
} else {
Err(
ConstEvalError::NeedsRfc("Pointer arithmetic or comparison".to_string()).into(),
ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
)
}
}
Expand Down Expand Up @@ -404,7 +404,7 @@ impl<'mir, 'tcx> super::Machine<'mir, 'tcx> for CompileTimeEvaluator {
_dest: Place,
) -> EvalResult<'tcx> {
Err(
ConstEvalError::NeedsRfc("Heap allocations via `box` keyword".to_string()).into(),
ConstEvalError::NeedsRfc("heap allocations via `box` keyword".to_string()).into(),
)
}

Expand Down
6 changes: 5 additions & 1 deletion src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
/// Allocation accessors
impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
fn const_eval_static(&self, def_id: DefId) -> EvalResult<'tcx, &'tcx Allocation> {
if self.tcx.is_foreign_item(def_id) {
return err!(ReadForeignStatic);
}
let instance = Instance::mono(self.tcx.tcx, def_id);
let gid = GlobalId {
instance,
Expand All @@ -302,7 +305,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
Some(alloc) => Ok(alloc),
None => {
// static alloc?
match self.tcx.alloc_map.lock().get(id) {
let alloc = self.tcx.alloc_map.lock().get(id);
match alloc {
Some(AllocType::Memory(mem)) => Ok(mem),
Some(AllocType::Function(..)) => {
Err(EvalErrorKind::DerefFunctionPointer.into())
Expand Down
Loading

0 comments on commit c697a56

Please sign in to comment.