From e753d2105159397eff162aa3f1f7715f96b5772d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Nov 2018 11:23:34 +0100 Subject: [PATCH 1/7] miri: accept extern types in structs if they are the only field --- src/librustc_mir/interpret/eval_context.rs | 15 ++++++++++++-- src/librustc_mir/interpret/place.rs | 3 ++- src/test/ui/consts/const-eval/issue-55541.rs | 21 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/consts/const-eval/issue-55541.rs diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index bc7ad16dc97bc..48a8d0bbe56d6 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -371,8 +371,19 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc // the last field). Can't have foreign types here, how would we // adjust alignment and size for them? let field = layout.field(self, layout.fields.count() - 1)?; - let (unsized_size, unsized_align) = self.size_and_align_of(metadata, field)? - .expect("Fields cannot be extern types"); + let (unsized_size, unsized_align) = match self.size_and_align_of(metadata, field)? { + Some(size_and_align) => size_and_align, + None => { + // A field with extern type. If this is the only field, + // we treat this struct just the same. Else, this is an error + // (for now). + if layout.fields.count() == 1 { + return Ok(None) + } else { + bug!("Fields cannot be extern types, unless they are the only field") + } + } + }; // FIXME (#26403, #27023): We should be adding padding // to `sized_size` (to accommodate the `unsized_align` diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 3b104e2284fe2..8bd29aff841cf 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -354,7 +354,8 @@ where let (meta, offset) = if field_layout.is_unsized() { // re-use parent metadata to determine dynamic field layout let (_, align) = self.size_and_align_of(base.meta, field_layout)? - .expect("Fields cannot be extern types"); + // If this is an extern type, we fall back to its static size and alignment. + .unwrap_or_else(|| base.layout.size_and_align()); (base.meta, offset.abi_align(align)) } else { // base.meta could be present; we might be accessing a sized field of an unsized diff --git a/src/test/ui/consts/const-eval/issue-55541.rs b/src/test/ui/consts/const-eval/issue-55541.rs new file mode 100644 index 0000000000000..bf8965e836182 --- /dev/null +++ b/src/test/ui/consts/const-eval/issue-55541.rs @@ -0,0 +1,21 @@ +// compile-pass + +// Test that we can handle newtypes wrapping extern types + +#![feature(extern_types, const_transmute)] + +extern "C" { + pub type ExternType; +} +unsafe impl Sync for ExternType {} + +#[repr(transparent)] +pub struct Wrapper(ExternType); + +static MAGIC_FFI_STATIC: u8 = 42; + +pub static MAGIC_FFI_REF: &'static Wrapper = unsafe { + std::mem::transmute(&MAGIC_FFI_STATIC) +}; + +fn main() {} From aca76d42a05c419a539d9b34fa30b88d4cdcadcc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Nov 2018 11:55:39 +0100 Subject: [PATCH 2/7] test for offset and alignment of the sized part, instead of field count --- src/librustc_mir/interpret/eval_context.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 48a8d0bbe56d6..c1c2efdae7516 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -374,13 +374,13 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc let (unsized_size, unsized_align) = match self.size_and_align_of(metadata, field)? { Some(size_and_align) => size_and_align, None => { - // A field with extern type. If this is the only field, - // we treat this struct just the same. Else, this is an error - // (for now). - if layout.fields.count() == 1 { + // A field with extern type. If this field is at offset 0 and the sized + // part makes no alignment constraints, we behave like the underlying + // extern type. + if sized_size == Size::ZERO && sized_align.abi() == 1 { return Ok(None) } else { - bug!("Fields cannot be extern types, unless they are the only field") + bug!("Fields cannot be extern types, unless they are at offset 0") } } }; From bb17f717499132a23e1b54bf3fdd0727c09715ff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Nov 2018 14:43:05 +0100 Subject: [PATCH 3/7] also test with PhantomData --- src/test/ui/consts/const-eval/issue-55541.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/ui/consts/const-eval/issue-55541.rs b/src/test/ui/consts/const-eval/issue-55541.rs index bf8965e836182..611fb89341de4 100644 --- a/src/test/ui/consts/const-eval/issue-55541.rs +++ b/src/test/ui/consts/const-eval/issue-55541.rs @@ -4,18 +4,24 @@ #![feature(extern_types, const_transmute)] +use std::marker::PhantomData; + extern "C" { pub type ExternType; } unsafe impl Sync for ExternType {} +static MAGIC_FFI_STATIC: u8 = 42; #[repr(transparent)] pub struct Wrapper(ExternType); - -static MAGIC_FFI_STATIC: u8 = 42; - pub static MAGIC_FFI_REF: &'static Wrapper = unsafe { std::mem::transmute(&MAGIC_FFI_STATIC) }; +#[repr(transparent)] +pub struct Wrapper2(PhantomData>, ExternType); +pub static MAGIC_FFI_REF2: &'static Wrapper2 = unsafe { + std::mem::transmute(&MAGIC_FFI_STATIC) +}; + fn main() {} From 5ebd077f54c735aeb634d18080c9f127016f1c87 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Nov 2018 11:14:21 +0100 Subject: [PATCH 4/7] make it more obvious that the size is not relevant --- src/librustc_mir/interpret/place.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 8bd29aff841cf..3dcd081edf9d9 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -353,9 +353,10 @@ where // Offset may need adjustment for unsized fields let (meta, offset) = if field_layout.is_unsized() { // re-use parent metadata to determine dynamic field layout - let (_, align) = self.size_and_align_of(base.meta, field_layout)? - // If this is an extern type, we fall back to its static size and alignment. - .unwrap_or_else(|| base.layout.size_and_align()); + let align = self.size_and_align_of(base.meta, field_layout)? + .map(|(_, align)| align) + // If this is an extern type, we fall back to its static alignment. + .unwrap_or_else(|| base.layout.align); (base.meta, offset.abi_align(align)) } else { // base.meta could be present; we might be accessing a sized field of an unsized From a6622c265c9a359c277af576c4849a74d476f597 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Nov 2018 11:20:01 +0100 Subject: [PATCH 5/7] note some FIXMEs --- src/librustc_mir/interpret/eval_context.rs | 3 +++ src/librustc_mir/interpret/place.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index c1c2efdae7516..797e0458e5312 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -377,6 +377,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc // A field with extern type. If this field is at offset 0 and the sized // part makes no alignment constraints, we behave like the underlying // extern type. + // FIXME: Once we have made decisions for how to handle size and alignment + // of `extern type`, this should be adapted. It is just a temporary hack + // to get some code to work that probably ought to work. if sized_size == Size::ZERO && sized_align.abi() == 1 { return Ok(None) } else { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 3dcd081edf9d9..c60ae7b4a00c4 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -356,6 +356,9 @@ where let align = self.size_and_align_of(base.meta, field_layout)? .map(|(_, align)| align) // If this is an extern type, we fall back to its static alignment. + // FIXME: Once we have made decisions for how to handle size and alignment + // of `extern type`, this should be adapted. It is just a temporary hack + // to get some code to work that probably ought to work. .unwrap_or_else(|| base.layout.align); (base.meta, offset.abi_align(align)) } else { From ba0bab39e04a13ad996e41a2ef2ca9b83fbb2cf4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Nov 2018 08:41:56 +0100 Subject: [PATCH 6/7] make sure we only guess field alignment at offset 0 --- src/librustc_mir/interpret/eval_context.rs | 7 +++---- src/librustc_mir/interpret/place.rs | 18 +++++++++++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 797e0458e5312..cca4e8a3ce31a 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -374,13 +374,12 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc let (unsized_size, unsized_align) = match self.size_and_align_of(metadata, field)? { Some(size_and_align) => size_and_align, None => { - // A field with extern type. If this field is at offset 0 and the sized - // part makes no alignment constraints, we behave like the underlying - // extern type. + // A field with extern type. If this field is at offset 0, we behave + // like the underlying extern type. // FIXME: Once we have made decisions for how to handle size and alignment // of `extern type`, this should be adapted. It is just a temporary hack // to get some code to work that probably ought to work. - if sized_size == Size::ZERO && sized_align.abi() == 1 { + if sized_size == Size::ZERO { return Ok(None) } else { bug!("Fields cannot be extern types, unless they are at offset 0") diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index c60ae7b4a00c4..a836a199f768d 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -353,13 +353,17 @@ where // Offset may need adjustment for unsized fields let (meta, offset) = if field_layout.is_unsized() { // re-use parent metadata to determine dynamic field layout - let align = self.size_and_align_of(base.meta, field_layout)? - .map(|(_, align)| align) - // If this is an extern type, we fall back to its static alignment. - // FIXME: Once we have made decisions for how to handle size and alignment - // of `extern type`, this should be adapted. It is just a temporary hack - // to get some code to work that probably ought to work. - .unwrap_or_else(|| base.layout.align); + let align = match self.size_and_align_of(base.meta, field_layout)? { + Some((_, align)) => align, + None if offset == Size::ZERO => + // An extern type at offset 0, we fall back to its static alignment. + // FIXME: Once we have made decisions for how to handle size and alignment + // of `extern type`, this should be adapted. It is just a temporary hack + // to get some code to work that probably ought to work. + field_layout.align, + None => + bug!("Cannot compute offset for extern type field at non-0 offset"), + }; (base.meta, offset.abi_align(align)) } else { // base.meta could be present; we might be accessing a sized field of an unsized From 86073253d563a7792765933f24acbf93fd3fee1d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 18 Nov 2018 19:08:06 -0800 Subject: [PATCH 7/7] Increase `Duration` approximate equal threshold to 1us Previously this threshold when testing was 100ns, but the Windows documentation states: > which is a high resolution (<1us) time stamp which presumably means that we could have up to 1us resolution, which means that 100ns doesn't capture "equivalent" time intervals due to various bits of rounding here and there. It's hoped that this.. Closes #56034 --- src/libstd/time.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/time.rs b/src/libstd/time.rs index d124cf53dda5d..58be1972a8121 100644 --- a/src/libstd/time.rs +++ b/src/libstd/time.rs @@ -481,7 +481,7 @@ mod tests { let (a, b) = ($a, $b); if a != b { let (a, b) = if a > b {(a, b)} else {(b, a)}; - assert!(a - Duration::new(0, 100) <= b, + assert!(a - Duration::new(0, 1000) <= b, "{:?} is not almost equal to {:?}", a, b); } })