Skip to content

Commit

Permalink
Variants::Single: do not use invalid VariantIdx for uninhabited enums
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Dec 1, 2024
1 parent 3dc9b1b commit c7b2a22
Show file tree
Hide file tree
Showing 21 changed files with 161 additions and 146 deletions.
14 changes: 7 additions & 7 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
.max_by_key(|niche| niche.available(dl));

LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
fields: FieldsShape::Arbitrary {
offsets: [Size::ZERO, b_offset].into(),
memory_index: [0, 1].into(),
Expand Down Expand Up @@ -214,7 +214,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
) -> LayoutData<FieldIdx, VariantIdx> {
let dl = self.cx.data_layout();
LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Single { index: None },
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Uninhabited,
largest_niche: None,
Expand Down Expand Up @@ -385,7 +385,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
};

Ok(LayoutData {
variants: Variants::Single { index: only_variant_idx },
variants: Variants::Single { index: Some(only_variant_idx) },
fields: FieldsShape::Union(union_field_count),
backend_repr: abi,
largest_niche: None,
Expand Down Expand Up @@ -424,7 +424,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
};

let mut st = self.univariant(&variants[v], repr, kind)?;
st.variants = Variants::Single { index: v };
st.variants = Variants::Single { index: Some(v) };

if is_unsafe_cell {
let hide_niches = |scalar: &mut _| match scalar {
Expand Down Expand Up @@ -543,7 +543,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
.iter_enumerated()
.map(|(j, v)| {
let mut st = self.univariant(v, repr, StructKind::AlwaysSized).ok()?;
st.variants = Variants::Single { index: j };
st.variants = Variants::Single { index: Some(j) };

align = align.max(st.align);
max_repr_align = max_repr_align.max(st.max_repr_align);
Expand Down Expand Up @@ -736,7 +736,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
repr,
StructKind::Prefixed(min_ity.size(), prefix_align),
)?;
st.variants = Variants::Single { index: i };
st.variants = Variants::Single { index: Some(i) };
// Find the first field we can't move later
// to make room for a larger discriminant.
for field_idx in st.fields.index_by_increasing_offset() {
Expand Down Expand Up @@ -1344,7 +1344,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
};

Ok(LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
fields: FieldsShape::Arbitrary { offsets, memory_index },
backend_repr: abi,
largest_niche,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1506,9 +1506,9 @@ impl BackendRepr {
pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
/// Single enum variants, structs/tuples, unions, and all non-ADTs.
Single {
/// Always 0 for non-enums/generators.
/// For enums without a variant, this is an invalid index!
index: VariantIdx,
/// Always `Some(0)` for types without variants (i.e., everything except for `!`, enums, and
/// generators). `None` indicates an uninhabited type; this is used for zero-variant enums.
index: Option<VariantIdx>,
},

/// Enum-likes with more than one variant: each variant comes with
Expand Down Expand Up @@ -1706,7 +1706,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
let size = scalar.size(cx);
let align = scalar.align(cx);
LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Scalar(scalar),
largest_niche,
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_codegen_cranelift/src/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
}
match layout.variants {
Variants::Single { index } => {
assert_eq!(index, variant_index);
assert_eq!(index.unwrap(), variant_index);
}
Variants::Multiple {
tag: _,
Expand Down Expand Up @@ -86,9 +86,10 @@ pub(crate) fn codegen_get_discriminant<'tcx>(

let (tag_scalar, tag_field, tag_encoding) = match &layout.variants {
Variants::Single { index } => {
let index = index.unwrap();
let discr_val = layout
.ty
.discriminant_for_variant(fx.tcx, *index)
.discriminant_for_variant(fx.tcx, index)
.map_or(u128::from(index.as_u32()), |discr| discr.val);

let val = match dest_layout.ty.kind() {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,14 @@ fn uncached_gcc_type<'gcc, 'tcx>(
if !cx.sess().fewer_names() =>
{
let mut name = with_no_trimmed_paths!(layout.ty.to_string());
if let (&ty::Adt(def, _), &Variants::Single { index }) =
if let (&ty::Adt(def, _), &Variants::Single { index: Some(index) }) =
(layout.ty.kind(), &layout.variants)
{
if def.is_enum() && !def.variants().is_empty() {
write!(&mut name, "::{}", def.variant(index).name).unwrap();
}
}
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
if let (&ty::Coroutine(_, _), &Variants::Single { index: Some(index) }) =
(layout.ty.kind(), &layout.variants)
{
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
Expand Down Expand Up @@ -230,7 +230,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {

// Check the cache.
let variant_index = match self.variants {
Variants::Single { index } => Some(index),
Variants::Single { index } => index,
_ => None,
};
let cached_type = cx.types.borrow().get(&(self.ty, variant_index)).cloned();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,11 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
|cx, enum_type_di_node| {
match enum_type_and_layout.variants {
Variants::Single { index: variant_index } => {
if enum_adt_def.variants().is_empty() {
let Some(variant_index) = variant_index else {
// Uninhabited enums have Variants::Single. We don't generate
// any members for them.
return smallvec![];
}
};

build_single_variant_union_fields(
cx,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ fn uncached_llvm_type<'a, 'tcx>(
if !cx.sess().fewer_names() =>
{
let mut name = with_no_visible_paths!(with_no_trimmed_paths!(layout.ty.to_string()));
if let (&ty::Adt(def, _), &Variants::Single { index }) =
if let (&ty::Adt(def, _), &Variants::Single { index: Some(index) }) =
(layout.ty.kind(), &layout.variants)
{
if def.is_enum() && !def.variants().is_empty() {
if def.is_enum() {
write!(&mut name, "::{}", def.variant(index).name).unwrap();
}
}
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
if let (&ty::Coroutine(_, _), &Variants::Single { index: Some(index) }) =
(layout.ty.kind(), &layout.variants)
{
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
Expand Down Expand Up @@ -216,7 +216,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {

// Check the cache.
let variant_index = match self.variants {
Variants::Single { index } => Some(index),
Variants::Single { index } => index,
_ => None,
};
if let Some(llty) = cx.type_lowering.borrow().get(&(self.ty, variant_index)) {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
}
let (tag_scalar, tag_encoding, tag_field) = match self.layout.variants {
Variants::Single { index } => {
let index = index.unwrap(); // we already checked `is_uninhabited`
let discr_val = self
.layout
.ty
Expand Down Expand Up @@ -365,7 +366,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
}
match self.layout.variants {
Variants::Single { index } => {
assert_eq!(index, variant_index);
assert_eq!(index.unwrap(), variant_index);
}
Variants::Multiple { tag_encoding: TagEncoding::Direct, tag_field, .. } => {
let ptr = self.project_field(bx, tag_field);
Expand Down
23 changes: 10 additions & 13 deletions compiler/rustc_const_eval/src/interpret/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}
}

/// Read discriminant, return the runtime value as well as the variant index.
/// Read discriminant, return the variant index.
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
///
/// Will never return an uninhabited variant.
Expand All @@ -66,20 +66,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout().variants {
Variants::Single { index } => {
// Do some extra checks on enums.
if ty.is_enum() {
// Hilariously, `Single` is used even for 0-variant enums.
// (See https://github.com/rust-lang/rust/issues/89765).
if ty.ty_adt_def().unwrap().variants().is_empty() {
throw_ub!(UninhabitedEnumVariantRead(index))
}
if op.layout().is_uninhabited() {
// For consistency with `write_discriminant`, and to make sure that
// `project_downcast` cannot fail due to strange layouts, we declare immediate UB
// for uninhabited variants.
if op.layout().for_variant(self, index).is_uninhabited() {
throw_ub!(UninhabitedEnumVariantRead(index))
}
// for uninhabited enums.
throw_ub!(UninhabitedEnumVariantRead(None));
}
// Sanity checks.
let index = index.unwrap();
assert!(!op.layout().for_variant(self, index).is_uninhabited());
return interp_ok(index);
}
Variants::Multiple { tag, ref tag_encoding, tag_field, .. } => {
Expand Down Expand Up @@ -199,11 +194,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// `uninhabited_enum_branching` MIR pass. It also ensures consistency with
// `write_discriminant`.
if op.layout().for_variant(self, index).is_uninhabited() {
throw_ub!(UninhabitedEnumVariantRead(index))
throw_ub!(UninhabitedEnumVariantRead(Some(index)))
}
interp_ok(index)
}

/// Read discriminant, return the user-visible discriminant.
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
pub fn discriminant_for_variant(
&self,
ty: Ty<'tcx>,
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,9 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
match layout.variants {
Variants::Single { index } => {
// Inside a variant
PathElem::Field(def.variant(index).fields[FieldIdx::from_usize(field)].name)
PathElem::Field(
def.variant(index.unwrap()).fields[FieldIdx::from_usize(field)].name,
)
}
Variants::Multiple { .. } => bug!("we handled variants above"),
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
/// A discriminant of an uninhabited enum variant is written.
UninhabitedEnumVariantWritten(VariantIdx),
/// An uninhabited enum variant is projected.
UninhabitedEnumVariantRead(VariantIdx),
UninhabitedEnumVariantRead(Option<VariantIdx>),
/// Trying to set discriminant to the niched variant, but the value does not match.
InvalidNichedEnumVariantWritten { enum_ty: Ty<'tcx> },
/// ABI-incompatible argument types.
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ where
let layout = match this.variants {
Variants::Single { index }
// If all variants but one are uninhabited, the variant layout is the enum layout.
if index == variant_index &&
if index == Some(variant_index) &&
// Don't confuse variants of uninhabited enums with the enum itself.
// For more details see https://github.com/rust-lang/rust/issues/69763.
this.fields != FieldsShape::Primitive =>
Expand All @@ -741,6 +741,8 @@ where
}

Variants::Single { index } => {
// `Single` variant enums *can* have other variants, but those are uninhabited.

let tcx = cx.tcx();
let typing_env = cx.typing_env();

Expand All @@ -756,7 +758,7 @@ where
_ => bug!("`ty_and_layout_for_variant` on unexpected type {}", this.ty),
};
tcx.mk_layout(LayoutData {
variants: Variants::Single { index: variant_index },
variants: Variants::Single { index: Some(variant_index) },
fields: match NonZero::new(fields) {
Some(fields) => FieldsShape::Union(fields),
None => FieldsShape::Arbitrary { offsets: IndexVec::new(), memory_index: IndexVec::new() },
Expand All @@ -773,7 +775,7 @@ where
Variants::Multiple { ref variants, .. } => cx.tcx().mk_layout(variants[variant_index].clone()),
};

assert_eq!(*layout.variants(), Variants::Single { index: variant_index });
assert_eq!(*layout.variants(), Variants::Single { index: Some(variant_index) });

TyAndLayout { ty: this.ty, layout }
}
Expand Down Expand Up @@ -903,7 +905,7 @@ where
Variants::Single { index } => TyMaybeWithLayout::Ty(
args.as_coroutine()
.state_tys(def_id, tcx)
.nth(index.as_usize())
.nth(index.unwrap().as_usize())
.unwrap()
.nth(i)
.unwrap(),
Expand All @@ -922,7 +924,8 @@ where
ty::Adt(def, args) => {
match this.variants {
Variants::Single { index } => {
let field = &def.variant(index).fields[FieldIdx::from_usize(i)];
let field =
&def.variant(index.unwrap()).fields[FieldIdx::from_usize(i)];
TyMaybeWithLayout::Ty(field.ty(tcx, args))
}

Expand Down
31 changes: 7 additions & 24 deletions compiler/rustc_mir_transform/src/jump_threading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
//! Likewise, applying the optimisation can create a lot of new MIR, so we bound the instruction
//! cost by `MAX_COST`.
use rustc_abi::{TagEncoding, Variants};
use rustc_arena::DroplessArena;
use rustc_const_eval::const_eval::DummyMachine;
use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, Projectable};
Expand Down Expand Up @@ -565,31 +564,15 @@ impl<'a, 'tcx> TOFinder<'a, 'tcx> {
StatementKind::SetDiscriminant { box place, variant_index } => {
let Some(discr_target) = self.map.find_discr(place.as_ref()) else { return };
let enum_ty = place.ty(self.body, self.tcx).ty;
// `SetDiscriminant` may be a no-op if the assigned variant is the untagged variant
// of a niche encoding. If we cannot ensure that we write to the discriminant, do
// nothing.
let Ok(enum_layout) = self.ecx.layout_of(enum_ty) else {
// `SetDiscriminant` guarantees that the discriminant is now `variant_index`.
// Even if the discriminant write does nothing due to niches, it is UB to set the
// discriminant when the data does not encode the desired discriminant.
let Some(discr) =
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
else {
return;
};
let writes_discriminant = match enum_layout.variants {
Variants::Single { index } => {
assert_eq!(index, *variant_index);
true
}
Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => true,
Variants::Multiple {
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
..
} => *variant_index != untagged_variant,
};
if writes_discriminant {
let Some(discr) =
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
else {
return;
};
self.process_immediate(bb, discr_target, discr, state);
}
self.process_immediate(bb, discr_target, discr, state);
}
// If we expect `lhs ?= true`, we have an opportunity if we assume `lhs == true`.
StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ fn variant_discriminants<'tcx>(
tcx: TyCtxt<'tcx>,
) -> FxHashSet<u128> {
match &layout.variants {
Variants::Single { index } => {
Variants::Single { index: None } => {
// Uninhabited, no valid discriminant.
FxHashSet::default()
}
Variants::Single { index: Some(index) } => {
let mut res = FxHashSet::default();
res.insert(
ty.discriminant_for_variant(tcx, *index)
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_smir/src/rustc_smir/convert/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ impl<'tcx> Stable<'tcx> for rustc_abi::Variants<rustc_abi::FieldIdx, rustc_abi::

fn stable(&self, tables: &mut Tables<'_>) -> Self::T {
match self {
rustc_abi::Variants::Single { index } => {
VariantsShape::Single { index: index.stable(tables) }
}
rustc_abi::Variants::Single { index } => VariantsShape::Single {
index: index.unwrap_or(rustc_abi::VariantIdx::from_u32(0)).stable(tables),
},
rustc_abi::Variants::Multiple { tag, tag_encoding, tag_field, variants } => {
VariantsShape::Multiple {
tag: tag.stable(tables),
Expand Down
Loading

0 comments on commit c7b2a22

Please sign in to comment.