Skip to content

Commit

Permalink
Rollup merge of #96814 - RalfJung:enum-repr-align, r=oli-obk
Browse files Browse the repository at this point in the history
Fix repr(align) enum handling

`enum`, for better or worse, supports `repr(align)`. That has already caused a bug in #92464, which was "fixed" in #92932, but it turns out that that fix is wrong and caused #96185.

So this reverts #92932 (which fixes #96185), and attempts another strategy for fixing #92464: special-case enums when doing a cast, re-using the code to load the discriminant rather than assuming that the enum has scalar layout. This works fine for the interpreter.

However, #92464 contained another testcase that was previously not in the test suite -- and after adding it, it ICEs again. This is not surprising; codegen needs the same patch that I did in the interpreter. Probably this has to happen [around here](https://github.com/rust-lang/rust/blob/d32ce37a171663048a4c4a536803434e40f52bd6/compiler/rustc_codegen_ssa/src/mir/rvalue.rs#L276). Unfortunately I don't know how to do that -- the interpreter can load a discriminant from an operand, but codegen can only do that from a place. `@oli-obk` `@eddyb` `@bjorn3` any idea?
  • Loading branch information
GuillaumeGomez authored Jul 5, 2022
2 parents 0a7f2c3 + d5721ce commit 3e802d7
Show file tree
Hide file tree
Showing 4 changed files with 205 additions and 4 deletions.
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1418,9 +1418,9 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {

if layout_variants.iter().all(|v| v.abi.is_uninhabited()) {
abi = Abi::Uninhabited;
} else if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) {
// Without latter check aligned enums with custom discriminant values
// Would result in ICE see the issue #92464 for more info
} else if tag.size(dl) == size {
// Make sure we only use scalar layout when the enum is entirely its
// own tag (i.e. it has no padding nor any non-ZST variant fields).
abi = Abi::Scalar(tag);
} else {
// Try to use a ScalarPair for all tagged enums.
Expand Down
12 changes: 11 additions & 1 deletion src/test/ui/aligned_enum_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,15 @@ enum Aligned {
fn main() {
let aligned = Aligned::Zero;
let fo = aligned as u8;
println!("foo {}",fo);
println!("foo {}", fo);
assert_eq!(fo, 0);
println!("{}", tou8(Aligned::Zero));
assert_eq!(tou8(Aligned::Zero), 0);
}

#[inline(never)]
fn tou8(al: Aligned) -> u8 {
// Cast behind a function call so ConstProp does not see it
// (so that we can test codegen).
al as u8
}
19 changes: 19 additions & 0 deletions src/test/ui/layout/issue-96185-overaligned-enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
#![crate_type = "lib"]
#![feature(rustc_attrs)]

// This cannot use `Scalar` abi since there is padding.
#[rustc_layout(debug)]
#[repr(align(8))]
pub enum Aligned1 { //~ ERROR: layout_of
Zero = 0,
One = 1,
}

// This should use `Scalar` abi.
#[rustc_layout(debug)]
#[repr(align(1))]
pub enum Aligned2 { //~ ERROR: layout_of
Zero = 0,
One = 1,
}
172 changes: 172 additions & 0 deletions src/test/ui/layout/issue-96185-overaligned-enum.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
error: layout_of(Aligned1) = Layout {
fields: Arbitrary {
offsets: [
Size(0 bytes),
],
memory_index: [
0,
],
},
variants: Multiple {
tag: Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
tag_encoding: Direct,
tag_field: 0,
variants: [
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 0,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $PREF_ALIGN,
},
size: Size(8 bytes),
},
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 1,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $PREF_ALIGN,
},
size: Size(8 bytes),
},
],
},
abi: Aggregate {
sized: true,
},
largest_niche: Some(
Niche {
offset: Size(0 bytes),
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
),
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $PREF_ALIGN,
},
size: Size(8 bytes),
}
--> $DIR/issue-96185-overaligned-enum.rs:8:1
|
LL | pub enum Aligned1 {
| ^^^^^^^^^^^^^^^^^

error: layout_of(Aligned2) = Layout {
fields: Arbitrary {
offsets: [
Size(0 bytes),
],
memory_index: [
0,
],
},
variants: Multiple {
tag: Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
tag_encoding: Direct,
tag_field: 0,
variants: [
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 0,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(1 bytes),
pref: $PREF_ALIGN,
},
size: Size(1 bytes),
},
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 1,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(1 bytes),
pref: $PREF_ALIGN,
},
size: Size(1 bytes),
},
],
},
abi: Scalar(
Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
),
largest_niche: Some(
Niche {
offset: Size(0 bytes),
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
),
align: AbiAndPrefAlign {
abi: Align(1 bytes),
pref: $PREF_ALIGN,
},
size: Size(1 bytes),
}
--> $DIR/issue-96185-overaligned-enum.rs:16:1
|
LL | pub enum Aligned2 {
| ^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

0 comments on commit 3e802d7

Please sign in to comment.