fix some gc bugs

Dockerfile

@@ -4,7 +4,7 @@ ENV DEBIAN_FRONTEND=noninteractive
 ENV TZ=Etc/UTC
 
 RUN apt-get update
-RUN apt-get install -y git valgrind
+RUN apt-get install -y git valgrind gcc
 
 ENTRYPOINT git clone https://github.com/ocen-lang/ocen /ocen/ \
     && cd /ocen/ \

compiler/bytecode.oc

@@ -5,6 +5,7 @@ import std::buffer::{ Buffer }
 import std::mem
 
 import @vm::value::{ Value, String, Object }
+import @vm::{ VM }
 
 enum OpCode {
     Null
@@ -100,14 +101,24 @@ def Chunk::push_with_arg_u8(&this, op: OpCode, arg: u8, span: Span) {
     .push_u8(arg, span)
 }
 
-def Chunk::push_with_literal(&this, op: OpCode, value: Value, span: Span) {
+//! Push an instruction with a literal argument. The VM is required here to be able
+//! to temporarily push the literal onto the stack - this is so the garbage collector
+//! can see it if it is invoked during the push. 
+def Chunk::push_with_literal(&this, vm: &VM, op: OpCode, value: Value, span: Span) {
     let it = .literal_map.get_item(value)
     let idx = match it? {
         true => it.value
         false => {
             let idx = .literals.size
             assert idx < 65536, "Too many literals in a chunk"
+
+            assert vm.stack.size + 1 < vm.stack.capacity, "Not enough space in VM stack"
+            vm.stack.push(value)
+
             .literals.push(value)
+
+            vm.stack.pop()
+
             .literal_map[value] = idx
             yield idx
         }

compiler/compiler.oc

@@ -160,7 +160,7 @@ def Compiler::define_variable(&this, sym: &Symbol) {
     }
 
     // Global variable
-    .chunk.push_with_literal(SetGlobal, .make_str(sym.name), sym.span)
+    .chunk.push_with_literal(.vm, SetGlobal, .make_str(sym.name), sym.span)
     .chunk.push_op(Pop, sym.span)
 }
 
@@ -180,7 +180,7 @@ def Compiler::compile_variable(&this, name: SV, span: Span) {
     }
 
     // Global Variable
-    .chunk.push_with_literal(GetGlobal, .make_str(name), span)
+    .chunk.push_with_literal(.vm, GetGlobal, .make_str(name), span)
 }
 
 def Compiler::make_jump(&this, op: OpCode, span: Span): u32 {
@@ -233,19 +233,19 @@ def Compiler::compile_expression(&this, node: &AST) {
     match node.type {
         IntLiteral => {
             let value = Value::Int(node.u.num_literal.text.to_i32())
-            .chunk.push_with_literal(Constant, value, node.span)
+            .chunk.push_with_literal(.vm, Constant, value, node.span)
         }
         FloatLiteral => {
             let value = Value::Float(std::libc::strtod(node.u.num_literal.text, null))
-            .chunk.push_with_literal(Constant, value, node.span)
+            .chunk.push_with_literal(.vm, Constant, value, node.span)
         }
         StringLiteral => {
             let text = node.u.string_literal
-            .chunk.push_with_literal(Constant, .make_str(text), node.span)
+            .chunk.push_with_literal(.vm, Constant, .make_str(text), node.span)
         }
         BoolLiteral => {
             let value = Value::Bool(node.u.bool_literal)
-            .chunk.push_with_literal(Constant, value, node.span)
+            .chunk.push_with_literal(.vm, Constant, value, node.span)
         }
         Null => .chunk.push_op(Null, node.span)
         Identifier => .compile_variable(node.u.ident.name, node.span)
@@ -282,7 +282,7 @@ def Compiler::compile_expression(&this, node: &AST) {
 
                         // Local variable
                         } else {
-                            .chunk.push_with_literal(SetGlobal, .make_str(name), node.span)
+                            .chunk.push_with_literal(.vm, SetGlobal, .make_str(name), node.span)
 
                         }
                     }
@@ -374,12 +374,12 @@ def Compiler::compile_statement(&this, node: &AST) {
             cc.compile_statement(ast_func.body)
 
             // Add a placeholder return statement if none was provided
-            cc.chunk.push_with_literal(Constant, Value::Null(), node.span)
+            cc.chunk.push_with_literal(.vm, Constant, Value::Null(), node.span)
             cc.chunk.push_op(Return, node.span)
             cc.end_scope(ast_func.sym.span)
 
             let func_val = Value::Object(&cc.func.obj)
-            .chunk.push_with_literal(Constant, func_val, node.span)
+            .chunk.push_with_literal(.vm, Constant, func_val, node.span)
 
             .chunk.push_with_arg_u16(CloseFunction, cc.upvars.size as u16, node.span)
             for up in cc.upvars.iter() {
@@ -418,7 +418,7 @@ def Compiler::compile_statement(&this, node: &AST) {
             if loop.cond? {
                 .compile_expression(loop.cond)
             } else {
-                .chunk.push_with_literal(Constant, Value::True(), node.span)
+                .chunk.push_with_literal(.vm, Constant, Value::True(), node.span)
             }
 
             let false_jump = .make_jump(JumpIfFalse, node.span)

compiler/vm/gc.oc

@@ -21,39 +21,43 @@ def set_compiler(compiler: &Compiler) => state::compiler = compiler
 def set_vm(vm: &VM) => state::vm = vm
 
 def gc_mem(old: untyped_ptr, new_size: u32): untyped_ptr {
-    let old_base = if old? then old - 4 else null
-    let old_size = if old? then *(old_base as &u32) else 0
-
-    if old_size < new_size and not state::paused {
+    // This function handles _all_ the memory management for the Compiler + VM.
+    //   (Note that parsing+lexing is handled by a `bump` allocator - all temp)
+    //
+    // We fall back to C's functions to actually allocate and free memory, but for our
+    // bookkeeping purposes, we want to know the sizes of the pointers. C's functions
+    // don't give us that, so we allocate 8 extra bytes to store the size of the pointer
+    // before the actual pointer.
+    //
+    // Memory layout:
+    //   [ xxxxxxxx | ........ ........ ........ ........ ........  ]
+    //     ^          ^
+    // 8 byte size    Pointer to actual memory return from this function
+
+    let old_base = if old? then old - 8 else null
+    let old_size = if old? then *(old_base as &u64) else 0u64
+
+    if old_size < new_size as u64 and not state::paused {
         if bytes_allocated > next_gc {
             collect_garbage(state::vm, state::compiler)
             next_gc = bytes_allocated * GC_HEAP_GROW_FACTOR
         }
     }
 
-    // alloc
-    if old_size == 0 {
-        let res = mem::impl::calloc(1, new_size + 4) // 4 bytes for the size
-        bytes_allocated += new_size as u64
-        if GC_DEBUG then println(f"[GC] Allocated {new_size} bytes, total: {bytes_allocated} bytes allocated")
-        *(res as &u32) = new_size
-        return res + 4
-    }
-
     // free
     if new_size == 0 {
-        mem::impl::free(old - 4)
+        mem::impl::free(old_base)
         if GC_DEBUG then println(f"[GC] Deleted {old_size} bytes from {bytes_allocated} bytes allocated")
         bytes_allocated -= old_size as u64
         return null
     }
 
-    // realloc
-    let new_base = mem::impl::realloc(old_base, new_size + 4)
+    // alloc / realloc
+    let new_base = mem::impl::realloc(old_base, new_size + 8)
     bytes_allocated += new_size as u64 - old_size as u64
     if GC_DEBUG then println(f"[GC] Reallocated {old_size} -> {new_size} bytes, total: {bytes_allocated} bytes allocated")
-    *(new_base as &u32) = new_size
-    return new_base + 4
+    *(new_base as &u64) = new_size as u64
+    return new_base + 8
 }
 
 def gc_alloc(_: mem::State, size: u32): untyped_ptr => gc_mem(null, size)
@@ -283,4 +287,6 @@ def print_stats(vm: &VM) {
         }
         println(`[GC]     Remaining objects: {num}`)
     }
+    println(`[GC] Remaining bytes allocated: {bytes_allocated}`)
+    println("[GC] === END ===")
 }

compiler/vm/mod.oc

@@ -70,7 +70,7 @@ def VM::make(): VM {
         ip: null,
         stack_base: 0,
         cur_inst_ip: null,
-        stack: Vector<Value>::new(),
+        stack: Vector<Value>::new(capacity: 256),
         globals: Map<&String, Value>::new(),
         strings: Map<ValueCompareString, Value>::new(),
         open_upvalues: null,
@@ -92,6 +92,7 @@ def VM::free(&this) {
     .globals.free()
     .strings.free()
     .grays.free()
+    .frames.free()
     log(Info, "VM freed")
 }
 
@@ -117,12 +118,18 @@ def VM::copy_string(&this, data: str, len: u32): Value {
     let item = .strings.get_item(candidate)
     if item? return item.value
 
-    let string = gc::allocate_object<String>(ObjectType::String, this)
     let new_data = mem::alloc<char>(len+1) // +1 for null terminator, so we can print it
     std::libc::memcpy(new_data, data, len)
+
+    let string = gc::allocate_object<String>(ObjectType::String, this)
     string.init(new_data, len, candidate.shash)
     let value = Value::String(string)
+
+    // Push on the stack so GC can find it if we collect
+    .stack.push(value)
     .strings.insert(candidate, value)
+    .stack.pop()
+
     return value
 }
 
@@ -140,7 +147,15 @@ def VM::add(&this, a: Value, b: Value): Value => if {
     a.is_string() and b.is_string() => {
         let a_str = a.as_string()
         let b_str = b.as_string()
-        return .take_string(`{a_str.data}{b_str.data}`, a_str.len + b_str.len)
+        // FIXME: Don't pop and push these back on, just peek the values earlier
+        //        We need to do this so that the GC can find the strings when we
+        //        allocate the new one (which can trigger a GC collection)
+        .stack.push(a)
+        .stack.push(b)
+        let res = .take_string(`{a_str.data}{b_str.data}`, a_str.len + b_str.len)
+        .stack.pop() // b
+        .stack.pop() // a
+        yield res
     }
     else => .error(f"Cant add {a.type_str()} and {b.type_str()}")
 }