From 4eb5fcb09c76d9aa72340b7eae8d8f0a1cbd024b Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 2 Oct 2019 10:39:35 +0200 Subject: [PATCH 1/4] Compute the layout of uninhabited structs --- src/librustc/mir/interpret/error.rs | 6 ------ src/librustc/ty/layout.rs | 16 ++++++++++++---- src/librustc_mir/interpret/place.rs | 13 +++++-------- src/test/ui/issues/issue-64506.rs | 20 ++++++++++++++++++++ 4 files changed, 37 insertions(+), 18 deletions(-) create mode 100644 src/test/ui/issues/issue-64506.rs diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 71967b513a049..ac99ccd45eafe 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -389,10 +389,6 @@ pub enum UnsupportedOpInfo<'tcx> { /// Free-form case. Only for errors that are never caught! Unsupported(String), - /// FIXME(#64506) Error used to work around accessing projections of - /// uninhabited types. - UninhabitedValue, - // -- Everything below is not categorized yet -- FunctionAbiMismatch(Abi, Abi), FunctionArgMismatch(Ty<'tcx>, Ty<'tcx>), @@ -556,8 +552,6 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> { not a power of two"), Unsupported(ref msg) => write!(f, "{}", msg), - UninhabitedValue => - write!(f, "tried to use an uninhabited value"), } } } diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 6b22ded49f3fc..2751ce57e3e81 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -825,10 +825,18 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { }); (present_variants.next(), present_variants.next()) }; - if present_first.is_none() { - // Uninhabited because it has no variants, or only absent ones. - return tcx.layout_raw(param_env.and(tcx.types.never)); - } + let present_first = if present_first.is_none() { + if def.is_enum() { + // Uninhabited because it has no variants, or only absent ones. + return tcx.layout_raw(param_env.and(tcx.types.never)); + } else { + // if it's a struct, still compute a layout so that we can still compute the + // field offsets + Some(VariantIdx::new(0)) + } + } else { + present_first + }; let is_struct = !def.is_enum() || // Only one variant is present. diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 1166ca9bf2444..f57c180191d28 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -9,7 +9,7 @@ use rustc::mir; use rustc::mir::interpret::truncate; use rustc::ty::{self, Ty}; use rustc::ty::layout::{ - self, Size, Abi, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt + self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt }; use rustc::ty::TypeFoldable; @@ -377,20 +377,17 @@ where layout::FieldPlacement::Array { stride, .. } => { let len = base.len(self)?; if field >= len { - // This can be violated because this runs during promotion on code where the - // type system has not yet ensured that such things don't happen. + // This can be violated because the index (field) can be a runtime value + // provided by the user. debug!("tried to access element {} of array/slice with length {}", field, len); throw_panic!(BoundsCheck { len, index: field }); } stride * field } layout::FieldPlacement::Union(count) => { - // FIXME(#64506) `UninhabitedValue` can be removed when this issue is resolved - if base.layout.abi == Abi::Uninhabited { - throw_unsup!(UninhabitedValue); - } assert!(field < count as u64, - "Tried to access field {} of union with {} fields", field, count); + "Tried to access field {} of union {:#?} with {} fields", + field, base.layout, count); // Offset is always 0 Size::from_bytes(0) } diff --git a/src/test/ui/issues/issue-64506.rs b/src/test/ui/issues/issue-64506.rs new file mode 100644 index 0000000000000..db3e85a7bdfd1 --- /dev/null +++ b/src/test/ui/issues/issue-64506.rs @@ -0,0 +1,20 @@ +// check-pass + +#[derive(Copy, Clone)] +pub struct ChildStdin { + inner: AnonPipe, +} + +#[derive(Copy, Clone)] +enum AnonPipe {} + +const FOO: () = { + union Foo { + a: ChildStdin, + b: (), + } + let x = unsafe { Foo { b: () }.a }; + let x = &x.inner; +}; + +fn main() {} From 76a7667e792e4f4e4849ace0f6b016e2695a693c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 9 Oct 2019 19:09:08 +0200 Subject: [PATCH 2/4] Move test next to likeminded ones --- src/test/ui/{issues => consts}/issue-64506.rs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/test/ui/{issues => consts}/issue-64506.rs (100%) diff --git a/src/test/ui/issues/issue-64506.rs b/src/test/ui/consts/issue-64506.rs similarity index 100% rename from src/test/ui/issues/issue-64506.rs rename to src/test/ui/consts/issue-64506.rs From 76fe6a41ba156b59a78c162c0c1b0fadd02dd3f6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 9 Oct 2019 19:12:49 +0200 Subject: [PATCH 3/4] Refactor a nested `if` to a `match` --- src/librustc/ty/layout.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 2751ce57e3e81..d12194aefa6be 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -825,17 +825,13 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { }); (present_variants.next(), present_variants.next()) }; - let present_first = if present_first.is_none() { - if def.is_enum() { - // Uninhabited because it has no variants, or only absent ones. - return tcx.layout_raw(param_env.and(tcx.types.never)); - } else { - // if it's a struct, still compute a layout so that we can still compute the - // field offsets - Some(VariantIdx::new(0)) - } - } else { - present_first + let present_first = match present_first { + present_first @ Some(_) => present_first, + // Uninhabited because it has no variants, or only absent ones. + None if def.is_enum() => return tcx.layout_raw(param_env.and(tcx.types.never)), + // if it's a struct, still compute a layout so that we can still compute the + // field offsets + None => Some(VariantIdx::new(0)), }; let is_struct = !def.is_enum() || From 373c362b7ed0df2dfbc21853e53d5082f20d3de8 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 10 Oct 2019 18:21:34 +0200 Subject: [PATCH 4/4] Check that we don't access nonexisting union fields --- src/librustc_target/abi/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 26d37f196befa..fde5c5bed4d91 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -738,7 +738,11 @@ impl FieldPlacement { pub fn offset(&self, i: usize) -> Size { match *self { - FieldPlacement::Union(_) => Size::ZERO, + FieldPlacement::Union(count) => { + assert!(i < count, + "Tried to access field {} of union with {} fields", i, count); + Size::ZERO + }, FieldPlacement::Array { stride, count } => { let i = i as u64; assert!(i < count);