Skip to content

Commit

Permalink
fix(vm): Deep clone partial applications properly
Browse files Browse the repository at this point in the history
Fixes a segfault caused by accidentally storing non-root generation data in the global table. This data would then not be scanned by non-root gcs and freed prematurely yet it would still be handed out when the global were requested.
  • Loading branch information
Marwes committed Nov 27, 2017
1 parent 5642b97 commit d7877ca
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 14 deletions.
46 changes: 45 additions & 1 deletion tests/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use gluon::base::pos::BytePos;
use gluon::base::types::Type;
use gluon::base::source;
use gluon::vm::api::{FunctionRef, Hole, OpaqueValue, ValueRef};
use gluon::vm::thread::{Thread, ThreadInternal};
use gluon::vm::thread::{RootedThread, Thread, ThreadInternal};
use gluon::vm::internal::Value;
use gluon::vm::internal::Value::Int;
use gluon::vm::stack::{StackFrame, State};
Expand Down Expand Up @@ -791,3 +791,47 @@ fn dont_use_the_implicit_prelude_span_in_the_top_expr() {
fn value_size() {
assert!(::std::mem::size_of::<Value>() <= 16);
}

#[test]
fn deep_clone_partial_application() {
use gluon::base::symbol::Symbol;
use gluon::base::metadata::Metadata;

let _ = ::env_logger::init();
let vm = RootedThread::new();

assert_eq!(vm.context().gc.allocated_memory(), 0);

let child = vm.new_thread().unwrap();

assert_eq!(child.context().gc.allocated_memory(), 0);

let result = Compiler::new()
.implicit_prelude(false)
.run_expr::<OpaqueValue<&Thread, Hole>>(
&child,
"test",
r#"
let f x y = y
f 1
"#,
);
assert!(result.is_ok(), "{}", result.err().unwrap());

let global_memory_without_closures = vm.global_env().gc.lock().unwrap().allocated_memory();
let memory_for_closures = child.context().gc.allocated_memory();

vm.set_global(
Symbol::from("test"),
Type::hole(),
Metadata::default(),
unsafe { result.unwrap().0.get_value() },
).unwrap();

let global_memory_with_closures = vm.global_env().gc.lock().unwrap().allocated_memory();

assert_eq!(
global_memory_without_closures + memory_for_closures,
global_memory_with_closures
);
}
6 changes: 5 additions & 1 deletion vm/src/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ impl<T: ?Sized> GcPtr<T> {
unsafe { &(*self.header().type_info).fields }
}

pub fn fields(&self) -> &Arc<Vec<InternedStr>> {
pub fn field_names(&self) -> &Arc<Vec<InternedStr>> {
unsafe { &(*self.header().type_info).fields_key }
}

Expand Down Expand Up @@ -603,6 +603,10 @@ impl Gc {
}
}

pub fn allocated_memory(&self) -> usize {
self.allocated_memory
}

pub fn set_memory_limit(&mut self, memory_limit: usize) {
self.memory_limit = memory_limit;
}
Expand Down
2 changes: 1 addition & 1 deletion vm/src/interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl Interner {

impl fmt::Debug for InternedStr {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "InternedStr({:?})", self.0)
write!(f, "InternedStr({:p}, {:?})", &*self.0, self.0)
}
}
impl fmt::Display for InternedStr {
Expand Down
2 changes: 1 addition & 1 deletion vm/src/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ pub mod gc {
S: Serializer,
{
let tag = if self.is_record() {
let fields = unsafe { GcPtr::from_raw(self).fields().clone() };
let fields = unsafe { GcPtr::from_raw(self).field_names().clone() };
DataTag::Record(fields)
} else {
DataTag::Data(self.tag())
Expand Down
7 changes: 5 additions & 2 deletions vm/src/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,13 @@ impl Traverseable for RootedThread {
impl RootedThread {
/// Creates a new virtual machine with an empty global environment
pub fn new() -> RootedThread {
let mut global_state = GlobalVmState::new();
let thread = Thread {
global_state: Arc::new(GlobalVmState::new()),
parent: None,
context: Mutex::new(Context::new(Gc::new(Generation::default(), usize::MAX))),
context: Mutex::new(Context::new(
global_state.gc.get_mut().unwrap().new_child_gc(),
)),
global_state: Arc::new(global_state),
roots: RwLock::new(Vec::new()),
rooted_values: RwLock::new(Vec::new()),
child_threads: RwLock::new(Vec::new()),
Expand Down
32 changes: 24 additions & 8 deletions vm/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1196,12 +1196,19 @@ impl<'t> Cloner<'t> {
)
}
}
fn deep_clone_data(&mut self, data: GcPtr<DataStruct>) -> Result<GcPtr<DataStruct>> {
let result = self.deep_clone_ptr(data, |gc, data| {
let ptr = gc.alloc(Def {
tag: data.tag,
elems: &data.fields,
})?;
fn deep_clone_data(&mut self, data_ptr: GcPtr<DataStruct>) -> Result<GcPtr<DataStruct>> {
let result = self.deep_clone_ptr(data_ptr, |gc, data| {
let ptr = if data.is_record() {
gc.alloc(RecordDef {
fields: data_ptr.field_names(),
elems: &data.fields,
})?
} else {
gc.alloc(Def {
tag: data.tag,
elems: &data.fields,
})?
};
Ok((Value::Data(ptr), ptr))
})?;
match result {
Expand All @@ -1210,7 +1217,7 @@ impl<'t> Cloner<'t> {
Err(mut new_data) => {
{
let new_fields = unsafe { &mut new_data.as_mut().fields };
for (new, old) in new_fields.iter_mut().zip(&data.fields) {
for (new, old) in new_fields.iter_mut().zip(&data_ptr.fields) {
*new = self.deep_clone(*old)?;
}
}
Expand Down Expand Up @@ -1267,6 +1274,8 @@ impl<'t> Cloner<'t> {

fn deep_clone_closure(&mut self, data: GcPtr<ClosureData>) -> Result<GcPtr<ClosureData>> {
let result = self.deep_clone_ptr(data, |gc, data| {
debug_assert!(data.function.generation().is_root());

let ptr = gc.alloc(ClosureDataDef(data.function, &data.upvars))?;
Ok((Closure(ptr), ptr))
})?;
Expand All @@ -1288,8 +1297,15 @@ impl<'t> Cloner<'t> {
&mut self,
data: GcPtr<PartialApplicationData>,
) -> Result<GcPtr<PartialApplicationData>> {
let function = match data.function {
Callable::Closure(closure) => Callable::Closure(self.deep_clone_closure(closure)?),
Callable::Extern(ext) => {
Callable::Extern(self.gc.alloc(Move(ExternFunction::clone(&ext)))?)
}
};

let result = self.deep_clone_ptr(data, |gc, data| {
let ptr = gc.alloc(PartialApplicationDataDef(data.function, &data.args))?;
let ptr = gc.alloc(PartialApplicationDataDef(function, &data.args))?;
Ok((PartialApplication(ptr), ptr))
})?;
match result {
Expand Down

0 comments on commit d7877ca

Please sign in to comment.