Skip to content

Commit

Permalink
Unrolled build for rust-lang#123043
Browse files Browse the repository at this point in the history 
Rollup merge of rust-lang#123043 - GoldsteinE:fix/repr-c-dead-branches, r=oli-obk

Disable dead variant removal for `#[repr(C)]` enums.

This prevents removing dead branches from a `#[repr(C)]` enum (they now get discriminants allocated as if they were inhabited).

Implementation notes: ABI of something like

```rust
#[repr(C)]
enum Foo {
    Foo(!),
}
```

is still `Uninhabited`, but its layout is now computed as if all the branches were inhabited.
This seemed to me like a proper way to do it, especially given that ABI sanity check explicitly asserts that type-level uninhabitedness implies ABI uninhabitedness.

This probably needs some sort of FCP (given that it changes `#[repr(C)]` layout, which is a stable guarantee), but I’m not sure how to call for one or which team is the most relevant.

See rust-lang/unsafe-code-guidelines#500.
  • Loading branch information
@rust-timer
rust-timer authored Jul 4, 2024
2 parents 9f877c9 + 8a3c07a commit eea9cd6
Show file tree
Hide file tree
Showing 10 changed files with 1,549 additions and 3 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_abi/src/layout.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub trait LayoutCalculator {
let (present_first, present_second) = {
let mut present_variants = variants
.iter_enumerated()
.filter_map(|(i, v)| if absent(v) { None } else { Some(i) });
.filter_map(|(i, v)| if !repr.c() && absent(v) { None } else { Some(i) });
(present_variants.next(), present_variants.next())
};
let present_first = match present_first {
Expand Down Expand Up @@ -621,7 +621,7 @@ where
let discr_type = repr.discr_type();
let bits = Integer::from_attr(dl, discr_type).size().bits();
for (i, mut val) in discriminants {
if variants[i].iter().any(|f| f.abi.is_uninhabited()) {
if !repr.c() && variants[i].iter().any(|f| f.abi.is_uninhabited()) {
continue;
}
if discr_type.is_signed() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_abi/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,7 @@ pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
/// Single enum variants, structs/tuples, unions, and all non-ADTs.
Single { index: VariantIdx },

/// Enum-likes with more than one inhabited variant: each variant comes with
/// Enum-likes with more than one variant: each variant comes with
/// a *discriminant* (usually the same as the variant index but the user can
/// assign explicit discriminant values). That discriminant is encoded
/// as a *tag* on the machine. The layout of each variant is
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/abi/compatibility.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ test_abi_compatible!(zst_unit, Zst, ());
test_abi_compatible!(zst_array, Zst, [u8; 0]);
test_abi_compatible!(nonzero_int, NonZero<i32>, i32);

// `#[repr(C)]` enums should not change ABI based on individual variant inhabitedness.
// (However, this is *not* a guarantee. We only guarantee same layout, not same ABI.)
enum Void {}
test_abi_compatible!(repr_c_enum_void, ReprCEnum<Void>, ReprCEnum<ReprCUnion<Void>>);

// `DispatchFromDyn` relies on ABI compatibility.
// This is interesting since these types are not `repr(transparent)`. So this is not part of our
// public ABI guarantees, but is relied on by the compiler.
Expand Down
288 changes: 288 additions & 0 deletions tests/ui/repr/repr-c-dead-variants.aarch64-unknown-linux-gnu.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,288 @@
error: layout_of(Univariant) = Layout {
size: Size(4 bytes),
align: AbiAndPrefAlign {
abi: Align(4 bytes),
pref: $SOME_ALIGN,
},
abi: Uninhabited,
fields: Arbitrary {
offsets: [
Size(0 bytes),
],
memory_index: [
0,
],
},
largest_niche: Some(
Niche {
offset: Size(0 bytes),
value: Int(
I32,
false,
),
valid_range: 0..=0,
},
),
variants: Multiple {
tag: Initialized {
value: Int(
I32,
false,
),
valid_range: 0..=0,
},
tag_encoding: Direct,
tag_field: 0,
variants: [
Layout {
size: Size(4 bytes),
align: AbiAndPrefAlign {
abi: Align(4 bytes),
pref: $SOME_ALIGN,
},
abi: Uninhabited,
fields: Arbitrary {
offsets: [
Size(4 bytes),
],
memory_index: [
0,
],
},
largest_niche: None,
variants: Single {
index: 0,
},
max_repr_align: None,
unadjusted_abi_align: Align(4 bytes),
},
],
},
max_repr_align: None,
unadjusted_abi_align: Align(4 bytes),
}
--> $DIR/repr-c-dead-variants.rs:38:1
|
LL | enum Univariant {
| ^^^^^^^^^^^^^^^

error: layout_of(TwoVariants) = Layout {
size: Size(8 bytes),
align: AbiAndPrefAlign {
abi: Align(4 bytes),
pref: $SOME_ALIGN,
},
abi: ScalarPair(
Initialized {
value: Int(
I32,
false,
),
valid_range: 0..=1,
},
Union {
value: Int(
I8,
false,
),
},
),
fields: Arbitrary {
offsets: [
Size(0 bytes),
],
memory_index: [
0,
],
},
largest_niche: Some(
Niche {
offset: Size(0 bytes),
value: Int(
I32,
false,
),
valid_range: 0..=1,
},
),
variants: Multiple {
tag: Initialized {
value: Int(
I32,
false,
),
valid_range: 0..=1,
},
tag_encoding: Direct,
tag_field: 0,
variants: [
Layout {
size: Size(4 bytes),
align: AbiAndPrefAlign {
abi: Align(4 bytes),
pref: $SOME_ALIGN,
},
abi: Uninhabited,
fields: Arbitrary {
offsets: [
Size(4 bytes),
],
memory_index: [
0,
],
},
largest_niche: None,
variants: Single {
index: 0,
},
max_repr_align: None,
unadjusted_abi_align: Align(4 bytes),
},
Layout {
size: Size(8 bytes),
align: AbiAndPrefAlign {
abi: Align(4 bytes),
pref: $SOME_ALIGN,
},
abi: ScalarPair(
Initialized {
value: Int(
I32,
false,
),
valid_range: 0..=1,
},
Union {
value: Int(
I8,
false,
),
},
),
fields: Arbitrary {
offsets: [
Size(4 bytes),
],
memory_index: [
0,
],
},
largest_niche: None,
variants: Single {
index: 1,
},
max_repr_align: None,
unadjusted_abi_align: Align(4 bytes),
},
],
},
max_repr_align: None,
unadjusted_abi_align: Align(4 bytes),
}
--> $DIR/repr-c-dead-variants.rs:45:1
|
LL | enum TwoVariants {
| ^^^^^^^^^^^^^^^^

error: layout_of(DeadBranchHasOtherField) = Layout {
size: Size(16 bytes),
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $SOME_ALIGN,
},
abi: Aggregate {
sized: true,
},
fields: Arbitrary {
offsets: [
Size(0 bytes),
],
memory_index: [
0,
],
},
largest_niche: Some(
Niche {
offset: Size(0 bytes),
value: Int(
I32,
false,
),
valid_range: 0..=1,
},
),
variants: Multiple {
tag: Initialized {
value: Int(
I32,
false,
),
valid_range: 0..=1,
},
tag_encoding: Direct,
tag_field: 0,
variants: [
Layout {
size: Size(16 bytes),
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $SOME_ALIGN,
},
abi: Uninhabited,
fields: Arbitrary {
offsets: [
Size(8 bytes),
Size(8 bytes),
],
memory_index: [
0,
1,
],
},
largest_niche: None,
variants: Single {
index: 0,
},
max_repr_align: Some(
Align(8 bytes),
),
unadjusted_abi_align: Align(8 bytes),
},
Layout {
size: Size(16 bytes),
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $SOME_ALIGN,
},
abi: Aggregate {
sized: true,
},
fields: Arbitrary {
offsets: [
Size(8 bytes),
],
memory_index: [
0,
],
},
largest_niche: None,
variants: Single {
index: 1,
},
max_repr_align: None,
unadjusted_abi_align: Align(8 bytes),
},
],
},
max_repr_align: Some(
Align(8 bytes),
),
unadjusted_abi_align: Align(8 bytes),
}
--> $DIR/repr-c-dead-variants.rs:57:1
|
LL | enum DeadBranchHasOtherField {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

Loading

0 comments on commit eea9cd6

Please sign in to comment.