Skip to content

Commit

Permalink
Auto merge of rust-lang#71665 - RalfJung:miri-intern-no-ice, r=oli-obk
Browse files Browse the repository at this point in the history
Miri interning: replace ICEs by proper errors

Fixes rust-lang#71316

I also did some refactoring, as I kept being confused by all the parameters to `intern_shallow`, some of which have invalid combinations (such as a mutable const). So instead `InternMode` now contains all the information that is needed and invalid combinations are ruled out by the type system.

Also I removed interpreter errors from interning. We already ignored almost all errors, and the `ValidationFailure` errors that we handled separately actually cannot ever happen here. The only interpreter failure that was actually reachable was the UB on dangling pointers -- and arguably, a dangling raw pointer is not UB, so the error was not even correct. It's just that the rest of the compiler does not like "dangling" `AllocId`.

It should be possible to review the 3 commits separately.

r? @oli-obk
Cc @rust-lang/wg-const-eval
  • Loading branch information
bors committed May 16, 2020
2 parents 8453936 + e73ee41 commit 584252b
Show file tree
Hide file tree
Showing 23 changed files with 331 additions and 336 deletions.
2 changes: 1 addition & 1 deletion src/librustc_mir/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
intern_kind,
ret,
body.ignore_interior_mut_in_const_validation,
)?;
);

debug!("eval_body_using_ecx done: {:?}", *ret);
Ok(ret)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub(crate) fn const_caller_location(
let mut ecx = mk_eval_cx(tcx, DUMMY_SP, ty::ParamEnv::reveal_all(), false);

let loc_place = ecx.alloc_caller_location(file, line, col);
intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place, false).unwrap();
intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place, false);
ConstValue::Scalar(loc_place.ptr)
}

Expand Down
3 changes: 3 additions & 0 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Our result will later be validated anyway, and there seems no good reason
// to have to fail early here. This is also more consistent with
// `Memory::get_static_alloc` which has to use `const_eval_raw` to avoid cycles.
// FIXME: We can hit delay_span_bug if this is an invalid const, interning finds
// that problem, but we never run validation to show an error. Can we ensure
// this does not happen?
let val = self.tcx.const_eval_raw(param_env.and(gid))?;
self.raw_const_to_mplace(val)
}
Expand Down
330 changes: 177 additions & 153 deletions src/librustc_mir/interpret/intern.rs

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,8 +709,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
)) => l.is_bits() && r.is_bits(),
interpret::Operand::Indirect(_) if mir_opt_level >= 2 => {
let mplace = op.assert_mem_place(&self.ecx);
intern_const_alloc_recursive(&mut self.ecx, InternKind::ConstProp, mplace, false)
.expect("failed to intern alloc");
intern_const_alloc_recursive(&mut self.ecx, InternKind::ConstProp, mplace, false);
true
}
_ => false,
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/consts/dangling-alloc-id-ice.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// https://github.com/rust-lang/rust/issues/55223
#![allow(const_err)]

union Foo<'a> {
y: &'a (),
long_live_the_unit: &'static (),
}

const FOO: &() = { //~ ERROR any use of this value will cause an error
const FOO: &() = { //~ ERROR it is undefined behavior to use this value
//~^ ERROR encountered dangling pointer in final constant
let y = ();
unsafe { Foo { y: &y }.long_live_the_unit }
};
Expand Down
22 changes: 17 additions & 5 deletions src/test/ui/consts/dangling-alloc-id-ice.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
error: any use of this value will cause an error
--> $DIR/dangling-alloc-id-ice.rs:8:1
error: encountered dangling pointer in final constant
--> $DIR/dangling-alloc-id-ice.rs:9:1
|
LL | / const FOO: &() = {
LL | |
LL | | let y = ();
LL | | unsafe { Foo { y: &y }.long_live_the_unit }
LL | | };
| |__^ encountered dangling pointer in final constant
| |__^

error[E0080]: it is undefined behavior to use this value
--> $DIR/dangling-alloc-id-ice.rs:9:1
|
LL | / const FOO: &() = {
LL | |
LL | | let y = ();
LL | | unsafe { Foo { y: &y }.long_live_the_unit }
LL | | };
| |__^ type validation failed: encountered a dangling reference (use-after-free)
|
= note: `#[deny(const_err)]` on by default
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0080`.
2 changes: 1 addition & 1 deletion src/test/ui/consts/dangling_raw_ptr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const FOO: *const u32 = { //~ ERROR any use of this value will cause an error
const FOO: *const u32 = { //~ ERROR encountered dangling pointer in final constant
let x = 42;
&x
};
Expand Down
6 changes: 2 additions & 4 deletions src/test/ui/consts/dangling_raw_ptr.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
error: any use of this value will cause an error
error: encountered dangling pointer in final constant
--> $DIR/dangling_raw_ptr.rs:1:1
|
LL | / const FOO: *const u32 = {
LL | | let x = 42;
LL | | &x
LL | | };
| |__^ encountered dangling pointer in final constant
|
= note: `#[deny(const_err)]` on by default
| |__^

error: aborting due to previous error

25 changes: 0 additions & 25 deletions src/test/ui/consts/miri_unleashed/mutable_const.rs

This file was deleted.

39 changes: 0 additions & 39 deletions src/test/ui/consts/miri_unleashed/mutable_const.stderr

This file was deleted.

16 changes: 0 additions & 16 deletions src/test/ui/consts/miri_unleashed/mutable_const2.rs

This file was deleted.

29 changes: 0 additions & 29 deletions src/test/ui/consts/miri_unleashed/mutable_const2.stderr

This file was deleted.

3 changes: 1 addition & 2 deletions src/test/ui/consts/miri_unleashed/mutable_references.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ struct Foo<T>(T);
// this is fine for the same reason as `BAR`.
static BOO: &mut Foo<()> = &mut Foo(());

// interior mutability is fine
struct Meh {
x: &'static UnsafeCell<i32>,
}

unsafe impl Sync for Meh {}

static MEH: Meh = Meh {
x: &UnsafeCell::new(42),
};
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/consts/miri_unleashed/mutable_references.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0594]: cannot assign to `*OH_YES`, as `OH_YES` is an immutable static item
--> $DIR/mutable_references.rs:37:5
--> $DIR/mutable_references.rs:36:5
|
LL | *OH_YES = 99;
| ^^^^^^^^^^^^ cannot assign
Expand All @@ -22,12 +22,12 @@ help: skipping check for `const_mut_refs` feature
LL | static BOO: &mut Foo<()> = &mut Foo(());
| ^^^^^^^^^^^^
help: skipping check that does not even have a feature gate
--> $DIR/mutable_references.rs:27:8
--> $DIR/mutable_references.rs:26:8
|
LL | x: &UnsafeCell::new(42),
| ^^^^^^^^^^^^^^^^^^^^
help: skipping check for `const_mut_refs` feature
--> $DIR/mutable_references.rs:31:27
--> $DIR/mutable_references.rs:30:27
|
LL | static OH_YES: &mut i32 = &mut 42;
| ^^^^^^^
Expand Down
37 changes: 37 additions & 0 deletions src/test/ui/consts/miri_unleashed/mutable_references_err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// compile-flags: -Zunleash-the-miri-inside-of-you

#![allow(const_err)]

use std::cell::UnsafeCell;

// this test ensures that our mutability story is sound

struct Meh {
x: &'static UnsafeCell<i32>,
}
unsafe impl Sync for Meh {}

// the following will never be ok! no interior mut behind consts, because
// all allocs interned here will be marked immutable.
const MUH: Meh = Meh { //~ ERROR: mutable memory (`UnsafeCell`) is not allowed in constant
x: &UnsafeCell::new(42),
};

struct Synced {
x: UnsafeCell<i32>,
}
unsafe impl Sync for Synced {}

// Make sure we also catch this behind a type-erased `dyn Trait` reference.
const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
//~^ ERROR: mutable memory (`UnsafeCell`) is not allowed in constant

// Make sure we also catch mutable references.
const BLUNT: &mut i32 = &mut 42;
//~^ ERROR: mutable memory (`&mut`) is not allowed in constant

fn main() {
unsafe {
*MUH.x.get() = 99;
}
}
40 changes: 40 additions & 0 deletions src/test/ui/consts/miri_unleashed/mutable_references_err.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: mutable memory (`UnsafeCell`) is not allowed in constant
--> $DIR/mutable_references_err.rs:16:1
|
LL | / const MUH: Meh = Meh {
LL | | x: &UnsafeCell::new(42),
LL | | };
| |__^

error: mutable memory (`UnsafeCell`) is not allowed in constant
--> $DIR/mutable_references_err.rs:26:1
|
LL | const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: mutable memory (`&mut`) is not allowed in constant
--> $DIR/mutable_references_err.rs:30:1
|
LL | const BLUNT: &mut i32 = &mut 42;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: skipping const checks
|
help: skipping check that does not even have a feature gate
--> $DIR/mutable_references_err.rs:17:8
|
LL | x: &UnsafeCell::new(42),
| ^^^^^^^^^^^^^^^^^^^^
help: skipping check that does not even have a feature gate
--> $DIR/mutable_references_err.rs:26:27
|
LL | const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: skipping check for `const_mut_refs` feature
--> $DIR/mutable_references_err.rs:30:25
|
LL | const BLUNT: &mut i32 = &mut 42;
| ^^^^^^^

error: aborting due to 3 previous errors; 1 warning emitted

29 changes: 0 additions & 29 deletions src/test/ui/consts/miri_unleashed/mutable_references_ice.rs

This file was deleted.

25 changes: 0 additions & 25 deletions src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr

This file was deleted.

Loading

0 comments on commit 584252b

Please sign in to comment.