Rollup merge of #63448 - RalfJung:miri-discriminant, r=eddyb
fix Miri discriminant handling This can be reviewed commit-by-commit fairly well. The Miri side is at https://github.com/rust-lang/miri/pull/903. Fixes https://github.com/rust-lang/rust/issues/62138 r? @eddyb @oli-obk
This commit is contained in:
commit
bda52e558e
@ -127,6 +127,7 @@ impl IntegerExt for Integer {
|
||||
|
||||
pub trait PrimitiveExt {
|
||||
fn to_ty<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx>;
|
||||
fn to_int_ty<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx>;
|
||||
}
|
||||
|
||||
impl PrimitiveExt for Primitive {
|
||||
@ -138,6 +139,16 @@ impl PrimitiveExt for Primitive {
|
||||
Pointer => tcx.mk_mut_ptr(tcx.mk_unit()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Return an *integer* type matching this primitive.
|
||||
/// Useful in particular when dealing with enum discriminants.
|
||||
fn to_int_ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
|
||||
match *self {
|
||||
Int(i, signed) => i.to_ty(tcx, signed),
|
||||
Pointer => tcx.types.usize,
|
||||
Float(..) => bug!("floats do not have an int type"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The first half of a fat pointer.
|
||||
|
@ -1,11 +1,11 @@
|
||||
//! Functions concerning immediate values and operands, and reading from operands.
|
||||
//! All high-level functions to read from memory work on operands as sources.
|
||||
|
||||
use std::convert::TryInto;
|
||||
use std::convert::{TryInto, TryFrom};
|
||||
|
||||
use rustc::{mir, ty};
|
||||
use rustc::ty::layout::{
|
||||
self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx,
|
||||
self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, PrimitiveExt, VariantIdx,
|
||||
};
|
||||
|
||||
use rustc::mir::interpret::{
|
||||
@ -609,15 +609,20 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
|
||||
) -> InterpResult<'tcx, (u128, VariantIdx)> {
|
||||
trace!("read_discriminant_value {:#?}", rval.layout);
|
||||
|
||||
let (discr_kind, discr_index) = match rval.layout.variants {
|
||||
let (discr_layout, discr_kind, discr_index) = match rval.layout.variants {
|
||||
layout::Variants::Single { index } => {
|
||||
let discr_val = rval.layout.ty.discriminant_for_variant(*self.tcx, index).map_or(
|
||||
index.as_u32() as u128,
|
||||
|discr| discr.val);
|
||||
return Ok((discr_val, index));
|
||||
}
|
||||
layout::Variants::Multiple { ref discr_kind, discr_index, .. } =>
|
||||
(discr_kind, discr_index),
|
||||
layout::Variants::Multiple {
|
||||
discr: ref discr_layout,
|
||||
ref discr_kind,
|
||||
discr_index,
|
||||
..
|
||||
} =>
|
||||
(discr_layout, discr_kind, discr_index),
|
||||
};
|
||||
|
||||
// read raw discriminant value
|
||||
@ -634,7 +639,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
|
||||
.map_err(|_| err_unsup!(InvalidDiscriminant(raw_discr.erase_tag())))?;
|
||||
let real_discr = if discr_val.layout.ty.is_signed() {
|
||||
// going from layout tag type to typeck discriminant type
|
||||
// requires first sign extending with the layout discriminant
|
||||
// requires first sign extending with the discriminant layout
|
||||
let sexted = sign_extend(bits_discr, discr_val.layout.size) as i128;
|
||||
// and then zeroing with the typeck discriminant type
|
||||
let discr_ty = rval.layout.ty
|
||||
@ -666,8 +671,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
|
||||
ref niche_variants,
|
||||
niche_start,
|
||||
} => {
|
||||
let variants_start = niche_variants.start().as_u32() as u128;
|
||||
let variants_end = niche_variants.end().as_u32() as u128;
|
||||
let variants_start = niche_variants.start().as_u32();
|
||||
let variants_end = niche_variants.end().as_u32();
|
||||
let raw_discr = raw_discr.not_undef().map_err(|_| {
|
||||
err_unsup!(InvalidDiscriminant(ScalarMaybeUndef::Undef))
|
||||
})?;
|
||||
@ -682,18 +687,34 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
|
||||
(dataful_variant.as_u32() as u128, dataful_variant)
|
||||
},
|
||||
Ok(raw_discr) => {
|
||||
let adjusted_discr = raw_discr.wrapping_sub(niche_start)
|
||||
.wrapping_add(variants_start);
|
||||
if variants_start <= adjusted_discr && adjusted_discr <= variants_end {
|
||||
let index = adjusted_discr as usize;
|
||||
assert_eq!(index as u128, adjusted_discr);
|
||||
assert!(index < rval.layout.ty
|
||||
// We need to use machine arithmetic to get the relative variant idx:
|
||||
// variant_index_relative = discr_val - niche_start_val
|
||||
let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?;
|
||||
let discr_val = ImmTy::from_uint(raw_discr, discr_layout);
|
||||
let niche_start_val = ImmTy::from_uint(niche_start, discr_layout);
|
||||
let variant_index_relative_val = self.binary_op(
|
||||
mir::BinOp::Sub,
|
||||
discr_val,
|
||||
niche_start_val,
|
||||
)?;
|
||||
let variant_index_relative = variant_index_relative_val
|
||||
.to_scalar()?
|
||||
.assert_bits(discr_val.layout.size);
|
||||
// Check if this is in the range that indicates an actual discriminant.
|
||||
if variant_index_relative <= u128::from(variants_end - variants_start) {
|
||||
let variant_index_relative = u32::try_from(variant_index_relative)
|
||||
.expect("we checked that this fits into a u32");
|
||||
// Then computing the absolute variant idx should not overflow any more.
|
||||
let variant_index = variants_start
|
||||
.checked_add(variant_index_relative)
|
||||
.expect("oveflow computing absolute variant idx");
|
||||
assert!((variant_index as usize) < rval.layout.ty
|
||||
.ty_adt_def()
|
||||
.expect("tagged layout for non adt")
|
||||
.variants.len());
|
||||
(adjusted_discr, VariantIdx::from_usize(index))
|
||||
(u128::from(variant_index), VariantIdx::from_u32(variant_index))
|
||||
} else {
|
||||
(dataful_variant.as_u32() as u128, dataful_variant)
|
||||
(u128::from(dataful_variant.as_u32()), dataful_variant)
|
||||
}
|
||||
},
|
||||
}
|
||||
|
@ -8,7 +8,9 @@ use std::hash::Hash;
|
||||
use rustc::mir;
|
||||
use rustc::mir::interpret::truncate;
|
||||
use rustc::ty::{self, Ty};
|
||||
use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx};
|
||||
use rustc::ty::layout::{
|
||||
self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt
|
||||
};
|
||||
use rustc::ty::TypeFoldable;
|
||||
|
||||
use super::{
|
||||
@ -1027,7 +1029,7 @@ where
|
||||
}
|
||||
layout::Variants::Multiple {
|
||||
discr_kind: layout::DiscriminantKind::Tag,
|
||||
ref discr,
|
||||
discr: ref discr_layout,
|
||||
discr_index,
|
||||
..
|
||||
} => {
|
||||
@ -1038,7 +1040,7 @@ where
|
||||
// raw discriminants for enums are isize or bigger during
|
||||
// their computation, but the in-memory tag is the smallest possible
|
||||
// representation
|
||||
let size = discr.value.size(self);
|
||||
let size = discr_layout.value.size(self);
|
||||
let discr_val = truncate(discr_val, size);
|
||||
|
||||
let discr_dest = self.place_field(dest, discr_index as u64)?;
|
||||
@ -1050,6 +1052,7 @@ where
|
||||
ref niche_variants,
|
||||
niche_start,
|
||||
},
|
||||
discr: ref discr_layout,
|
||||
discr_index,
|
||||
..
|
||||
} => {
|
||||
@ -1057,15 +1060,24 @@ where
|
||||
variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(),
|
||||
);
|
||||
if variant_index != dataful_variant {
|
||||
let niche_dest =
|
||||
self.place_field(dest, discr_index as u64)?;
|
||||
let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
|
||||
let niche_value = (niche_value as u128)
|
||||
.wrapping_add(niche_start);
|
||||
self.write_scalar(
|
||||
Scalar::from_uint(niche_value, niche_dest.layout.size),
|
||||
niche_dest
|
||||
let variants_start = niche_variants.start().as_u32();
|
||||
let variant_index_relative = variant_index.as_u32()
|
||||
.checked_sub(variants_start)
|
||||
.expect("overflow computing relative variant idx");
|
||||
// We need to use machine arithmetic when taking into account `niche_start`:
|
||||
// discr_val = variant_index_relative + niche_start_val
|
||||
let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?;
|
||||
let niche_start_val = ImmTy::from_uint(niche_start, discr_layout);
|
||||
let variant_index_relative_val =
|
||||
ImmTy::from_uint(variant_index_relative, discr_layout);
|
||||
let discr_val = self.binary_op(
|
||||
mir::BinOp::Add,
|
||||
variant_index_relative_val,
|
||||
niche_start_val,
|
||||
)?;
|
||||
// Write result.
|
||||
let niche_dest = self.place_field(dest, discr_index as u64)?;
|
||||
self.write_immediate(*discr_val, niche_dest)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
110
src/test/ui/consts/miri_unleashed/enum_discriminants.rs
Normal file
110
src/test/ui/consts/miri_unleashed/enum_discriminants.rs
Normal file
@ -0,0 +1,110 @@
|
||||
// compile-flags: -Zunleash-the-miri-inside-of-you
|
||||
// run-pass
|
||||
|
||||
//! Make sure that we read and write enum discriminants correctly for corner cases caused
|
||||
//! by layout optimizations.
|
||||
|
||||
const OVERFLOW: usize = {
|
||||
// Tests for https://github.com/rust-lang/rust/issues/62138.
|
||||
#[repr(u8)]
|
||||
#[allow(dead_code)]
|
||||
enum WithWraparoundInvalidValues {
|
||||
X = 1,
|
||||
Y = 254,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
enum Foo {
|
||||
A,
|
||||
B,
|
||||
C(WithWraparoundInvalidValues),
|
||||
}
|
||||
|
||||
let x = Foo::B;
|
||||
match x {
|
||||
Foo::B => 0,
|
||||
_ => panic!(),
|
||||
}
|
||||
};
|
||||
|
||||
const MORE_OVERFLOW: usize = {
|
||||
pub enum Infallible {}
|
||||
|
||||
// The check that the `bool` field of `V1` is encoding a "niche variant"
|
||||
// (i.e. not `V1`, so `V3` or `V4`) used to be mathematically incorrect,
|
||||
// causing valid `V1` values to be interpreted as other variants.
|
||||
#[allow(dead_code)]
|
||||
pub enum E1 {
|
||||
V1 { f: bool },
|
||||
V2 { f: Infallible },
|
||||
V3,
|
||||
V4,
|
||||
}
|
||||
|
||||
// Computing the discriminant used to be done using the niche type (here `u8`,
|
||||
// from the `bool` field of `V1`), overflowing for variants with large enough
|
||||
// indices (`V3` and `V4`), causing them to be interpreted as other variants.
|
||||
#[allow(dead_code)]
|
||||
pub enum E2<X> {
|
||||
V1 { f: bool },
|
||||
|
||||
/*_00*/ _01(X), _02(X), _03(X), _04(X), _05(X), _06(X), _07(X),
|
||||
_08(X), _09(X), _0A(X), _0B(X), _0C(X), _0D(X), _0E(X), _0F(X),
|
||||
_10(X), _11(X), _12(X), _13(X), _14(X), _15(X), _16(X), _17(X),
|
||||
_18(X), _19(X), _1A(X), _1B(X), _1C(X), _1D(X), _1E(X), _1F(X),
|
||||
_20(X), _21(X), _22(X), _23(X), _24(X), _25(X), _26(X), _27(X),
|
||||
_28(X), _29(X), _2A(X), _2B(X), _2C(X), _2D(X), _2E(X), _2F(X),
|
||||
_30(X), _31(X), _32(X), _33(X), _34(X), _35(X), _36(X), _37(X),
|
||||
_38(X), _39(X), _3A(X), _3B(X), _3C(X), _3D(X), _3E(X), _3F(X),
|
||||
_40(X), _41(X), _42(X), _43(X), _44(X), _45(X), _46(X), _47(X),
|
||||
_48(X), _49(X), _4A(X), _4B(X), _4C(X), _4D(X), _4E(X), _4F(X),
|
||||
_50(X), _51(X), _52(X), _53(X), _54(X), _55(X), _56(X), _57(X),
|
||||
_58(X), _59(X), _5A(X), _5B(X), _5C(X), _5D(X), _5E(X), _5F(X),
|
||||
_60(X), _61(X), _62(X), _63(X), _64(X), _65(X), _66(X), _67(X),
|
||||
_68(X), _69(X), _6A(X), _6B(X), _6C(X), _6D(X), _6E(X), _6F(X),
|
||||
_70(X), _71(X), _72(X), _73(X), _74(X), _75(X), _76(X), _77(X),
|
||||
_78(X), _79(X), _7A(X), _7B(X), _7C(X), _7D(X), _7E(X), _7F(X),
|
||||
_80(X), _81(X), _82(X), _83(X), _84(X), _85(X), _86(X), _87(X),
|
||||
_88(X), _89(X), _8A(X), _8B(X), _8C(X), _8D(X), _8E(X), _8F(X),
|
||||
_90(X), _91(X), _92(X), _93(X), _94(X), _95(X), _96(X), _97(X),
|
||||
_98(X), _99(X), _9A(X), _9B(X), _9C(X), _9D(X), _9E(X), _9F(X),
|
||||
_A0(X), _A1(X), _A2(X), _A3(X), _A4(X), _A5(X), _A6(X), _A7(X),
|
||||
_A8(X), _A9(X), _AA(X), _AB(X), _AC(X), _AD(X), _AE(X), _AF(X),
|
||||
_B0(X), _B1(X), _B2(X), _B3(X), _B4(X), _B5(X), _B6(X), _B7(X),
|
||||
_B8(X), _B9(X), _BA(X), _BB(X), _BC(X), _BD(X), _BE(X), _BF(X),
|
||||
_C0(X), _C1(X), _C2(X), _C3(X), _C4(X), _C5(X), _C6(X), _C7(X),
|
||||
_C8(X), _C9(X), _CA(X), _CB(X), _CC(X), _CD(X), _CE(X), _CF(X),
|
||||
_D0(X), _D1(X), _D2(X), _D3(X), _D4(X), _D5(X), _D6(X), _D7(X),
|
||||
_D8(X), _D9(X), _DA(X), _DB(X), _DC(X), _DD(X), _DE(X), _DF(X),
|
||||
_E0(X), _E1(X), _E2(X), _E3(X), _E4(X), _E5(X), _E6(X), _E7(X),
|
||||
_E8(X), _E9(X), _EA(X), _EB(X), _EC(X), _ED(X), _EE(X), _EF(X),
|
||||
_F0(X), _F1(X), _F2(X), _F3(X), _F4(X), _F5(X), _F6(X), _F7(X),
|
||||
_F8(X), _F9(X), _FA(X), _FB(X), _FC(X), _FD(X), _FE(X), _FF(X),
|
||||
|
||||
V3,
|
||||
V4,
|
||||
}
|
||||
|
||||
if let E1::V2 { .. } = (E1::V1 { f: true }) {
|
||||
unreachable!()
|
||||
}
|
||||
if let E1::V1 { .. } = (E1::V1 { f: true }) {
|
||||
} else {
|
||||
unreachable!()
|
||||
}
|
||||
|
||||
if let E2::V1 { .. } = E2::V3::<Infallible> {
|
||||
unreachable!()
|
||||
}
|
||||
if let E2::V3 { .. } = E2::V3::<Infallible> {
|
||||
} else {
|
||||
unreachable!()
|
||||
}
|
||||
|
||||
0
|
||||
};
|
||||
|
||||
fn main() {
|
||||
assert_eq!(OVERFLOW, 0);
|
||||
assert_eq!(MORE_OVERFLOW, 0);
|
||||
}
|
72
src/test/ui/consts/miri_unleashed/enum_discriminants.stderr
Normal file
72
src/test/ui/consts/miri_unleashed/enum_discriminants.stderr
Normal file
@ -0,0 +1,72 @@
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:23:13
|
||||
|
|
||||
LL | let x = Foo::B;
|
||||
| ^^^^^^
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:25:9
|
||||
|
|
||||
LL | Foo::B => 0,
|
||||
| ^^^^^^
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:88:28
|
||||
|
|
||||
LL | if let E1::V2 { .. } = (E1::V1 { f: true }) {
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:88:12
|
||||
|
|
||||
LL | if let E1::V2 { .. } = (E1::V1 { f: true }) {
|
||||
| ^^^^^^^^^^^^^
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:108:5
|
||||
|
|
||||
LL | assert_eq!(OVERFLOW, 0);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:108:5
|
||||
|
|
||||
LL | assert_eq!(OVERFLOW, 0);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:108:5
|
||||
|
|
||||
LL | assert_eq!(OVERFLOW, 0);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:109:5
|
||||
|
|
||||
LL | assert_eq!(MORE_OVERFLOW, 0);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:109:5
|
||||
|
|
||||
LL | assert_eq!(MORE_OVERFLOW, 0);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
|
||||
|
||||
warning: skipping const checks
|
||||
--> $DIR/enum_discriminants.rs:109:5
|
||||
|
|
||||
LL | assert_eq!(MORE_OVERFLOW, 0);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
|
||||
|
Loading…
x
Reference in New Issue
Block a user