Skip to content

Commit fb662f2

Browse files
committed
safe transmute: support Variants::Single enums
Previously, the implementation of `Tree::from_enum` incorrectly treated enums with `Variants::Single` and `Variants::Multiple` identically. This is incorrect for `Variants::Single` enums, which delegate their layout to that of a variant with a particular index (or no variant at all if the enum is empty). This flaw manifested first as an ICE. `Tree::from_enum` attempted to compute the tag of variants other than the one at `Variants::Single`'s `index`, and fell afoul of a sanity-checking assertion in `compiler/rustc_const_eval/src/interpret/discriminant.rs`. This assertion is non-load-bearing, and can be removed; the routine its in is well-behaved even without it. With the assertion removed, the proximate issue becomes apparent: calling `Tree::from_variant` on a variant that does not exist is ill-defined. A sanity check the given variant has `FieldShapes::Arbitrary` fails, and the analysis is (correctly) aborted with `Err::NotYetSupported`. This commit corrects this chain of failures by ensuring that `Tree::from_variant` is not called on variants that are, as far as layout is concerned, nonexistent. Specifically, the implementation of `Tree::from_enum` is now partitioned into three cases: 1. enums that are uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants `Tree::from_variant` is now only invoked in the third case. In the first case, `Tree::uninhabited()` is produced. In the second case, the layout is delegated to `Variants::Single`'s index. Fixes #125811
1 parent 1d43fbb commit fb662f2

File tree

4 files changed

+81
-59
lines changed

4 files changed

+81
-59
lines changed

compiler/rustc_const_eval/src/interpret/discriminant.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
241241
variant_index: VariantIdx,
242242
) -> InterpResult<'tcx, Option<(ScalarInt, usize)>> {
243243
match self.layout_of(ty)?.variants {
244-
abi::Variants::Single { index } => {
245-
assert_eq!(index, variant_index);
246-
Ok(None)
247-
}
244+
abi::Variants::Single { .. } => Ok(None),
248245

249246
abi::Variants::Multiple {
250247
tag_encoding: TagEncoding::Direct,

compiler/rustc_transmute/src/layout/tree.rs

+54-21
Original file line numberDiff line numberDiff line change
@@ -331,30 +331,63 @@ pub(crate) mod rustc {
331331
assert!(def.is_enum());
332332
let layout = ty_and_layout.layout;
333333

334-
if let Variants::Multiple { tag_field, .. } = layout.variants() {
335-
// For enums (but not coroutines), the tag field is
336-
// currently always the first field of the layout.
337-
assert_eq!(*tag_field, 0);
338-
}
334+
// Computes the variant of a given index.
335+
let layout_of_variant = |index| {
336+
let tag = cx.tcx.tag_for_variant((ty_and_layout.ty, index));
337+
let variant_def = Def::Variant(def.variant(index));
338+
let variant_ty_and_layout = ty_and_layout.for_variant(&cx, index);
339+
Self::from_variant(variant_def, tag, variant_ty_and_layout, layout.size, cx)
340+
};
339341

340-
let variants = def.discriminants(cx.tcx()).try_fold(
341-
Self::uninhabited(),
342-
|variants, (idx, ref discriminant)| {
343-
let tag = cx.tcx.tag_for_variant((ty_and_layout.ty, idx));
344-
let variant_def = Def::Variant(def.variant(idx));
345-
let variant_ty_and_layout = ty_and_layout.for_variant(&cx, idx);
346-
let variant = Self::from_variant(
347-
variant_def,
348-
tag,
349-
variant_ty_and_layout,
350-
layout.size,
351-
cx,
342+
// We consider three kinds of enums, each demanding a different
343+
// treatment of their layout computation:
344+
// 1. enums that are uninhabited
345+
// 2. enums for which all but one variant is uninhabited
346+
// 3. enums with multiple inhabited variants
347+
match layout.variants() {
348+
_ if layout.abi.is_uninhabited() => {
349+
// Uninhabited enums are usually (always?) zero-sized. In
350+
// the (unlikely?) event that an uninhabited enum is
351+
// non-zero-sized, this assert will trigger an ICE, and this
352+
// code should be modified such that a `layout.size` amount
353+
// of uninhabited bytes is returned instead.
354+
//
355+
// Uninhabited enums are currently implemented such that
356+
// their layout is described with `Variants::Single`, even
357+
// though they don't necessarily have a 'single' variant to
358+
// defer to. That said, we don't bother specifically
359+
// matching on `Variants::Single` in this arm because the
360+
// behavioral principles here remain true even if, for
361+
// whatever reason, the compiler describes an uninhabited
362+
// enum with `Variants::Multiple`.
363+
assert_eq!(layout.size, Size::ZERO);
364+
Ok(Self::uninhabited())
365+
}
366+
Variants::Single { index } => {
367+
// `Variants::Single` on non-uninhabited enums denotes that
368+
// the enum delegates its layout to the variant at `index`.
369+
layout_of_variant(*index)
370+
}
371+
Variants::Multiple { tag_field, .. } => {
372+
// `Variants::Multiple` denotes an enum with multiple
373+
// inhabited variants. The layout of such an enum is the
374+
// disjunction of the layouts of its tagged variants.
375+
376+
// For enums (but not coroutines), the tag field is
377+
// currently always the first field of the layout.
378+
assert_eq!(*tag_field, 0);
379+
380+
let variants = def.discriminants(cx.tcx()).try_fold(
381+
Self::uninhabited(),
382+
|variants, (idx, ref discriminant)| {
383+
let variant = layout_of_variant(idx)?;
384+
Result::<Self, Err>::Ok(variants.or(variant))
385+
},
352386
)?;
353-
Result::<Self, Err>::Ok(variants.or(variant))
354-
},
355-
)?;
356387

357-
return Ok(Self::def(Def::Adt(def)).then(variants));
388+
return Ok(Self::def(Def::Adt(def)).then(variants));
389+
}
390+
}
358391
}
359392

360393
/// Constructs a `Tree` from a 'variant-like' layout.

tests/crashes/125811.rs

-34
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//@ check-pass
2+
//! Tests that we do not regress rust-lang/rust#125811
3+
#![feature(transmutability)]
4+
5+
fn assert_transmutable<T>()
6+
where
7+
(): std::mem::BikeshedIntrinsicFrom<T>
8+
{}
9+
10+
enum Uninhabited {}
11+
12+
enum SingleInhabited {
13+
X,
14+
Y(Uninhabited)
15+
}
16+
17+
enum SingleUninhabited {
18+
X(Uninhabited),
19+
Y(Uninhabited),
20+
}
21+
22+
fn main() {
23+
assert_transmutable::<Uninhabited>();
24+
assert_transmutable::<SingleInhabited>();
25+
assert_transmutable::<SingleUninhabited>();
26+
}

0 commit comments

Comments
 (0)