Skip to content

Commit

Permalink
Rollup merge of #90895 - RalfJung:read-discriminant-valid, r=oli-obk
Browse files Browse the repository at this point in the history
require full validity when determining the discriminant of a value

This resolves (for now) the semantic question that came up in #89764: arguably, reading the discriminant of a value is 'using' that value, so we are in our right to demand full validity. Reading a discriminant is somewhat special in that it works for values of *arbitrary* type; all the other primitive MIR operations work on specific types (e.g. `bool` or an integer) and basically implicitly require validity as part of just "doing their job".

The alternative would be to just require that the discriminant itself is valid, if any -- but then what do we do for types that do not have a discriminant, which kind of validity do we check? [This code](https://github.com/rust-lang/rust/blob/81117ff930fbf3792b4f9504e3c6bccc87b10823/compiler/rustc_codegen_ssa/src/mir/place.rs#L206-L215) means we have to at least reject uninhabited types, but I would rather not special case that.

I don't think this can be tested in CTFE (since validity is not enforced there), I will add a compile-fail test to Miri:
```rust
#[allow(enum_intrinsics_non_enums)]
fn main() {
    let i = 2u8;
    std::mem::discriminant(unsafe { &*(&i as *const _ as *const bool) }); // UB
}
```

(I tried running the check even on the CTFE machines, but then it runs during ConstProp and that causes all sorts of problems. We could run it for ConstEval but not ConstProp, but that simply does not seem worth the effort currently.)

r? ``@oli-obk``
  • Loading branch information
JohnTitor authored Nov 18, 2021
2 parents 47c1bd1 + 498ebc4 commit 0a2b7d7
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
sym::discriminant_value => {
let place = self.deref_operand(&args[0])?;
if M::enforce_validity(self) {
// This is 'using' the value, so make sure the validity invariant is satisfied.
// (Also see https://github.com/rust-lang/rust/pull/89764.)
self.validate_operand(&place.into())?;
}

let discr_val = self.read_discriminant(&place.into())?.0;
self.write_scalar(discr_val, dest)?;
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

Discriminant(place) => {
let op = self.eval_place_to_op(place, None)?;
if M::enforce_validity(self) {
// This is 'using' the value, so make sure the validity invariant is satisfied.
// (Also see https://github.com/rust-lang/rust/pull/89764.)
self.validate_operand(&op)?;
}

let discr_val = self.read_discriminant(&op)?.0;
self.write_scalar(discr_val, &dest)?;
}
Expand Down

0 comments on commit 0a2b7d7

Please sign in to comment.