From a8db3d5d1a97e76a28cd08bfff9a3af95e1a4d9d Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 22 Nov 2017 21:05:37 +0100 Subject: [PATCH] fix(vm): Don't let rustc think the Array type can't exist THe use of `Void` in the `Array` type was meant to absolutely prevent the `Array` type from existing except behind a pointer which is invalid (except the empty array I guess) as the elements are stored inline behind the length field. Unfortunately https://github.com/rust-lang/rust/pull/45225 made rustc think that fields of the `Array` type could not exist at all and placed fields of `Array` at the start of any struct containing them, causing any write to the `Array` to overwrite other fields. --- vm/src/array.rs | 25 ++++++++++++++++---- vm/src/value.rs | 61 +++++++++++++++++++++++++------------------------ 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/vm/src/array.rs b/vm/src/array.rs index 61881772d8..b00ac0ef78 100644 --- a/vm/src/array.rs +++ b/vm/src/array.rs @@ -7,15 +7,12 @@ use std::slice; use gc::{Gc, Traverseable}; -enum Void {} - /// Abstract array type which have their length store inline with the elements. /// Fills the same role as Box<[T]> but takes only 8 bytes on the stack instead of 16 +#[repr(C)] pub struct Array { len: usize, array_start: [T; 0], - /// Prevents the array from being instantiated directly - _void: Void, } impl AsRef<[T]> for Array { @@ -125,3 +122,23 @@ impl<'a, T: Copy + 'a> IntoIterator for &'a mut Array { (&mut **self).into_iter() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn array_values_offset() { + use std::mem; + use std::ptr; + + unsafe { + let p: *const Array = ptr::null(); + assert_eq!(p as *const u8, &(*p).len as *const _ as *const u8); + assert_eq!( + (p as *const u8).offset(mem::size_of::() as isize), + &(*p).array_start as *const _ as *const u8 + ); + } + } +} diff --git a/vm/src/value.rs b/vm/src/value.rs index 1729d0c79e..bafd4f63fb 100644 --- a/vm/src/value.rs +++ b/vm/src/value.rs @@ -36,6 +36,7 @@ impl PartialEq for Userdata { } #[derive(Debug, PartialEq)] +#[repr(C)] pub struct ClosureData { pub function: GcPtr, pub upvars: Array, @@ -110,12 +111,9 @@ pub struct BytecodeFunction { pub instructions: Vec, #[cfg_attr(feature = "serde_derive", serde(state))] pub inner_functions: Vec>, - #[cfg_attr(feature = "serde_derive", serde(state))] - pub strings: Vec, - #[cfg_attr(feature = "serde_derive", serde(state))] - pub records: Vec>, - #[cfg_attr(feature = "serde_derive", serde(state))] - pub debug_info: DebugInfo, + #[cfg_attr(feature = "serde_derive", serde(state))] pub strings: Vec, + #[cfg_attr(feature = "serde_derive", serde(state))] pub records: Vec>, + #[cfg_attr(feature = "serde_derive", serde(state))] pub debug_info: DebugInfo, } impl Traverseable for BytecodeFunction { @@ -125,6 +123,7 @@ impl Traverseable for BytecodeFunction { } #[derive(Debug)] +#[repr(C)] pub struct DataStruct { tag: VmTag, pub fields: Array, @@ -279,10 +278,7 @@ pub enum Value { Byte(u8), Int(VmInt), Float(f64), - String( - #[cfg_attr(feature = "serde_derive", serde(deserialize_state))] - GcStr, - ), + String(#[cfg_attr(feature = "serde_derive", serde(deserialize_state))] GcStr), Tag(VmTag), Data( #[cfg_attr(feature = "serde_derive", @@ -296,10 +292,7 @@ pub enum Value { #[cfg_attr(feature = "serde_derive", serde(serialize_state))] GcPtr, ), - Function( - #[cfg_attr(feature = "serde_derive", serde(state))] - GcPtr, - ), + Function(#[cfg_attr(feature = "serde_derive", serde(state))] GcPtr), Closure( #[cfg_attr(feature = "serde_derive", serde(state_with = "::serialization::closure"))] GcPtr, @@ -319,10 +312,7 @@ pub enum Value { ), #[cfg_attr(feature = "serde_derive", serde(skip_deserializing))] #[cfg_attr(feature = "serde_derive", serde(skip_serializing))] - Thread( - #[cfg_attr(feature = "serde_derive", serde(deserialize_state))] - GcPtr, - ), + Thread(#[cfg_attr(feature = "serde_derive", serde(deserialize_state))] GcPtr), } impl Value { @@ -541,10 +531,7 @@ pub enum Callable { #[cfg_attr(feature = "serde_derive", serde(state_with = "::serialization::closure"))] GcPtr, ), - Extern( - #[cfg_attr(feature = "serde_derive", serde(state))] - GcPtr, - ), + Extern(#[cfg_attr(feature = "serde_derive", serde(state))] GcPtr), } impl Callable { @@ -573,19 +560,18 @@ impl Traverseable for Callable { fn traverse(&self, gc: &mut Gc) { match *self { Callable::Closure(ref closure) => closure.traverse(gc), - Callable::Extern(_) => (), + Callable::Extern(ref ext) => ext.traverse(gc), } } } #[derive(Debug)] +#[repr(C)] #[cfg_attr(feature = "serde_derive", derive(SerializeState))] #[cfg_attr(feature = "serde_derive", serde(serialize_state = "::serialization::SeSeed"))] pub struct PartialApplicationData { - #[cfg_attr(feature = "serde_derive", serde(serialize_state))] - pub function: Callable, - #[cfg_attr(feature = "serde_derive", serde(serialize_state))] - pub args: Array, + #[cfg_attr(feature = "serde_derive", serde(serialize_state))] pub function: Callable, + #[cfg_attr(feature = "serde_derive", serde(serialize_state))] pub args: Array, } impl PartialEq for PartialApplicationData { @@ -734,8 +720,8 @@ impl Clone for ExternFunction { impl PartialEq for ExternFunction { fn eq(&self, other: &ExternFunction) -> bool { - self.id == other.id && self.args == other.args && - self.function as usize == other.function as usize + self.id == other.id && self.args == other.args + && self.function as usize == other.function as usize } } @@ -865,7 +851,7 @@ macro_rules! on_array { } } - +#[repr(C)] pub struct ValueArray { repr: Repr, array: Array<()>, @@ -1384,4 +1370,19 @@ mod tests { "[1, 2, 3]" ); } + + #[test] + fn closure_data_upvars_location() { + use std::mem; + use std::ptr; + + unsafe { + let p: *const ClosureData = ptr::null(); + assert_eq!(p as *const u8, &(*p).function as *const _ as *const u8); + assert_eq!( + (p as *const u8).offset(mem::size_of::<*const ()>() as isize), + &(*p).upvars as *const _ as *const u8 + ); + } + } }