Skip to content

Commit

Permalink
Rollup merge of rust-lang#65485 - ecstatic-morse:const-validation-mis…
Browse files Browse the repository at this point in the history
…match-ugliness, r=eddyb

Suppress ICE when validators disagree on `LiveDrop`s in presence of `&mut`

Resolves rust-lang#65394.

This hack disables the validator mismatch ICE in cases where a `MutBorrow` error has been emitted by both validators, but they don't agree on the number of `LiveDrop` errors.

The new validator is more conservative about whether a value is moved from in the presence of mutable borrows. For example, the new validator will emit a `LiveDrop` error on the following code.

```rust
const _: Vec<i32> = {
    let mut x = Vec::new();
    let px = &mut x as *mut _;
    let y = x;
    unsafe { ptr::write(px, Vec::new()); }
    y
};
```

This code is not UB AFAIK (it passes MIRI at least). The current validator does not emit a `LiveDrop` error for `x` upon exit from the initializer. `x` is not actually dropped, so I think this is correct? A proper fix for this would require a new `MaybeInitializedLocals` dataflow analysis or maybe a relaxation of the existing `IndirectlyMutableLocals` one.

r? @RalfJung
  • Loading branch information
Centril authored Oct 19, 2019
2 parents 0ace8e0 + af691de commit 6d6681a
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 17 deletions.
1 change: 1 addition & 0 deletions src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
#![feature(core_intrinsics)]
#![feature(const_fn)]
#![feature(decl_macro)]
#![feature(drain_filter)]
#![feature(exhaustive_patterns)]
#![feature(never_type)]
#![feature(specialization)]
Expand Down
75 changes: 58 additions & 17 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1024,23 +1024,12 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
new_errors.dedup();

if self.errors != new_errors {
error!("old validator: {:?}", self.errors);
error!("new validator: {:?}", new_errors);

// ICE on nightly if the validators do not emit exactly the same errors.
// Users can supress this panic with an unstable compiler flag (hopefully after
// filing an issue).
let opts = &self.tcx.sess.opts;
let trigger_ice = opts.unstable_features.is_nightly_build()
&& !opts.debugging_opts.suppress_const_validation_back_compat_ice;

if trigger_ice {
span_bug!(
body.span,
"{}",
VALIDATOR_MISMATCH_ERR,
);
}
validator_mismatch(
self.tcx,
body,
std::mem::replace(&mut self.errors, vec![]),
new_errors,
);
}
}

Expand Down Expand Up @@ -1870,6 +1859,58 @@ fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option<FxHashSet<usize
Some(ret)
}

fn validator_mismatch(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
mut old_errors: Vec<(Span, String)>,
mut new_errors: Vec<(Span, String)>,
) {
error!("old validator: {:?}", old_errors);
error!("new validator: {:?}", new_errors);

// ICE on nightly if the validators do not emit exactly the same errors.
// Users can supress this panic with an unstable compiler flag (hopefully after
// filing an issue).
let opts = &tcx.sess.opts;
let strict_validation_enabled = opts.unstable_features.is_nightly_build()
&& !opts.debugging_opts.suppress_const_validation_back_compat_ice;

if !strict_validation_enabled {
return;
}

// If this difference would cause a regression from the old to the new or vice versa, trigger
// the ICE.
if old_errors.is_empty() || new_errors.is_empty() {
span_bug!(body.span, "{}", VALIDATOR_MISMATCH_ERR);
}

// HACK: Borrows that would allow mutation are forbidden in const contexts, but they cause the
// new validator to be more conservative about when a dropped local has been moved out of.
//
// Supress the mismatch ICE in cases where the validators disagree only on the number of
// `LiveDrop` errors and both observe the same sequence of `MutBorrow`s.

let is_live_drop = |(_, s): &mut (_, String)| s.starts_with("LiveDrop");
let is_mut_borrow = |(_, s): &&(_, String)| s.starts_with("MutBorrow");

let old_live_drops: Vec<_> = old_errors.drain_filter(is_live_drop).collect();
let new_live_drops: Vec<_> = new_errors.drain_filter(is_live_drop).collect();

let only_live_drops_differ = old_live_drops != new_live_drops && old_errors == new_errors;

let old_mut_borrows = old_errors.iter().filter(is_mut_borrow);
let new_mut_borrows = new_errors.iter().filter(is_mut_borrow);

let at_least_one_mut_borrow = old_mut_borrows.clone().next().is_some();

if only_live_drops_differ && at_least_one_mut_borrow && old_mut_borrows.eq(new_mut_borrows) {
return;
}

span_bug!(body.span, "{}", VALIDATOR_MISMATCH_ERR);
}

const VALIDATOR_MISMATCH_ERR: &str =
r"Disagreement between legacy and dataflow-based const validators.
After filing an issue, use `-Zsuppress-const-validation-back-compat-ice` to compile your code.";
13 changes: 13 additions & 0 deletions src/test/ui/consts/const-eval/issue-65394.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Test for absence of validation mismatch ICE in #65394

#![feature(rustc_attrs)]

#[rustc_mir(borrowck_graphviz_postflow="hello.dot")]
const _: Vec<i32> = {
let mut x = Vec::<i32>::new();
let r = &mut x; //~ ERROR references in constants may only refer to immutable values
let y = x;
y
};

fn main() {}
11 changes: 11 additions & 0 deletions src/test/ui/consts/const-eval/issue-65394.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0017]: references in constants may only refer to immutable values
--> $DIR/issue-65394.rs:8:13
|
LL | let r = &mut x;
| ^^^^^^ constants require immutable values

[ERROR rustc_mir::transform::qualify_consts] old validator: [($DIR/issue-65394.rs:8:13: 8:19, "MutBorrow(Mut { allow_two_phase_borrow: false })")]
[ERROR rustc_mir::transform::qualify_consts] new validator: [($DIR/issue-65394.rs:8:13: 8:19, "MutBorrow(Mut { allow_two_phase_borrow: false })"), ($DIR/issue-65394.rs:7:9: 7:14, "LiveDrop")]
error: aborting due to previous error

For more information about this error, try `rustc --explain E0017`.

0 comments on commit 6d6681a

Please sign in to comment.