Skip to content

Commit

Permalink
Replace uses of Cell with UnsafeCell
Browse files Browse the repository at this point in the history
The OCaml runtime may touch that memory, and Rust is not aware of it.
  • Loading branch information
tizoc committed Dec 24, 2020
1 parent de5e6d6 commit e795c4c
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::*;
macro_rules! ocaml_frame {
($cr:ident, ($($rootvar:ident),+ $(,)?), $body:block) => {{
let mut frame = $cr.open_frame();
let local_roots = $crate::repeat_slice!(::core::cell::Cell::new($crate::internal::UNIT), $($rootvar)+);
let local_roots = $crate::repeat_slice!(::core::cell::UnsafeCell::new($crate::internal::UNIT), $($rootvar)+);
let gc = frame.initialize(&local_roots);
$(
let $rootvar = unsafe { &mut $crate::internal::OCamlRawRoot::reserve(gc) };
Expand Down
28 changes: 14 additions & 14 deletions src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
runtime::OCamlRuntime,
value::OCaml,
};
use core::{cell::Cell, marker::PhantomData, ptr};
use core::{cell::UnsafeCell, marker::PhantomData, ptr};
pub use ocaml_sys::{
caml_alloc, local_roots as ocaml_sys_local_roots, set_local_roots as ocaml_sys_set_local_roots,
store_field,
Expand Down Expand Up @@ -61,8 +61,8 @@ pub struct GCFrame<'gc> {

impl<'gc> GCFrame<'gc> {
#[doc(hidden)]
pub fn initialize(&mut self, frame_local_roots: &[Cell<RawOCaml>]) -> &mut Self {
self.block.local_roots = frame_local_roots[0].as_ptr();
pub fn initialize(&mut self, frame_local_roots: &[UnsafeCell<RawOCaml>]) -> &mut Self {
self.block.local_roots = frame_local_roots[0].get();
self.block.ntables = 1;
unsafe {
self.block.next = local_roots();
Expand All @@ -85,23 +85,23 @@ impl<'gc> Drop for GCFrame<'gc> {
}

pub struct OCamlRawRoot<'a> {
cell: &'a Cell<RawOCaml>,
cell: &'a UnsafeCell<RawOCaml>,
}

impl<'a> OCamlRawRoot<'a> {
#[doc(hidden)]
pub unsafe fn reserve<'gc>(_gc: &GCFrame<'gc>) -> OCamlRawRoot<'gc> {
assert_eq!(&_gc.block as *const _, local_roots());
let block = &mut *local_roots();
let locals: *const Cell<RawOCaml> = &*(block.local_roots as *const Cell<RawOCaml>);
let locals: *const UnsafeCell<RawOCaml> = &*(block.local_roots as *const UnsafeCell<RawOCaml>);
let cell = &*locals.offset(block.nitems);
block.nitems += 1;
OCamlRawRoot { cell }
}

/// Roots an [`OCaml`] value.
pub fn keep<'tmp, T>(&'tmp mut self, val: OCaml<T>) -> OCamlRoot<'tmp, T> {
self.cell.set(unsafe { val.raw() });
unsafe { *self.cell.get() = val.raw() };
OCamlRoot {
_marker: PhantomData,
cell: self.cell,
Expand All @@ -115,7 +115,7 @@ impl<'a> OCamlRawRoot<'a> {
/// This method is unsafe because there is no way to validate that the [`RawOCaml`] value
/// is of the correct type.
pub unsafe fn keep_raw<T>(&mut self, val: RawOCaml) -> OCamlRoot<T> {
self.cell.set(val);
*self.cell.get() = val;
OCamlRoot {
_marker: PhantomData,
cell: self.cell,
Expand All @@ -128,8 +128,8 @@ impl<'a> OCamlRawRoot<'a> {
/// Roots can be used to recover a fresh reference to an [`OCaml`]`<T>` value what would
/// otherwise become stale after a call to the OCaml runtime.
pub struct OCamlRoot<'a, T> {
pub(crate) cell: &'a Cell<RawOCaml>,
_marker: PhantomData<Cell<T>>,
pub(crate) cell: &'a UnsafeCell<RawOCaml>,
_marker: PhantomData<T>,
}

impl<'a, T> OCamlRoot<'a, T> {
Expand All @@ -143,7 +143,7 @@ impl<'a, T> OCamlRoot<'a, T> {

/// Updates the value of this GC tracked reference.
pub fn set(&mut self, x: OCaml<T>) {
self.cell.set(unsafe { x.raw() });
unsafe { *self.cell.get() = x.raw() };
}

/// Borrows the raw value contained in this root.
Expand All @@ -152,16 +152,16 @@ impl<'a, T> OCamlRoot<'a, T> {
///
/// This method is unsafe, because the RawOCaml value obtained will not be tracked.
pub unsafe fn get_raw(&self) -> RawOCaml {
self.cell.get()
*self.cell.get()
}
}

struct ConstantRoot(Cell<RawOCaml>);
struct ConstantRoot(UnsafeCell<RawOCaml>);

unsafe impl Sync for ConstantRoot {}

static ROOT_UNIT: ConstantRoot = ConstantRoot(Cell::new(ocaml_sys::UNIT));
static ROOT_NONE: ConstantRoot = ConstantRoot(Cell::new(ocaml_sys::NONE));
static ROOT_UNIT: ConstantRoot = ConstantRoot(UnsafeCell::new(ocaml_sys::UNIT));
static ROOT_NONE: ConstantRoot = ConstantRoot(UnsafeCell::new(ocaml_sys::NONE));

impl OCamlRoot<'static, ()> {
/// Convenience method to obtain a root containing an unit value.
Expand Down
2 changes: 1 addition & 1 deletion src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl OCamlRuntime {
pub fn get<'tmp, T>(&'tmp self, reference: &OCamlRoot<T>) -> OCaml<'tmp, T> {
OCaml {
_marker: PhantomData,
raw: reference.cell.get(),
raw: unsafe { *reference.cell.get() },
}
}
}
Expand Down

0 comments on commit e795c4c

Please sign in to comment.