Merge pull request #3 from csgordon/gcfixes

Adopt a couple fixes from @stephen-hansen and add tests for the bugs he found

examples/gctest4.ir

@@ -0,0 +1,75 @@
+data:
+
+code:
+triggerGC(recv):
+  %cnt = 10
+  jump loopheader
+
+loopheader:
+  if %cnt then doalloc else finish
+
+doalloc:
+  %cnt = %cnt - 1
+  %blah = alloc(10)
+  jump loopheader
+
+finish:
+  ret 5
+
+main:
+  # Allocate for a diamond structure
+  # We'll set up for:
+  #   a
+  #  / \
+  # b   c
+  #  \ /
+  #   d
+  %a = alloc(3)
+  setelt(%a, 0, 0)
+  %b = alloc(2)
+  setelt(%b, 0, 0)
+  %c = alloc(2)
+  setelt(%c, 0, 0)
+  %d = alloc(2)
+  setelt(%d, 0, 0)
+
+  # Create the diamond shape
+  setelt(%a, 1, %b)
+  setelt(%a, 2, %c)
+  setelt(%b, 1, %d)
+  setelt(%c, 1, %d)
+
+  # get addresses for the slot maps for the GC
+  %amap = %a - 8
+  %bmap = %b - 8
+  %cmap = %c - 8
+  %dmap = %d - 8
+
+  # Init a's slot map to b110 (6), b and c's to b010 (2), and d's to 2 (see below)
+  store(%amap, 6)
+  store(%bmap, 2)
+  store(%cmap, 2)
+  # Unlike previous gc tests, mark d's empty field as a pointer to check that the GC doesn't blindly dereference null
+  # We'll overwrite it later before another GC (heap is 100 slots) so rest of test is unchanged
+  store(%dmap, 2)
+  setelt(%d, 1, 0)
+
+  # Now we allocate enough to trigger a GC
+  %lalala = call(triggerGC, 0)
+  # At this point we've triggered a GC, which should have
+  # preserved the diamond shape.
+  # To check this we'll directly update d's value to 4096...
+  setelt(%d, 1, 4096)
+
+  # ... then check that following either path from a observes the same value
+  %b2 = getelt(%a, 1)
+  %dViab = getelt(%b2, 1)
+  %c2 = getelt(%a, 2)
+  %dViac = getelt(%c2, 1)
+
+  # ... where by "check" we mean print for visual inspection
+  %dvalViab = getelt(%dViab, 1)
+  print(%dvalViab)
+  %dvalViac = getelt(%dViac, 1)
+  print(%dvalViac)
+  ret %dvalViac

examples/gctest5.ir

@@ -0,0 +1,79 @@
+data:
+
+code:
+triggerGC(recv):
+  %cnt = 10
+  jump loopheader
+
+loopheader:
+  if %cnt then doalloc else finish
+
+doalloc:
+  %cnt = %cnt - 1
+  %blah = alloc(10)
+  jump loopheader
+
+finish:
+  ret 5
+
+main:
+  # Allocate for a diamond structure
+  # We'll set up for:
+  #   a
+  #  / \
+  # b   c
+  #  \ /
+  #   d
+  %a = alloc(3)
+  setelt(%a, 0, 0)
+  %b = alloc(2)
+  setelt(%b, 0, 0)
+  %c = alloc(2)
+  setelt(%c, 0, 0)
+  %d = alloc(2)
+  setelt(%d, 0, 0)
+
+  # Create the diamond shape
+  setelt(%a, 1, %b)
+  setelt(%a, 2, %c)
+  setelt(%b, 1, %d)
+  setelt(%c, 1, %d)
+  setelt(%d, 1, 47)
+
+  # get addresses for the slot maps for the GC
+  %amap = %a - 8
+  %bmap = %b - 8
+  %cmap = %c - 8
+  %dmap = %d - 8
+
+  # Init a's slot map to b110 (6), b and c's to b010 (2), and d's to 0
+  store(%amap, 6)
+  store(%bmap, 2)
+  store(%cmap, 2)
+  store(%dmap, 0)
+
+  # Now we allocate enough to trigger a GC
+  %lalala = call(triggerGC, 0)
+
+
+  # Now trigger *another* GC, to make sure we handle locals correctly on a second GC
+  %lalala = call(triggerGC, 0)
+
+  # At this point we've triggered a GC, which should have
+  # preserved the diamond shape.
+  # To check this we'll directly update d's value to 4096...
+  setelt(%d, 1, 4096)
+
+  # ... then check that following either path from a observes the same value
+  %b2 = getelt(%a, 1)
+  %dViab = getelt(%b2, 1)
+  %c2 = getelt(%a, 2)
+  %dViac = getelt(%c2, 1)
+
+  # ... where by "check" we mean print for visual inspection
+  %dvalViab = getelt(%dViab, 1)
+  print(%dvalViab)
+  %dvalViac = getelt(%dViac, 1)
+  print(%dvalViac)
+  ret %dvalViac
+

src/ir441/exec.rs

@@ -141,6 +141,7 @@ impl <'a> Memory<'a> {
                                 println!("Moved {} from {} to {}", x, val, newloc);
                             }
                             *v = VirtualVal::Data {val : newloc};
+                            self.allocations.insert(newloc);
                         } else {
                             if self.slot_cap.is_logging_gc() {
                                 println!("Skipping {}={} b/c it does not appear to be a valid allocation", x, val);
@@ -217,15 +218,19 @@ impl <'a> Memory<'a> {
                         let orig = self.mem_lookup(addr + i*8)?;
                         if slotmap & 0x1 == 1 {
                             // trace
-                            let moved_to = match orig {
+                            let to_trace = match orig {
                                             VirtualVal::GCTombstone => Err(RuntimeError::CorruptGCMetadata { val: orig }),
                                             VirtualVal::CodePtr{..} => Err(RuntimeError::BadGCField),
-                                            VirtualVal::Data{val:to_trace} => self.trace(to_trace)
+                                            VirtualVal::Data{val:trace_val} => Ok(trace_val)
                                           }?;
-                            self.mem_store(new_obj_base + i*8, VirtualVal::Data { val: moved_to })?;
-                            if self.slot_cap.is_logging_gc() {
-                                println!("Rewrote slot {} from {} to {}", i, orig, moved_to);
+                            if to_trace != 0 {
+                                let moved_to = self.trace(to_trace)?;
+                                self.mem_store(new_obj_base + i*8, VirtualVal::Data { val: moved_to })?;
+                                if self.slot_cap.is_logging_gc() {
+                                    println!("Rewrote slot {} from {} to {}", i, orig, moved_to);
+                                }
                             }
+                            // No else for the 0 case is necessary, as slots are initialized to 0
                         } else {
                             // blind copy
                             self.mem_store(new_obj_base + i*8, orig)?;
@@ -689,4 +694,4 @@ pub fn run_prog<'a>(prog: &'a IRProgram, tracing: bool, mut cycles: &mut ExecSta
         }
     };
     fresult
-}
+}

src/main.rs

@@ -194,4 +194,22 @@ mod systests {
         assert_eq!(result,Ok(VirtualVal::Data { val: 4096 }));
         Ok(())
     }
+    #[test]
+    fn check_gctest4() -> Result<(),Box<dyn std::error::Error>>{
+        let bytes = load_program("examples/gctest4.ir")?;
+        let prog = parse(&bytes)?;
+        let mut cycles = ExecStats { allocs: 0, calls: 0, fast_alu_ops: 0, slow_alu_ops: 0, phis: 0, conditional_branches: 0, unconditional_branches: 0, mem_reads: 0, mem_writes: 0, prints: 0, rets: 0 };
+        let result = run_prog(&prog, false, &mut cycles, ExecMode::GC { limit: 100 });
+        assert_eq!(result,Ok(VirtualVal::Data { val: 4096 }));
+        Ok(())
+    }
+    #[test]
+    fn check_gctest5() -> Result<(),Box<dyn std::error::Error>>{
+        let bytes = load_program("examples/gctest5.ir")?;
+        let prog = parse(&bytes)?;
+        let mut cycles = ExecStats { allocs: 0, calls: 0, fast_alu_ops: 0, slow_alu_ops: 0, phis: 0, conditional_branches: 0, unconditional_branches: 0, mem_reads: 0, mem_writes: 0, prints: 0, rets: 0 };
+        let result = run_prog(&prog, false, &mut cycles, ExecMode::GC { limit: 100 });
+        assert_eq!(result,Ok(VirtualVal::Data { val: 4096 }));
+        Ok(())
+    }
 }