Skip to content

Commit a5fb7fc

Browse files
authored
Rollup merge of #72419 - RalfJung:read-discriminant, r=oli-obk,eddyb
Miri read_discriminant: return a scalar instead of raw underlying bytes r? @oli-obk @eddyb
2 parents 3459eae + c4b6224 commit a5fb7fc

File tree

8 files changed

+129
-94
lines changed

8 files changed

+129
-94
lines changed

src/librustc_middle/mir/interpret/error.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::{AllocId, Pointer, RawConst, ScalarMaybeUninit};
1+
use super::{AllocId, Pointer, RawConst, Scalar};
22

33
use crate::mir::interpret::ConstValue;
44
use crate::ty::layout::LayoutError;
@@ -391,7 +391,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
391391
/// Using a non-character `u32` as character.
392392
InvalidChar(u32),
393393
/// An enum discriminant was set to a value which was outside the range of valid values.
394-
InvalidDiscriminant(ScalarMaybeUninit),
394+
InvalidDiscriminant(Scalar),
395395
/// Using a pointer-not-to-a-function as function pointer.
396396
InvalidFunctionPointer(Pointer),
397397
/// Using a string that is not valid UTF-8,

src/librustc_middle/mir/tcx.rs

+1-12
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
use crate::mir::*;
77
use crate::ty::subst::Subst;
8-
use crate::ty::util::IntTypeExt;
98
use crate::ty::{self, Ty, TyCtxt};
109
use rustc_hir as hir;
1110
use rustc_target::abi::VariantIdx;
@@ -174,17 +173,7 @@ impl<'tcx> Rvalue<'tcx> {
174173
tcx.intern_tup(&[ty, tcx.types.bool])
175174
}
176175
Rvalue::UnaryOp(UnOp::Not | UnOp::Neg, ref operand) => operand.ty(local_decls, tcx),
177-
Rvalue::Discriminant(ref place) => {
178-
let ty = place.ty(local_decls, tcx).ty;
179-
match ty.kind {
180-
ty::Adt(adt_def, _) => adt_def.repr.discr_type().to_ty(tcx),
181-
ty::Generator(_, substs, _) => substs.as_generator().discr_ty(tcx),
182-
_ => {
183-
// This can only be `0`, for now, so `u8` will suffice.
184-
tcx.types.u8
185-
}
186-
}
187-
}
176+
Rvalue::Discriminant(ref place) => place.ty(local_decls, tcx).ty.discriminant_ty(tcx),
188177
Rvalue::NullaryOp(NullOp::Box, t) => tcx.mk_box(t),
189178
Rvalue::NullaryOp(NullOp::SizeOf, _) => tcx.types.usize,
190179
Rvalue::Aggregate(ref ak, ref ops) => match **ak {

src/librustc_middle/ty/mod.rs

+5
Original file line numberDiff line numberDiff line change
@@ -2037,6 +2037,8 @@ impl ReprOptions {
20372037
self.flags.contains(ReprFlags::HIDE_NICHE)
20382038
}
20392039

2040+
/// Returns the discriminant type, given these `repr` options.
2041+
/// This must only be called on enums!
20402042
pub fn discr_type(&self) -> attr::IntType {
20412043
self.int.unwrap_or(attr::SignedInt(ast::IntTy::Isize))
20422044
}
@@ -2269,6 +2271,7 @@ impl<'tcx> AdtDef {
22692271

22702272
#[inline]
22712273
pub fn eval_explicit_discr(&self, tcx: TyCtxt<'tcx>, expr_did: DefId) -> Option<Discr<'tcx>> {
2274+
assert!(self.is_enum());
22722275
let param_env = tcx.param_env(expr_did);
22732276
let repr_type = self.repr.discr_type();
22742277
match tcx.const_eval_poly(expr_did) {
@@ -2305,6 +2308,7 @@ impl<'tcx> AdtDef {
23052308
&'tcx self,
23062309
tcx: TyCtxt<'tcx>,
23072310
) -> impl Iterator<Item = (VariantIdx, Discr<'tcx>)> + Captures<'tcx> {
2311+
assert!(self.is_enum());
23082312
let repr_type = self.repr.discr_type();
23092313
let initial = repr_type.initial_discriminant(tcx);
23102314
let mut prev_discr = None::<Discr<'tcx>>;
@@ -2337,6 +2341,7 @@ impl<'tcx> AdtDef {
23372341
tcx: TyCtxt<'tcx>,
23382342
variant_index: VariantIdx,
23392343
) -> Discr<'tcx> {
2344+
assert!(self.is_enum());
23402345
let (val, offset) = self.discriminant_def_for_variant(variant_index);
23412346
let explicit_value = val
23422347
.and_then(|expr_did| self.eval_explicit_discr(tcx, expr_did))

src/librustc_middle/ty/sty.rs

+16-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use std::borrow::Cow;
2929
use std::cmp::Ordering;
3030
use std::marker::PhantomData;
3131
use std::ops::Range;
32+
use ty::util::IntTypeExt;
3233

3334
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)]
3435
#[derive(HashStable, TypeFoldable, Lift)]
@@ -2096,14 +2097,28 @@ impl<'tcx> TyS<'tcx> {
20962097
variant_index: VariantIdx,
20972098
) -> Option<Discr<'tcx>> {
20982099
match self.kind {
2099-
TyKind::Adt(adt, _) => Some(adt.discriminant_for_variant(tcx, variant_index)),
2100+
TyKind::Adt(adt, _) if adt.is_enum() => {
2101+
Some(adt.discriminant_for_variant(tcx, variant_index))
2102+
}
21002103
TyKind::Generator(def_id, substs, _) => {
21012104
Some(substs.as_generator().discriminant_for_variant(def_id, tcx, variant_index))
21022105
}
21032106
_ => None,
21042107
}
21052108
}
21062109

2110+
/// Returns the type of the discriminant of this type.
2111+
pub fn discriminant_ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
2112+
match self.kind {
2113+
ty::Adt(adt, _) if adt.is_enum() => adt.repr.discr_type().to_ty(tcx),
2114+
ty::Generator(_, substs, _) => substs.as_generator().discr_ty(tcx),
2115+
_ => {
2116+
// This can only be `0`, for now, so `u8` will suffice.
2117+
tcx.types.u8
2118+
}
2119+
}
2120+
}
2121+
21072122
/// When we create a closure, we record its kind (i.e., what trait
21082123
/// it implements) into its `ClosureSubsts` using a type
21092124
/// parameter. This is kind of a phantom type, except that the

src/librustc_mir/interpret/intrinsics.rs

+1-9
Original file line numberDiff line numberDiff line change
@@ -220,15 +220,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
220220
sym::discriminant_value => {
221221
let place = self.deref_operand(args[0])?;
222222
let discr_val = self.read_discriminant(place.into())?.0;
223-
let scalar = match dest.layout.ty.kind {
224-
ty::Int(_) => Scalar::from_int(
225-
self.sign_extend(discr_val, dest.layout) as i128,
226-
dest.layout.size,
227-
),
228-
ty::Uint(_) => Scalar::from_uint(discr_val, dest.layout.size),
229-
_ => bug!("invalid `discriminant_value` return layout: {:?}", dest.layout),
230-
};
231-
self.write_scalar(scalar, dest)?;
223+
self.write_scalar(discr_val, dest)?;
232224
}
233225
sym::unchecked_shl
234226
| sym::unchecked_shr

src/librustc_mir/interpret/operand.rs

+95-68
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ use std::fmt::Write;
77
use rustc_errors::ErrorReported;
88
use rustc_hir::def::Namespace;
99
use rustc_macros::HashStable;
10-
use rustc_middle::ty::layout::{IntegerExt, PrimitiveExt, TyAndLayout};
10+
use rustc_middle::ty::layout::{PrimitiveExt, TyAndLayout};
1111
use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter, Printer};
1212
use rustc_middle::ty::Ty;
1313
use rustc_middle::{mir, ty};
14-
use rustc_target::abi::{Abi, DiscriminantKind, HasDataLayout, Integer, LayoutOf, Size};
14+
use rustc_target::abi::{Abi, DiscriminantKind, HasDataLayout, LayoutOf, Size};
1515
use rustc_target::abi::{VariantIdx, Variants};
1616

1717
use super::{
18-
from_known_layout, sign_extend, truncate, ConstValue, GlobalId, InterpCx, InterpResult,
18+
from_known_layout, mir_assign_valid_types, ConstValue, GlobalId, InterpCx, InterpResult,
1919
MPlaceTy, Machine, MemPlace, Place, PlaceTy, Pointer, Scalar, ScalarMaybeUninit,
2020
};
2121

@@ -469,6 +469,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
469469
.try_fold(base_op, |op, elem| self.operand_projection(op, elem))?;
470470

471471
trace!("eval_place_to_op: got {:?}", *op);
472+
// Sanity-check the type we ended up with.
473+
debug_assert!(mir_assign_valid_types(
474+
*self.tcx,
475+
self.layout_of(self.subst_from_current_frame_and_normalize_erasing_regions(
476+
place.ty(&self.frame().body.local_decls, *self.tcx).ty
477+
))?,
478+
op.layout,
479+
));
472480
Ok(op)
473481
}
474482

@@ -576,98 +584,113 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
576584
/// Read discriminant, return the runtime value as well as the variant index.
577585
pub fn read_discriminant(
578586
&self,
579-
rval: OpTy<'tcx, M::PointerTag>,
580-
) -> InterpResult<'tcx, (u128, VariantIdx)> {
581-
trace!("read_discriminant_value {:#?}", rval.layout);
582-
583-
let (discr_layout, discr_kind, discr_index) = match rval.layout.variants {
587+
op: OpTy<'tcx, M::PointerTag>,
588+
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, VariantIdx)> {
589+
trace!("read_discriminant_value {:#?}", op.layout);
590+
591+
// Get type and layout of the discriminant.
592+
let discr_layout = self.layout_of(op.layout.ty.discriminant_ty(*self.tcx))?;
593+
trace!("discriminant type: {:?}", discr_layout.ty);
594+
595+
// We use "discriminant" to refer to the value associated with a particular enum variant.
596+
// This is not to be confused with its "variant index", which is just determining its position in the
597+
// declared list of variants -- they can differ with explicitly assigned discriminants.
598+
// We use "tag" to refer to how the discriminant is encoded in memory, which can be either
599+
// straight-forward (`DiscriminantKind::Tag`) or with a niche (`DiscriminantKind::Niche`).
600+
// Unfortunately, the rest of the compiler calls the latter "discriminant", too, which makes things
601+
// rather confusing.
602+
let (tag_scalar_layout, tag_kind, tag_index) = match op.layout.variants {
584603
Variants::Single { index } => {
585-
let discr_val = rval
586-
.layout
587-
.ty
588-
.discriminant_for_variant(*self.tcx, index)
589-
.map_or(u128::from(index.as_u32()), |discr| discr.val);
590-
return Ok((discr_val, index));
604+
let discr = match op.layout.ty.discriminant_for_variant(*self.tcx, index) {
605+
Some(discr) => {
606+
// This type actually has discriminants.
607+
assert_eq!(discr.ty, discr_layout.ty);
608+
Scalar::from_uint(discr.val, discr_layout.size)
609+
}
610+
None => {
611+
// On a type without actual discriminants, variant is 0.
612+
assert_eq!(index.as_u32(), 0);
613+
Scalar::from_uint(index.as_u32(), discr_layout.size)
614+
}
615+
};
616+
return Ok((discr, index));
591617
}
592-
Variants::Multiple { discr: ref discr_layout, ref discr_kind, discr_index, .. } => {
593-
(discr_layout, discr_kind, discr_index)
618+
Variants::Multiple { ref discr, ref discr_kind, discr_index, .. } => {
619+
(discr, discr_kind, discr_index)
594620
}
595621
};
596622

597-
// read raw discriminant value
598-
let discr_op = self.operand_field(rval, discr_index)?;
599-
let discr_val = self.read_immediate(discr_op)?;
600-
let raw_discr = discr_val.to_scalar_or_undef();
601-
trace!("discr value: {:?}", raw_discr);
602-
// post-process
603-
Ok(match *discr_kind {
623+
// There are *three* layouts that come into play here:
624+
// - The discriminant has a type for typechecking. This is `discr_layout`, and is used for
625+
// the `Scalar` we return.
626+
// - The tag (encoded discriminant) has layout `tag_layout`. This is always an integer type,
627+
// and used to interpret the value we read from the tag field.
628+
// For the return value, a cast to `discr_layout` is performed.
629+
// - The field storing the tag has a layout, which is very similar to `tag_layout` but
630+
// may be a pointer. This is `tag_val.layout`; we just use it for sanity checks.
631+
632+
// Get layout for tag.
633+
let tag_layout = self.layout_of(tag_scalar_layout.value.to_int_ty(*self.tcx))?;
634+
635+
// Read tag and sanity-check `tag_layout`.
636+
let tag_val = self.read_immediate(self.operand_field(op, tag_index)?)?;
637+
assert_eq!(tag_layout.size, tag_val.layout.size);
638+
assert_eq!(tag_layout.abi.is_signed(), tag_val.layout.abi.is_signed());
639+
let tag_val = tag_val.to_scalar()?;
640+
trace!("tag value: {:?}", tag_val);
641+
642+
// Figure out which discriminant and variant this corresponds to.
643+
Ok(match *tag_kind {
604644
DiscriminantKind::Tag => {
605-
let bits_discr = raw_discr
606-
.not_undef()
607-
.and_then(|raw_discr| self.force_bits(raw_discr, discr_val.layout.size))
608-
.map_err(|_| err_ub!(InvalidDiscriminant(raw_discr.erase_tag())))?;
609-
let real_discr = if discr_val.layout.abi.is_signed() {
610-
// going from layout tag type to typeck discriminant type
611-
// requires first sign extending with the discriminant layout
612-
let sexted = sign_extend(bits_discr, discr_val.layout.size);
613-
// and then zeroing with the typeck discriminant type
614-
let discr_ty = rval
615-
.layout
616-
.ty
617-
.ty_adt_def()
618-
.expect("tagged layout corresponds to adt")
619-
.repr
620-
.discr_type();
621-
let size = Integer::from_attr(self, discr_ty).size();
622-
truncate(sexted, size)
623-
} else {
624-
bits_discr
625-
};
626-
// Make sure we catch invalid discriminants
627-
let index = match rval.layout.ty.kind {
645+
let tag_bits = self
646+
.force_bits(tag_val, tag_layout.size)
647+
.map_err(|_| err_ub!(InvalidDiscriminant(tag_val.erase_tag())))?;
648+
// Cast bits from tag layout to discriminant layout.
649+
let discr_val_cast = self.cast_from_scalar(tag_bits, tag_layout, discr_layout.ty);
650+
let discr_bits = discr_val_cast.assert_bits(discr_layout.size);
651+
// Convert discriminant to variant index, and catch invalid discriminants.
652+
let index = match op.layout.ty.kind {
628653
ty::Adt(adt, _) => {
629-
adt.discriminants(self.tcx.tcx).find(|(_, var)| var.val == real_discr)
654+
adt.discriminants(self.tcx.tcx).find(|(_, var)| var.val == discr_bits)
630655
}
631656
ty::Generator(def_id, substs, _) => {
632657
let substs = substs.as_generator();
633658
substs
634659
.discriminants(def_id, self.tcx.tcx)
635-
.find(|(_, var)| var.val == real_discr)
660+
.find(|(_, var)| var.val == discr_bits)
636661
}
637662
_ => bug!("tagged layout for non-adt non-generator"),
638663
}
639-
.ok_or_else(|| err_ub!(InvalidDiscriminant(raw_discr.erase_tag())))?;
640-
(real_discr, index.0)
664+
.ok_or_else(|| err_ub!(InvalidDiscriminant(tag_val.erase_tag())))?;
665+
// Return the cast value, and the index.
666+
(discr_val_cast, index.0)
641667
}
642668
DiscriminantKind::Niche { dataful_variant, ref niche_variants, niche_start } => {
669+
// Compute the variant this niche value/"tag" corresponds to. With niche layout,
670+
// discriminant (encoded in niche/tag) and variant index are the same.
643671
let variants_start = niche_variants.start().as_u32();
644672
let variants_end = niche_variants.end().as_u32();
645-
let raw_discr = raw_discr
646-
.not_undef()
647-
.map_err(|_| err_ub!(InvalidDiscriminant(ScalarMaybeUninit::Uninit)))?;
648-
match raw_discr.to_bits_or_ptr(discr_val.layout.size, self) {
673+
let variant = match tag_val.to_bits_or_ptr(tag_layout.size, self) {
649674
Err(ptr) => {
650675
// The niche must be just 0 (which an inbounds pointer value never is)
651676
let ptr_valid = niche_start == 0
652677
&& variants_start == variants_end
653678
&& !self.memory.ptr_may_be_null(ptr);
654679
if !ptr_valid {
655-
throw_ub!(InvalidDiscriminant(raw_discr.erase_tag().into()))
680+
throw_ub!(InvalidDiscriminant(tag_val.erase_tag()))
656681
}
657-
(u128::from(dataful_variant.as_u32()), dataful_variant)
682+
dataful_variant
658683
}
659-
Ok(raw_discr) => {
684+
Ok(tag_bits) => {
660685
// We need to use machine arithmetic to get the relative variant idx:
661-
// variant_index_relative = discr_val - niche_start_val
662-
let discr_layout =
663-
self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?;
664-
let discr_val = ImmTy::from_uint(raw_discr, discr_layout);
665-
let niche_start_val = ImmTy::from_uint(niche_start, discr_layout);
686+
// variant_index_relative = tag_val - niche_start_val
687+
let tag_val = ImmTy::from_uint(tag_bits, tag_layout);
688+
let niche_start_val = ImmTy::from_uint(niche_start, tag_layout);
666689
let variant_index_relative_val =
667-
self.binary_op(mir::BinOp::Sub, discr_val, niche_start_val)?;
690+
self.binary_op(mir::BinOp::Sub, tag_val, niche_start_val)?;
668691
let variant_index_relative = variant_index_relative_val
669692
.to_scalar()?
670-
.assert_bits(discr_val.layout.size);
693+
.assert_bits(tag_val.layout.size);
671694
// Check if this is in the range that indicates an actual discriminant.
672695
if variant_index_relative <= u128::from(variants_end - variants_start) {
673696
let variant_index_relative = u32::try_from(variant_index_relative)
@@ -676,20 +699,24 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
676699
let variant_index = variants_start
677700
.checked_add(variant_index_relative)
678701
.expect("overflow computing absolute variant idx");
679-
let variants_len = rval
702+
let variants_len = op
680703
.layout
681704
.ty
682705
.ty_adt_def()
683706
.expect("tagged layout for non adt")
684707
.variants
685708
.len();
686709
assert!(usize::try_from(variant_index).unwrap() < variants_len);
687-
(u128::from(variant_index), VariantIdx::from_u32(variant_index))
710+
VariantIdx::from_u32(variant_index)
688711
} else {
689-
(u128::from(dataful_variant.as_u32()), dataful_variant)
712+
dataful_variant
690713
}
691714
}
692-
}
715+
};
716+
// Compute the size of the scalar we need to return.
717+
// No need to cast, because the variant index directly serves as discriminant and is
718+
// encoded in the tag.
719+
(Scalar::from_uint(variant.as_u32(), discr_layout.size), variant)
693720
}
694721
})
695722
}

src/librustc_mir/interpret/place.rs

+8
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,14 @@ where
638638
}
639639

640640
self.dump_place(place_ty.place);
641+
// Sanity-check the type we ended up with.
642+
debug_assert!(mir_assign_valid_types(
643+
*self.tcx,
644+
self.layout_of(self.subst_from_current_frame_and_normalize_erasing_regions(
645+
place.ty(&self.frame().body.local_decls, *self.tcx).ty
646+
))?,
647+
place_ty.layout,
648+
));
641649
Ok(place_ty)
642650
}
643651

src/librustc_mir/interpret/step.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
262262
Discriminant(place) => {
263263
let op = self.eval_place_to_op(place, None)?;
264264
let discr_val = self.read_discriminant(op)?.0;
265-
let size = dest.layout.size;
266-
self.write_scalar(Scalar::from_uint(discr_val, size), dest)?;
265+
self.write_scalar(discr_val, dest)?;
267266
}
268267
}
269268

0 commit comments

Comments
 (0)