From 3841b29db865be45078a5e712f3512476c834262 Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 21 Nov 2023 12:50:10 -0700 Subject: [PATCH] fix copy_term/2 variable copying bug in lists (#923, #2127) --- src/machine/copier.rs | 32 +++++++++++++++++++++++++++---- src/machine/machine_state.rs | 19 +++++++++++++++++- src/machine/machine_state_impl.rs | 7 ++++++- src/machine/mock_wam.rs | 8 ++++++++ src/machine/system_calls.rs | 9 +++++++-- src/toplevel.pl | 2 +- 6 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/machine/copier.rs b/src/machine/copier.rs index be433ba14..c02854ffd 100644 --- a/src/machine/copier.rs +++ b/src/machine/copier.rs @@ -18,6 +18,7 @@ pub trait CopierTarget: IndexMut { 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 CopyTermState { if h >= self.old_h { *self.value_at_scan() = list_loc_as_cell!(h); self.scan += 1; - return; } } @@ -96,14 +96,19 @@ impl CopyTermState { .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 CopyTermState { 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 CopyTermState { } 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 CopyTermState { 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 CopyTermState { } } - 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 index 596eefbd3..3763b2c09 100644 --- 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,6 +312,7 @@ impl<'a> CopierTarget for CopyTerm<'a> { #[derive(Debug)] pub(super) struct CopyBallTerm<'a> { + attr_var_queue: &'a mut Vec, stack: &'a mut Stack, heap: &'a mut Heap, heap_boundary: usize, @@ -314,10 +320,16 @@ pub(super) struct CopyBallTerm<'a> { } 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, + 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 index 82a64bf48..edd6b673f 100644 --- 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 index 94eab6ff2..0da42e08d 100644 --- 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 index 0c425b969..e5f77bb47 100644 --- 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 index dc7342c04..a9a423c45 100644 --- a/src/toplevel.pl +++ b/src/toplevel.pl @@ -452,4 +452,4 @@ % is expected to be printed instead. ; print_exception(E) ). - \ No newline at end of file +