Skip to content

Commit eea0d6a

Browse files
committed
Disable dead variant removal for #[repr(C)] enums.
See rust-lang/unsafe-code-guidelines#500.
1 parent af98101 commit eea0d6a

File tree

3 files changed

+76
-2
lines changed

3 files changed

+76
-2
lines changed

Diff for: compiler/rustc_abi/src/layout.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ pub trait LayoutCalculator {
186186
let (present_first, present_second) = {
187187
let mut present_variants = variants
188188
.iter_enumerated()
189-
.filter_map(|(i, v)| if absent(v) { None } else { Some(i) });
189+
.filter_map(|(i, v)| if !repr.c() && absent(v) { None } else { Some(i) });
190190
(present_variants.next(), present_variants.next())
191191
};
192192
let present_first = match present_first {
@@ -621,7 +621,7 @@ where
621621
let discr_type = repr.discr_type();
622622
let bits = Integer::from_attr(dl, discr_type).size().bits();
623623
for (i, mut val) in discriminants {
624-
if variants[i].iter().any(|f| f.abi.is_uninhabited()) {
624+
if !repr.c() && variants[i].iter().any(|f| f.abi.is_uninhabited()) {
625625
continue;
626626
}
627627
if discr_type.is_signed() {

Diff for: compiler/rustc_abi/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,7 @@ pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
13881388
/// Single enum variants, structs/tuples, unions, and all non-ADTs.
13891389
Single { index: VariantIdx },
13901390

1391+
// REVIEW: probably this wording has to be adjusted somehow?
13911392
/// Enum-likes with more than one inhabited variant: each variant comes with
13921393
/// a *discriminant* (usually the same as the variant index but the user can
13931394
/// assign explicit discriminant values). That discriminant is encoded

Diff for: tests/ui/repr/repr-c-dead-variants.rs

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
//@ run-pass
2+
3+
#![allow(dead_code)]
4+
5+
use std::{alloc::Layout, ffi::c_int, mem::MaybeUninit, ptr};
6+
7+
// A simple uninhabited type.
8+
enum Void {}
9+
10+
#[repr(C)]
11+
enum Univariant<T> {
12+
Variant(T),
13+
}
14+
15+
#[repr(C, u8)]
16+
enum UnivariantU8<T> {
17+
Variant(T),
18+
}
19+
20+
#[repr(C)]
21+
enum TwoVariants<T> {
22+
Variant1(T),
23+
Variant2(u8),
24+
}
25+
26+
#[repr(C, u8)]
27+
enum TwoVariantsU8<T> {
28+
Variant1(T),
29+
Variant2(u8),
30+
}
31+
32+
#[repr(C, u8)]
33+
enum DeadBranchHasOtherField<T> {
34+
Variant1(T, u64),
35+
Variant2(u8),
36+
}
37+
38+
macro_rules! assert_layout_eq {
39+
($a:ty, $b:ty) => {
40+
assert_eq!(Layout::new::<$a>(), Layout::new::<$b>());
41+
};
42+
}
43+
44+
fn main() {
45+
// Compiler must not remove dead variants of `#[repr(C)]` ADTs.
46+
assert_layout_eq!(Univariant<Void>, c_int);
47+
// This should also hold for `#[repr(C, int)]` ADTs.
48+
assert_layout_eq!(UnivariantU8<Void>, u8);
49+
// And for ADTs with more than one variant.
50+
// These are twice the size: a tag plus the field in a second branch.
51+
assert_layout_eq!(TwoVariants<Void>, [c_int; 2]);
52+
assert_layout_eq!(TwoVariantsU8<Void>, [u8; 2]);
53+
// This one is 2 x u64: we reserve space for fields in a dead branch.
54+
assert_layout_eq!(DeadBranchHasOtherField<Void>, [u64; 2]);
55+
56+
// Some other useful invariants. See this UCG thread for more context:
57+
// https://github.com/rust-lang/unsafe-code-guidelines/issues/500
58+
assert_layout_eq!(Univariant<Void>, Univariant<MaybeUninit<Void>>);
59+
assert_layout_eq!(UnivariantU8<Void>, UnivariantU8<MaybeUninit<Void>>);
60+
assert_layout_eq!(TwoVariants<Void>, TwoVariants<MaybeUninit<Void>>);
61+
assert_layout_eq!(TwoVariantsU8<Void>, TwoVariantsU8<MaybeUninit<Void>>);
62+
assert_layout_eq!(DeadBranchHasOtherField<Void>, DeadBranchHasOtherField<MaybeUninit<Void>>);
63+
64+
// Check that discriminants are allocated properly.
65+
// SAFETY:
66+
// 1. We checked that layout requires proper alignment.
67+
// 2. Discriminant is guaranteed to be the first field of a `#[repr(C)]` enum.
68+
unsafe {
69+
assert_eq!(*ptr::from_ref(&TwoVariants::<Void>::Variant2(42)).cast::<c_int>(), 1);
70+
assert_eq!(*ptr::from_ref(&TwoVariantsU8::<Void>::Variant2(42)).cast::<u8>(), 1);
71+
assert_eq!(*ptr::from_ref(&DeadBranchHasOtherField::<Void>::Variant2(42)).cast::<u8>(), 1);
72+
}
73+
}

0 commit comments

Comments
 (0)