From 8c0f6011057b5b2f7bd4d74ce63caf8d6630c39e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 17 Aug 2019 12:57:44 +0200 Subject: [PATCH 1/6] fix Miri discriminant load/store when overflows are involved --- src/librustc_mir/interpret/operand.rs | 40 +++++++++++++++++++++---- src/librustc_mir/interpret/place.rs | 42 ++++++++++++++++++++------- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 9ad1542905b68..3b07202f898ba 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -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 @@ -682,8 +687,31 @@ 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); + // FIXME: WTF, some discriminants don't have integer type. + use layout::Primitive; + let discr_layout = self.layout_of(match discr_layout.value { + Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), + Primitive::Pointer => self.tcx.types.usize, + Primitive::Float(..) => bug!("there are no float discriminants"), + })?; + let discr_val = ImmTy::from_uint(raw_discr, discr_layout); + // We need to use machine arithmetic. + let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); + let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); + let adjusted_discr = self.binary_op( + mir::BinOp::Sub, + discr_val, + niche_start_val, + )?; + let adjusted_discr = self.binary_op( + mir::BinOp::Add, + adjusted_discr, + variants_start_val, + )?; + let adjusted_discr = adjusted_discr + .to_scalar()? + .assert_bits(discr_val.layout.size); + // Check if this is in the range that indicates an actual discriminant. if variants_start <= adjusted_discr && adjusted_discr <= variants_end { let index = adjusted_discr as usize; assert_eq!(index as u128, adjusted_discr); diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 1a28548618206..0a23c1222b4bf 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -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, IntegerExt +}; 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,32 @@ 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 + // FIXME: WTF, some discriminants don't have integer type. + use layout::Primitive; + let discr_layout = self.layout_of(match discr_layout.value { + Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), + Primitive::Pointer => self.tcx.types.usize, + Primitive::Float(..) => bug!("there are no float discriminants"), + })?; + + // We need to use machine arithmetic. + let variants_start = niche_variants.start().as_u32(); + let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); + let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); + let variant_index_val = ImmTy::from_uint(variant_index.as_u32(), discr_layout); + let niche_val = self.binary_op( + mir::BinOp::Sub, + variant_index_val, + variants_start_val, + )?; + let niche_val = self.binary_op( + mir::BinOp::Add, + niche_val, + niche_start_val, )?; + // Write result. + let niche_dest = self.place_field(dest, discr_index as u64)?; + self.write_immediate(*niche_val, niche_dest)?; } } } From b73f1a51dcd1011129dc5c5c55a7577134410dbc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 13:08:21 +0200 Subject: [PATCH 2/6] better and more consistent variable names --- src/librustc_mir/interpret/operand.rs | 16 ++++++++-------- src/librustc_mir/interpret/place.rs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3b07202f898ba..2a0bad2fa746c 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -698,28 +698,28 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // We need to use machine arithmetic. let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); - let adjusted_discr = self.binary_op( + let variant_index_relative_val = self.binary_op( mir::BinOp::Sub, discr_val, niche_start_val, )?; - let adjusted_discr = self.binary_op( + let variant_index_val = self.binary_op( mir::BinOp::Add, - adjusted_discr, + variant_index_relative_val, variants_start_val, )?; - let adjusted_discr = adjusted_discr + let variant_index = variant_index_val .to_scalar()? .assert_bits(discr_val.layout.size); // Check if this is in the range that indicates an actual discriminant. - if variants_start <= adjusted_discr && adjusted_discr <= variants_end { - let index = adjusted_discr as usize; - assert_eq!(index as u128, adjusted_discr); + if variants_start <= variant_index && variant_index <= variants_end { + let index = variant_index as usize; + assert_eq!(index as u128, variant_index); assert!(index < rval.layout.ty .ty_adt_def() .expect("tagged layout for non adt") .variants.len()); - (adjusted_discr, VariantIdx::from_usize(index)) + (variant_index, VariantIdx::from_usize(index)) } else { (dataful_variant.as_u32() as u128, dataful_variant) } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 0a23c1222b4bf..636b070a3d8f3 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1073,19 +1073,19 @@ where let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); let variant_index_val = ImmTy::from_uint(variant_index.as_u32(), discr_layout); - let niche_val = self.binary_op( + let variant_index_relative_val = self.binary_op( mir::BinOp::Sub, variant_index_val, variants_start_val, )?; - let niche_val = self.binary_op( + let discr_val = self.binary_op( mir::BinOp::Add, - niche_val, + variant_index_relative_val, niche_start_val, )?; // Write result. let niche_dest = self.place_field(dest, discr_index as u64)?; - self.write_immediate(*niche_val, niche_dest)?; + self.write_immediate(*discr_val, niche_dest)?; } } } From 75fe84f4fc764e0684e7fdafbe6139cc92082a44 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 13:15:23 +0200 Subject: [PATCH 3/6] factor getting the discriminant layout to a new method --- src/librustc/ty/layout.rs | 11 +++++++++++ src/librustc_mir/interpret/operand.rs | 10 ++-------- src/librustc_mir/interpret/place.rs | 11 ++--------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 5ec4754c4535b..e52feea1624c1 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -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. diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 2a0bad2fa746c..94fbd4ac7d7f3 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -5,7 +5,7 @@ use std::convert::TryInto; 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::{ @@ -687,13 +687,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dataful_variant.as_u32() as u128, dataful_variant) }, Ok(raw_discr) => { - // FIXME: WTF, some discriminants don't have integer type. - use layout::Primitive; - let discr_layout = self.layout_of(match discr_layout.value { - Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), - Primitive::Pointer => self.tcx.types.usize, - Primitive::Float(..) => bug!("there are no float discriminants"), - })?; + let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; let discr_val = ImmTy::from_uint(raw_discr, discr_layout); // We need to use machine arithmetic. let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 636b070a3d8f3..9a3d70144a586 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -9,7 +9,7 @@ use rustc::mir; use rustc::mir::interpret::truncate; use rustc::ty::{self, Ty}; use rustc::ty::layout::{ - self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, IntegerExt + self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt }; use rustc::ty::TypeFoldable; @@ -1060,14 +1060,7 @@ where variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(), ); if variant_index != dataful_variant { - // FIXME: WTF, some discriminants don't have integer type. - use layout::Primitive; - let discr_layout = self.layout_of(match discr_layout.value { - Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), - Primitive::Pointer => self.tcx.types.usize, - Primitive::Float(..) => bug!("there are no float discriminants"), - })?; - + let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; // We need to use machine arithmetic. let variants_start = niche_variants.start().as_u32(); let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); From 61a28d5c881d81cca9156527851911bb5aa5b818 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 15:04:33 +0200 Subject: [PATCH 4/6] do the variant idx computations on the host (non-overflowing) --- src/librustc_mir/interpret/operand.rs | 14 ++++++-------- src/librustc_mir/interpret/place.rs | 16 +++++++--------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 94fbd4ac7d7f3..9e7ae32f1eacf 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -687,24 +687,22 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dataful_variant.as_u32() as u128, dataful_variant) }, Ok(raw_discr) => { + // We need to use machine arithmetic to get the relative variant idx. let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; let discr_val = ImmTy::from_uint(raw_discr, discr_layout); - // We need to use machine arithmetic. let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); - let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); let variant_index_relative_val = self.binary_op( mir::BinOp::Sub, discr_val, niche_start_val, )?; - let variant_index_val = self.binary_op( - mir::BinOp::Add, - variant_index_relative_val, - variants_start_val, - )?; - let variant_index = variant_index_val + let variant_index_relative = variant_index_relative_val .to_scalar()? .assert_bits(discr_val.layout.size); + // 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"); // Check if this is in the range that indicates an actual discriminant. if variants_start <= variant_index && variant_index <= variants_end { let index = variant_index as usize; diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 9a3d70144a586..5f4903d61e3b2 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1060,17 +1060,15 @@ where variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(), ); if variant_index != dataful_variant { - let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; - // We need to use machine arithmetic. let variants_start = niche_variants.start().as_u32(); - let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); + 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`. + 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_val = ImmTy::from_uint(variant_index.as_u32(), discr_layout); - let variant_index_relative_val = self.binary_op( - mir::BinOp::Sub, - variant_index_val, - variants_start_val, - )?; + 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, From b21622cd6cc6c88cc6760e76ba1442aee1e2b9a8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 16:25:39 +0200 Subject: [PATCH 5/6] add test --- .../miri_unleashed/enum_discriminants.rs | 110 ++++++++++++++++++ .../miri_unleashed/enum_discriminants.stderr | 72 ++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 src/test/ui/consts/miri_unleashed/enum_discriminants.rs create mode 100644 src/test/ui/consts/miri_unleashed/enum_discriminants.stderr diff --git a/src/test/ui/consts/miri_unleashed/enum_discriminants.rs b/src/test/ui/consts/miri_unleashed/enum_discriminants.rs new file mode 100644 index 0000000000000..9f34fc73953a4 --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/enum_discriminants.rs @@ -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 { + 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:: { + unreachable!() + } + if let E2::V3 { .. } = E2::V3:: { + } else { + unreachable!() + } + + 0 +}; + +fn main() { + assert_eq!(OVERFLOW, 0); + assert_eq!(MORE_OVERFLOW, 0); +} diff --git a/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr b/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr new file mode 100644 index 0000000000000..8ca81ad22b72b --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr @@ -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) + From 1de533ac1a3bddd6bf61791f4ee6cf52cb186fdf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 19 Sep 2019 09:02:45 +0200 Subject: [PATCH 6/6] first determine if the variant is a niche-variant, then compute absolute variant --- src/librustc_mir/interpret/operand.rs | 29 ++++++++++++++------------- src/librustc_mir/interpret/place.rs | 3 ++- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 9e7ae32f1eacf..dd214c4a031f7 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -1,7 +1,7 @@ //! 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::{ @@ -671,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)) })?; @@ -687,7 +687,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dataful_variant.as_u32() as u128, dataful_variant) }, Ok(raw_discr) => { - // We need to use machine arithmetic to get the relative variant idx. + // 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); @@ -699,21 +700,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let variant_index_relative = variant_index_relative_val .to_scalar()? .assert_bits(discr_val.layout.size); - // 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"); // Check if this is in the range that indicates an actual discriminant. - if variants_start <= variant_index && variant_index <= variants_end { - let index = variant_index as usize; - assert_eq!(index as u128, variant_index); - assert!(index < rval.layout.ty + 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()); - (variant_index, 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) } }, } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 5f4903d61e3b2..c3660fb7a2e28 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1064,7 +1064,8 @@ where 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`. + // 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 =