about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGuillaume Gomez <guillaume1.gomez@gmail.com>2022-07-05 23:43:30 +0200
committerGitHub <noreply@github.com>2022-07-05 23:43:30 +0200
commit3e802d72bbe3b4f4ec200dd9a6110e530ed4fbb9 (patch)
tree8ead3384987de6799cc312168e6d88b070dd3c04
parent0a7f2c3a025c8bab1e10ccec6208a4c19b057b26 (diff)
parentd5721ce3a0f0d8eb2c46d87440b1977b5aef972c (diff)
downloadrust-3e802d72bbe3b4f4ec200dd9a6110e530ed4fbb9.tar.gz
rust-3e802d72bbe3b4f4ec200dd9a6110e530ed4fbb9.zip
Rollup merge of #96814 - RalfJung:enum-repr-align, r=oli-obk
Fix repr(align) enum handling

`enum`, for better or worse, supports `repr(align)`. That has already caused a bug in https://github.com/rust-lang/rust/issues/92464, which was "fixed" in https://github.com/rust-lang/rust/pull/92932, but it turns out that that fix is wrong and caused https://github.com/rust-lang/rust/issues/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?
-rw-r--r--compiler/rustc_middle/src/ty/layout.rs6
-rw-r--r--src/test/ui/aligned_enum_cast.rs12
-rw-r--r--src/test/ui/layout/issue-96185-overaligned-enum.rs19
-rw-r--r--src/test/ui/layout/issue-96185-overaligned-enum.stderr172
4 files changed, 205 insertions, 4 deletions
diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs
index 3b05e42a53e..e20f94b15c6 100644
--- a/compiler/rustc_middle/src/ty/layout.rs
+++ b/compiler/rustc_middle/src/ty/layout.rs
@@ -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.
diff --git a/src/test/ui/aligned_enum_cast.rs b/src/test/ui/aligned_enum_cast.rs
index 4b5776a6aa8..1ddf127172e 100644
--- a/src/test/ui/aligned_enum_cast.rs
+++ b/src/test/ui/aligned_enum_cast.rs
@@ -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
 }
diff --git a/src/test/ui/layout/issue-96185-overaligned-enum.rs b/src/test/ui/layout/issue-96185-overaligned-enum.rs
new file mode 100644
index 00000000000..ae1e6b012c3
--- /dev/null
+++ b/src/test/ui/layout/issue-96185-overaligned-enum.rs
@@ -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,
+}
diff --git a/src/test/ui/layout/issue-96185-overaligned-enum.stderr b/src/test/ui/layout/issue-96185-overaligned-enum.stderr
new file mode 100644
index 00000000000..8dc364fa7c9
--- /dev/null
+++ b/src/test/ui/layout/issue-96185-overaligned-enum.stderr
@@ -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
+