Skip to content

Commit

Permalink
require object types to have an alignment of 8 for computing offsets
Browse files Browse the repository at this point in the history
fixes #80
  • Loading branch information
y21 committed Mar 3, 2024
1 parent 530e010 commit e352f58
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
20 changes: 17 additions & 3 deletions crates/dash_vm/src/gc/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ pub struct ObjectVTable {
pub(crate) js_type_of: unsafe fn(*const ()) -> Typeof,
}

#[repr(C, align(8))]
#[doc(hidden)]
pub struct Align8<T>(pub T);

#[repr(C)]
pub struct GcNode<T> {
pub(crate) vtable: &'static ObjectVTable,
Expand All @@ -90,7 +94,17 @@ pub struct GcNode<T> {
pub(crate) refcount: Cell<u64>,
/// The next pointer in the linked list of nodes
pub(crate) next: Option<NonNull<GcNode<()>>>,
pub(crate) value: T,

/// The value. Typically seen in its type-erased form `()`.
///
/// IMPORTANT: since we are using `GcNode<()>` for offsetting the `value` field
/// but we've previously allocated the `GcNode` with a concrete type, we must make
/// sure that the alignment is <= the max alignment of all other fields, since
/// that could otherwise cause `value` in `GcNode<Concrete>` to have a different offset.
/// `Align8` on its own does not guarantee a maximum alignment, instead this condition is checked in `register_gc!`.
///
/// It must also be the last field, since offsetting the other fields would be wrong. This `T` is really `!Sized` in disguise
pub(crate) value: Align8<T>,
}

#[repr(C)]
Expand All @@ -99,7 +113,7 @@ pub struct Handle(NonNull<GcNode<()>>);

impl Debug for Handle {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
unsafe { (self.vtable().debug_fmt)(addr_of!((*self.0.as_ptr()).value), f) }
unsafe { (self.vtable().debug_fmt)(addr_of!((*self.0.as_ptr()).value.0), f) }
}
}

Expand All @@ -121,7 +135,7 @@ impl Handle {
}

pub fn erased_value(&self) -> *const () {
unsafe { addr_of!((*self.0.as_ptr()).value) }
unsafe { addr_of!((*self.0.as_ptr()).value.0) }
}

pub fn vtable(&self) -> &'static ObjectVTable {
Expand Down
9 changes: 8 additions & 1 deletion crates/dash_vm/src/gc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ unsafe fn drop_erased_gc_node(s: *mut GcNode<()>) {

macro_rules! register_gc {
($ty:ty, $gc:expr, $val:expr) => {{
assert!(
std::mem::align_of::<$ty>() <= 8,
"cannot register object of type {} because its alignment is {}",
std::any::type_name::<$ty>(),
std::mem::align_of::<$ty>()
);

let value: $ty = $val;
#[allow(unused_unsafe)]
let node = GcNode {
Expand Down Expand Up @@ -187,7 +194,7 @@ macro_rules! register_gc {
flags: Default::default(),
refcount: Default::default(),
next: None,
value,
value: $crate::gc::handle::Align8(value),
};
let ptr: *mut GcNode<()> = Box::into_raw(Box::new(node)).cast();

Expand Down
2 changes: 1 addition & 1 deletion crates/dash_vm/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ impl ExternalValue {
assert_eq!(this.inner.as_any().type_id(), TypeId::of::<Value>());
// SAFETY: casting to *mut GcNode<Value>, then dereferencing + writing
// to said pointer is safe, because it is asserted on the previous line that the type is correct
(*this.inner.as_ptr::<Value>()).value = value;
(*this.inner.as_ptr::<Value>()).value.0 = value;
}
}

Expand Down

0 comments on commit e352f58

Please sign in to comment.