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 rust-lang/rust#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.
gluon-lang · Nov 22, 2017 · a8db3d5 · a8db3d5
1 parent 5347278
commit a8db3d5
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 34 deletions.
diff --git 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<T: Copy> {
     len: usize,
     array_start: [T; 0],
-    /// Prevents the array from being instantiated directly
-    _void: Void,
 }
 
 impl<T: Copy> AsRef<[T]> for Array<T> {
@@ -125,3 +122,23 @@ impl<'a, T: Copy + 'a> IntoIterator for &'a mut Array<T> {
         (&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<i32> = 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::<usize>() as isize),
+                &(*p).array_start as *const _ as *const u8
+            );
+        }
+    }
+}
diff --git 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<BytecodeFunction>,
     pub upvars: Array<Value>,
@@ -110,12 +111,9 @@ pub struct BytecodeFunction {
     pub instructions: Vec<Instruction>,
     #[cfg_attr(feature = "serde_derive", serde(state))]
     pub inner_functions: Vec<GcPtr<BytecodeFunction>>,
-    #[cfg_attr(feature = "serde_derive", serde(state))]
-    pub strings: Vec<InternedStr>,
-    #[cfg_attr(feature = "serde_derive", serde(state))]
-    pub records: Vec<Vec<InternedStr>>,
-    #[cfg_attr(feature = "serde_derive", serde(state))]
-    pub debug_info: DebugInfo,
+    #[cfg_attr(feature = "serde_derive", serde(state))] pub strings: Vec<InternedStr>,
+    #[cfg_attr(feature = "serde_derive", serde(state))] pub records: Vec<Vec<InternedStr>>,
+    #[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<Value>,
@@ -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<ValueArray>,
     ),
-    Function(
-        #[cfg_attr(feature = "serde_derive", serde(state))]
-        GcPtr<ExternFunction>,
-    ),
+    Function(#[cfg_attr(feature = "serde_derive", serde(state))] GcPtr<ExternFunction>),
     Closure(
         #[cfg_attr(feature = "serde_derive", serde(state_with = "::serialization::closure"))]
         GcPtr<ClosureData>,
@@ -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>,
-    ),
+    Thread(#[cfg_attr(feature = "serde_derive", serde(deserialize_state))] GcPtr<Thread>),
 }
 
 impl Value {
@@ -541,10 +531,7 @@ pub enum Callable {
         #[cfg_attr(feature = "serde_derive", serde(state_with = "::serialization::closure"))]
         GcPtr<ClosureData>,
     ),
-    Extern(
-        #[cfg_attr(feature = "serde_derive", serde(state))]
-        GcPtr<ExternFunction>,
-    ),
+    Extern(#[cfg_attr(feature = "serde_derive", serde(state))] GcPtr<ExternFunction>),
 }
 
 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<Value>,
+    #[cfg_attr(feature = "serde_derive", serde(serialize_state))] pub function: Callable,
+    #[cfg_attr(feature = "serde_derive", serde(serialize_state))] pub args: Array<Value>,
 }
 
 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
+            );
+        }
+    }
 }