fix copy_term/2 variable copying bug in lists (#923, #2127)

mthom · Nov 21, 2023 · 3841b29 · 3841b29
1 parent a3e83d5
commit 3841b29
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 9 deletions.
diff --git a/src/machine/copier.rs b/src/machine/copier.rs
@@ -18,6 +18,7 @@ pub trait CopierTarget: IndexMut<usize, Output = HeapCellValue> {
     fn store(&self, value: HeapCellValue) -> HeapCellValue;
     fn deref(&self, value: HeapCellValue) -> HeapCellValue;
     fn push(&mut self, value: HeapCellValue);
+    fn push_attr_var_queue(&mut self, attr_var_loc: usize);
     fn stack(&mut self) -> &mut Stack;
     fn threshold(&self) -> usize;
 }
@@ -73,7 +74,6 @@ impl<T: CopierTarget> CopyTermState<T> {
                     if h >= self.old_h {
                         *self.value_at_scan() = list_loc_as_cell!(h);
                         self.scan += 1;
-
                         return;
                     }
                 }
@@ -96,14 +96,19 @@ impl<T: CopierTarget> CopyTermState<T> {
             .store(self.target.deref(heap_loc_as_cell!(addr + 1)));
 
         if !cdr.is_var() {
+            // mark addr + 1 as a list back edge in the cdr of the list
             self.trail_list_cell(addr + 1, threshold);
+            self.target[addr + 1].set_mark_bit(true);
+            self.target[addr + 1].set_forwarding_bit(true);
         } else {
             let car = self
                 .target
                 .store(self.target.deref(heap_loc_as_cell!(addr)));
 
             if !car.is_var() {
+                // mark addr as a list back edge in the car of the list
                 self.trail_list_cell(addr, threshold);
+                self.target[addr].set_mark_bit(true);
             }
         }
 
@@ -178,6 +183,7 @@ impl<T: CopierTarget> CopyTermState<T> {
 
             for (threshold, list_loc) in iter {
                 self.target[threshold] = list_loc_as_cell!(self.target.threshold());
+                self.target.push_attr_var_queue(threshold - 1);
                 self.copy_attr_var_list(list_loc);
             }
         }
@@ -263,6 +269,7 @@ impl<T: CopierTarget> CopyTermState<T> {
     }
 
     fn copy_var(&mut self, addr: HeapCellValue) {
+        let index = addr.get_value() as usize;
         let rd = self.target.deref(addr);
         let ra = self.target.store(rd);
 
@@ -271,7 +278,20 @@ impl<T: CopierTarget> CopyTermState<T> {
                 if h >= self.old_h {
                     *self.value_at_scan() = ra;
                     self.scan += 1;
+                    return;
+                }
+            }
+            (HeapCellValueTag::Lis, h) => {
+                if h >= self.old_h && self.target[index].get_mark_bit() {
+                    *self.value_at_scan() = heap_loc_as_cell!(
+                        if ra.get_forwarding_bit() {
+                            h + 1
+                        } else {
+                            h
+                        }
+                    );
 
+                    self.scan += 1;
                     return;
                 }
             }
@@ -356,12 +376,16 @@ impl<T: CopierTarget> CopyTermState<T> {
         }
     }
 
-    fn unwind_trail(&mut self) {
-        for (r, value) in self.trail.drain(0..) {
+    fn unwind_trail(mut self) {
+        for (r, value) in self.trail {
             let index = r.get_value() as usize;
 
             match r.get_tag() {
-                RefTag::AttrVar | RefTag::HeapCell => self.target[index] = value,
+                RefTag::AttrVar | RefTag::HeapCell => {
+                    self.target[index] = value;
+                    self.target[index].set_mark_bit(false);
+                    self.target[index].set_forwarding_bit(false);
+                }
                 RefTag::StackCell => self.target.stack()[index] = value,
             }
         }

diff --git a/src/machine/machine_state.rs b/src/machine/machine_state.rs
@@ -289,6 +289,11 @@ impl<'a> CopierTarget for CopyTerm<'a> {
         self.state.heap.push(hcv);
     }
 
+    #[inline(always)]
+    fn push_attr_var_queue(&mut self, attr_var_loc: usize) {
+        self.state.attr_var_init.attr_var_queue.push(attr_var_loc);
+    }
+
     #[inline(always)]
     fn store(&self, value: HeapCellValue) -> HeapCellValue {
         self.state.store(value)
@@ -307,17 +312,24 @@ impl<'a> CopierTarget for CopyTerm<'a> {
 
 #[derive(Debug)]
 pub(super) struct CopyBallTerm<'a> {
+    attr_var_queue: &'a mut Vec<usize>,
     stack: &'a mut Stack,
     heap: &'a mut Heap,
     heap_boundary: usize,
     stub: &'a mut Heap,
 }
 
 impl<'a> CopyBallTerm<'a> {
-    pub(super) fn new(stack: &'a mut Stack, heap: &'a mut Heap, stub: &'a mut Heap) -> Self {
+    pub(super) fn new(
+        attr_var_queue: &'a mut Vec<usize>,
+        stack: &'a mut Stack,
+        heap: &'a mut Heap,
+        stub: &'a mut Heap,
+    ) -> Self {
         let hb = heap.len();
 
         CopyBallTerm {
+            attr_var_queue,
             stack,
             heap,
             heap_boundary: hb,
@@ -359,6 +371,11 @@ impl<'a> CopierTarget for CopyBallTerm<'a> {
         self.stub.push(value);
     }
 
+    #[inline(always)]
+    fn push_attr_var_queue(&mut self, attr_var_loc: usize) {
+        self.attr_var_queue.push(attr_var_loc);
+    }
+
     fn store(&self, value: HeapCellValue) -> HeapCellValue {
         read_heap_cell!(value,
             (HeapCellValueTag::Var | HeapCellValueTag::AttrVar, h) => {

diff --git a/src/machine/machine_state_impl.rs b/src/machine/machine_state_impl.rs
@@ -334,7 +334,12 @@ impl MachineState {
         self.ball.boundary = self.heap.len();
 
         copy_term(
-            CopyBallTerm::new(&mut self.stack, &mut self.heap, &mut self.ball.stub),
+            CopyBallTerm::new(
+                &mut self.attr_var_init.attr_var_queue,
+                &mut self.stack,
+                &mut self.heap,
+                &mut self.ball.stub,
+            ),
             addr,
             AttrVarPolicy::DeepCopy,
         );

diff --git a/src/machine/mock_wam.rs b/src/machine/mock_wam.rs
@@ -159,6 +159,14 @@ impl<'a> CopierTarget for TermCopyingMockWAM<'a> {
         self.wam.machine_st.heap.push(val);
     }
 
+    fn push_attr_var_queue(&mut self, attr_var_loc: usize) {
+        self.wam
+            .machine_st
+            .attr_var_init
+            .attr_var_queue
+            .push(attr_var_loc);
+    }
+
     fn stack(&mut self) -> &mut Stack {
         &mut self.wam.machine_st.stack
     }

diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs
@@ -827,8 +827,12 @@ impl MachineState {
     ) -> usize {
         let threshold = self.lifted_heap.len() - lh_offset;
 
-        let mut copy_ball_term =
-            CopyBallTerm::new(&mut self.stack, &mut self.heap, &mut self.lifted_heap);
+        let mut copy_ball_term = CopyBallTerm::new(
+            &mut self.attr_var_init.attr_var_queue,
+            &mut self.stack,
+            &mut self.heap,
+            &mut self.lifted_heap,
+        );
 
         copy_ball_term.push(list_loc_as_cell!(threshold + 1));
         copy_ball_term.push(heap_loc_as_cell!(threshold + 3));
@@ -6789,6 +6793,7 @@ impl Machine {
 
         copy_term(
             CopyBallTerm::new(
+                &mut self.machine_st.attr_var_init.attr_var_queue,
                 &mut self.machine_st.stack,
                 &mut self.machine_st.heap,
                 &mut ball.stub,

diff --git a/src/toplevel.pl b/src/toplevel.pl
@@ -452,4 +452,4 @@
     % is expected to be printed instead.
     ;  print_exception(E)
     ).
-    
+
Original file line number	Diff line number	Diff line change
Expand Up		@@ -452,4 +452,4 @@
		% is expected to be printed instead.
		; print_exception(E)
		).