Skip to content

Commit 21de42b

Browse files
RalfJungRalfJung
committed
Variants::Single: do not use invalid VariantIdx for uninhabited enums
1 parent 37e7459 commit 21de42b

File tree

47 files changed

+548
-280
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+548
-280
lines changed

‎compiler/rustc_abi/src/layout.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
120120
.max_by_key(|niche| niche.available(dl));
121121

122122
LayoutData {
123-
variants: Variants::Single { index: VariantIdx::new(0) },
123+
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
124124
fields: FieldsShape::Arbitrary {
125125
offsets: [Size::ZERO, b_offset].into(),
126126
memory_index: [0, 1].into(),
@@ -214,7 +214,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
214214
) -> LayoutData<FieldIdx, VariantIdx> {
215215
let dl = self.cx.data_layout();
216216
LayoutData {
217-
variants: Variants::Single { index: VariantIdx::new(0) },
217+
variants: Variants::Single { index: None },
218218
fields: FieldsShape::Primitive,
219219
backend_repr: BackendRepr::Uninhabited,
220220
largest_niche: None,
@@ -385,7 +385,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
385385
};
386386

387387
Ok(LayoutData {
388-
variants: Variants::Single { index: only_variant_idx },
388+
variants: Variants::Single { index: Some(only_variant_idx) },
389389
fields: FieldsShape::Union(union_field_count),
390390
backend_repr: abi,
391391
largest_niche: None,
@@ -424,7 +424,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
424424
};
425425

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

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

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

13461346
Ok(LayoutData {
1347-
variants: Variants::Single { index: VariantIdx::new(0) },
1347+
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
13481348
fields: FieldsShape::Arbitrary { offsets, memory_index },
13491349
backend_repr: abi,
13501350
largest_niche,

‎compiler/rustc_abi/src/lib.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1506,9 +1506,9 @@ impl BackendRepr {
15061506
pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
15071507
/// Single enum variants, structs/tuples, unions, and all non-ADTs.
15081508
Single {
1509-
/// Always 0 for non-enums/generators.
1510-
/// For enums without a variant, this is an invalid index!
1511-
index: VariantIdx,
1509+
/// Always `Some(0)` for types without variants (i.e., everything except for `!`, enums, and
1510+
/// generators). `None` indicates an uninhabited type; this is used for zero-variant enums.
1511+
index: Option<VariantIdx>,
15121512
},
15131513

15141514
/// Enum-likes with more than one variant: each variant comes with
@@ -1706,7 +1706,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
17061706
let size = scalar.size(cx);
17071707
let align = scalar.align(cx);
17081708
LayoutData {
1709-
variants: Variants::Single { index: VariantIdx::new(0) },
1709+
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
17101710
fields: FieldsShape::Primitive,
17111711
backend_repr: BackendRepr::Scalar(scalar),
17121712
largest_niche,

‎compiler/rustc_codegen_cranelift/src/discriminant.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
1919
}
2020
match layout.variants {
2121
Variants::Single { index } => {
22-
assert_eq!(index, variant_index);
22+
assert_eq!(index.unwrap(), variant_index);
2323
}
2424
Variants::Multiple {
2525
tag: _,
@@ -86,9 +86,10 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
8686

8787
let (tag_scalar, tag_field, tag_encoding) = match &layout.variants {
8888
Variants::Single { index } => {
89+
let index = index.unwrap();
8990
let discr_val = layout
9091
.ty
91-
.discriminant_for_variant(fx.tcx, *index)
92+
.discriminant_for_variant(fx.tcx, index)
9293
.map_or(u128::from(index.as_u32()), |discr| discr.val);
9394

9495
let val = match dest_layout.ty.kind() {

‎compiler/rustc_codegen_gcc/src/type_of.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,14 @@ fn uncached_gcc_type<'gcc, 'tcx>(
9999
if !cx.sess().fewer_names() =>
100100
{
101101
let mut name = with_no_trimmed_paths!(layout.ty.to_string());
102-
if let (&ty::Adt(def, _), &Variants::Single { index }) =
102+
if let (&ty::Adt(def, _), &Variants::Single { index: Some(index) }) =
103103
(layout.ty.kind(), &layout.variants)
104104
{
105105
if def.is_enum() && !def.variants().is_empty() {
106106
write!(&mut name, "::{}", def.variant(index).name).unwrap();
107107
}
108108
}
109-
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
109+
if let (&ty::Coroutine(_, _), &Variants::Single { index: Some(index) }) =
110110
(layout.ty.kind(), &layout.variants)
111111
{
112112
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
@@ -230,7 +230,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
230230

231231
// Check the cache.
232232
let variant_index = match self.variants {
233-
Variants::Single { index } => Some(index),
233+
Variants::Single { index } => index,
234234
_ => None,
235235
};
236236
let cached_type = cx.types.borrow().get(&(self.ty, variant_index)).cloned();

‎compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,11 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
213213
|cx, enum_type_di_node| {
214214
match enum_type_and_layout.variants {
215215
Variants::Single { index: variant_index } => {
216-
if enum_adt_def.variants().is_empty() {
216+
let Some(variant_index) = variant_index else {
217217
// Uninhabited enums have Variants::Single. We don't generate
218218
// any members for them.
219219
return smallvec![];
220-
}
220+
};
221221

222222
build_single_variant_union_fields(
223223
cx,

‎compiler/rustc_codegen_llvm/src/type_of.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@ fn uncached_llvm_type<'a, 'tcx>(
3535
if !cx.sess().fewer_names() =>
3636
{
3737
let mut name = with_no_visible_paths!(with_no_trimmed_paths!(layout.ty.to_string()));
38-
if let (&ty::Adt(def, _), &Variants::Single { index }) =
38+
if let (&ty::Adt(def, _), &Variants::Single { index: Some(index) }) =
3939
(layout.ty.kind(), &layout.variants)
4040
{
41-
if def.is_enum() && !def.variants().is_empty() {
41+
if def.is_enum() {
4242
write!(&mut name, "::{}", def.variant(index).name).unwrap();
4343
}
4444
}
45-
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
45+
if let (&ty::Coroutine(_, _), &Variants::Single { index: Some(index) }) =
4646
(layout.ty.kind(), &layout.variants)
4747
{
4848
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
@@ -216,7 +216,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {
216216

217217
// Check the cache.
218218
let variant_index = match self.variants {
219-
Variants::Single { index } => Some(index),
219+
Variants::Single { index } => index,
220220
_ => None,
221221
};
222222
if let Some(llty) = cx.type_lowering.borrow().get(&(self.ty, variant_index)) {

‎compiler/rustc_codegen_ssa/src/mir/place.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
244244
}
245245
let (tag_scalar, tag_encoding, tag_field) = match self.layout.variants {
246246
Variants::Single { index } => {
247+
let index = index.unwrap(); // we already checked `is_uninhabited`
247248
let discr_val = self
248249
.layout
249250
.ty
@@ -366,7 +367,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
366367
}
367368
match self.layout.variants {
368369
Variants::Single { index } => {
369-
assert_eq!(index, variant_index);
370+
assert_eq!(index.unwrap(), variant_index);
370371
}
371372
Variants::Multiple { tag_encoding: TagEncoding::Direct, tag_field, .. } => {
372373
let ptr = self.project_field(bx, tag_field);

‎compiler/rustc_const_eval/src/interpret/discriminant.rs

+9-14
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
4444
}
4545
}
4646

47-
/// Read discriminant, return the runtime value as well as the variant index.
47+
/// Read discriminant, return the variant index.
4848
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
4949
///
5050
/// Will never return an uninhabited variant.
@@ -66,21 +66,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
6666
// straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
6767
let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout().variants {
6868
Variants::Single { index } => {
69-
// Do some extra checks on enums.
70-
if ty.is_enum() {
71-
// Hilariously, `Single` is used even for 0-variant enums.
72-
// (See https://github.com/rust-lang/rust/issues/89765).
73-
if ty.ty_adt_def().unwrap().variants().is_empty() {
74-
throw_ub!(UninhabitedEnumVariantRead(index))
75-
}
69+
if op.layout().is_uninhabited() {
7670
// For consistency with `write_discriminant`, and to make sure that
7771
// `project_downcast` cannot fail due to strange layouts, we declare immediate UB
78-
// for uninhabited variants.
79-
if op.layout().for_variant(self, index).is_uninhabited() {
80-
throw_ub!(UninhabitedEnumVariantRead(index))
81-
}
72+
// for uninhabited enums.
73+
throw_ub!(UninhabitedEnumVariantRead(None));
8274
}
83-
return interp_ok(index);
75+
// Since the type is inhabited, there must be an index.
76+
return interp_ok(index.unwrap());
8477
}
8578
Variants::Multiple { tag, ref tag_encoding, tag_field, .. } => {
8679
(tag, tag_encoding, tag_field)
@@ -199,11 +192,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
199192
// `uninhabited_enum_branching` MIR pass. It also ensures consistency with
200193
// `write_discriminant`.
201194
if op.layout().for_variant(self, index).is_uninhabited() {
202-
throw_ub!(UninhabitedEnumVariantRead(index))
195+
throw_ub!(UninhabitedEnumVariantRead(Some(index)))
203196
}
204197
interp_ok(index)
205198
}
206199

200+
/// Read discriminant, return the user-visible discriminant.
201+
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
207202
pub fn discriminant_for_variant(
208203
&self,
209204
ty: Ty<'tcx>,

‎compiler/rustc_const_eval/src/interpret/validity.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,9 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
342342
match layout.variants {
343343
Variants::Single { index } => {
344344
// Inside a variant
345-
PathElem::Field(def.variant(index).fields[FieldIdx::from_usize(field)].name)
345+
PathElem::Field(
346+
def.variant(index.unwrap()).fields[FieldIdx::from_usize(field)].name,
347+
)
346348
}
347349
Variants::Multiple { .. } => bug!("we handled variants above"),
348350
}

‎compiler/rustc_middle/src/mir/interpret/error.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
392392
/// A discriminant of an uninhabited enum variant is written.
393393
UninhabitedEnumVariantWritten(VariantIdx),
394394
/// An uninhabited enum variant is projected.
395-
UninhabitedEnumVariantRead(VariantIdx),
395+
UninhabitedEnumVariantRead(Option<VariantIdx>),
396396
/// Trying to set discriminant to the niched variant, but the value does not match.
397397
InvalidNichedEnumVariantWritten { enum_ty: Ty<'tcx> },
398398
/// ABI-incompatible argument types.

‎compiler/rustc_middle/src/ty/layout.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ where
734734
let layout = match this.variants {
735735
Variants::Single { index }
736736
// If all variants but one are uninhabited, the variant layout is the enum layout.
737-
if index == variant_index &&
737+
if index == Some(variant_index) &&
738738
// Don't confuse variants of uninhabited enums with the enum itself.
739739
// For more details see https://github.com/rust-lang/rust/issues/69763.
740740
this.fields != FieldsShape::Primitive =>
@@ -743,6 +743,8 @@ where
743743
}
744744

745745
Variants::Single { index } => {
746+
// `Single` variant enums *can* have other variants, but those are uninhabited.
747+
746748
let tcx = cx.tcx();
747749
let typing_env = cx.typing_env();
748750

@@ -758,7 +760,7 @@ where
758760
_ => bug!("`ty_and_layout_for_variant` on unexpected type {}", this.ty),
759761
};
760762
tcx.mk_layout(LayoutData {
761-
variants: Variants::Single { index: variant_index },
763+
variants: Variants::Single { index: Some(variant_index) },
762764
fields: match NonZero::new(fields) {
763765
Some(fields) => FieldsShape::Union(fields),
764766
None => FieldsShape::Arbitrary { offsets: IndexVec::new(), memory_index: IndexVec::new() },
@@ -775,7 +777,7 @@ where
775777
Variants::Multiple { ref variants, .. } => cx.tcx().mk_layout(variants[variant_index].clone()),
776778
};
777779

778-
assert_eq!(*layout.variants(), Variants::Single { index: variant_index });
780+
assert_eq!(*layout.variants(), Variants::Single { index: Some(variant_index) });
779781

780782
TyAndLayout { ty: this.ty, layout }
781783
}
@@ -905,7 +907,7 @@ where
905907
Variants::Single { index } => TyMaybeWithLayout::Ty(
906908
args.as_coroutine()
907909
.state_tys(def_id, tcx)
908-
.nth(index.as_usize())
910+
.nth(index.unwrap().as_usize())
909911
.unwrap()
910912
.nth(i)
911913
.unwrap(),
@@ -924,7 +926,8 @@ where
924926
ty::Adt(def, args) => {
925927
match this.variants {
926928
Variants::Single { index } => {
927-
let field = &def.variant(index).fields[FieldIdx::from_usize(i)];
929+
let field =
930+
&def.variant(index.unwrap()).fields[FieldIdx::from_usize(i)];
928931
TyMaybeWithLayout::Ty(field.ty(tcx, args))
929932
}
930933

‎compiler/rustc_mir_transform/src/jump_threading.rs

+7-24
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
//! Likewise, applying the optimisation can create a lot of new MIR, so we bound the instruction
3636
//! cost by `MAX_COST`.
3737
38-
use rustc_abi::{TagEncoding, Variants};
3938
use rustc_arena::DroplessArena;
4039
use rustc_const_eval::const_eval::DummyMachine;
4140
use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, Projectable};
@@ -565,31 +564,15 @@ impl<'a, 'tcx> TOFinder<'a, 'tcx> {
565564
StatementKind::SetDiscriminant { box place, variant_index } => {
566565
let Some(discr_target) = self.map.find_discr(place.as_ref()) else { return };
567566
let enum_ty = place.ty(self.body, self.tcx).ty;
568-
// `SetDiscriminant` may be a no-op if the assigned variant is the untagged variant
569-
// of a niche encoding. If we cannot ensure that we write to the discriminant, do
570-
// nothing.
571-
let Ok(enum_layout) = self.ecx.layout_of(enum_ty) else {
567+
// `SetDiscriminant` guarantees that the discriminant is now `variant_index`.
568+
// Even if the discriminant write does nothing due to niches, it is UB to set the
569+
// discriminant when the data does not encode the desired discriminant.
570+
let Some(discr) =
571+
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
572+
else {
572573
return;
573574
};
574-
let writes_discriminant = match enum_layout.variants {
575-
Variants::Single { index } => {
576-
assert_eq!(index, *variant_index);
577-
true
578-
}
579-
Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => true,
580-
Variants::Multiple {
581-
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
582-
..
583-
} => *variant_index != untagged_variant,
584-
};
585-
if writes_discriminant {
586-
let Some(discr) =
587-
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
588-
else {
589-
return;
590-
};
591-
self.process_immediate(bb, discr_target, discr, state);
592-
}
575+
self.process_immediate(bb, discr_target, discr, state);
593576
}
594577
// If we expect `lhs ?= true`, we have an opportunity if we assume `lhs == true`.
595578
StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(

‎compiler/rustc_mir_transform/src/unreachable_enum_branching.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ fn variant_discriminants<'tcx>(
5454
tcx: TyCtxt<'tcx>,
5555
) -> FxHashSet<u128> {
5656
match &layout.variants {
57-
Variants::Single { index } => {
57+
Variants::Single { index: None } => {
58+
// Uninhabited, no valid discriminant.
59+
FxHashSet::default()
60+
}
61+
Variants::Single { index: Some(index) } => {
5862
let mut res = FxHashSet::default();
5963
res.insert(
6064
ty.discriminant_for_variant(tcx, *index)

‎compiler/rustc_smir/src/rustc_smir/convert/abi.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ impl<'tcx> Stable<'tcx> for rustc_abi::Variants<rustc_abi::FieldIdx, rustc_abi::
164164

165165
fn stable(&self, tables: &mut Tables<'_>) -> Self::T {
166166
match self {
167-
rustc_abi::Variants::Single { index } => {
168-
VariantsShape::Single { index: index.stable(tables) }
169-
}
167+
rustc_abi::Variants::Single { index } => VariantsShape::Single {
168+
index: index.unwrap_or(rustc_abi::VariantIdx::from_u32(0)).stable(tables),
169+
},
170170
rustc_abi::Variants::Multiple { tag, tag_encoding, tag_field, variants } => {
171171
VariantsShape::Multiple {
172172
tag: tag.stable(tables),

0 commit comments

Comments
 (0)