restore [o|i]ip zeroing to trust and fix '$get_clause_p'/3 (#2238)

src/lib/builtins.pl

@@ -1239,7 +1239,10 @@ call_retract_helper(Head, Body, P, Module) :-
     ;  ClauseQualifier = Module
     ),
     ClauseQualifier:'$clause'(Head, Body),
-    '$get_clause_p'(Head, P, Module).
+    % ensure '$get_clause_p'/3 is not the last clause so it can
+    % recover the choice point of '$clause' if necessary.
+    '$get_clause_p'(Head, P, Module),
+    true.
 
 call_retract(Head, Body, Name, Arity, Module) :-
     findall(P, builtins:call_retract_helper(Head, Body, P, Module), Ps),

src/machine/mod.rs

@@ -1010,6 +1010,20 @@ impl Machine {
 
             self.machine_st.heap.truncate(target_h);
 
+            // these registers don't need to be reset here and MUST
+            // NOT be (nor in indexed_try! trust_epilogue is an
+            // exception, see next paragraph)! oip could be reset
+            // without any adverse effects but iip is needed by
+            // get_clause_p to find the last executed clause/2 clause.
+
+            // trust_epilogue must reset these for the sake of
+            // subsequent predicates beginning with
+            // switch_to_term. get_clause_p copes by checking
+            // self.machine_st.b > self.machine.e: if true, it is safe
+            // to use self.machine_st.iip; if false, use the choice
+            // point left at the top of the stack by '$clause'
+            // (specifically its biip value).
+
             // self.machine_st.oip = 0;
             // self.machine_st.iip = 0;
         } else {
@@ -1059,13 +1073,8 @@ impl Machine {
         self.machine_st.stack.truncate(b);
         self.machine_st.heap.truncate(target_h);
 
-        // these registers don't need to be reset here and MUST NOT be
+        self.machine_st.oip = 0;
-        // (nor in indexed_try to trust_epilogue)! oip could be reset
+        self.machine_st.iip = 0;
-        // without any adverse effects but iip is needed by
-        // get_clause_p to find the last executed clause/2 clause.
-
-        // self.machine_st.oip = 0;
-        // self.machine_st.iip = 0;
     }
 
     #[inline(always)]

src/machine/preprocessor.rs

@@ -565,21 +565,6 @@ impl Preprocessor {
         }
     }
 
-    /*
-    fn try_term_to_query<'a, LS: LoadState<'a>>(
-        &mut self,
-        loader: &mut Loader<'a, LS>,
-        terms: Vec<Term>,
-        cut_context: CutContext,
-    ) -> Result<TopLevel, CompilationError> {
-        Ok(TopLevel::Query(self.setup_query(
-            loader,
-            terms,
-            cut_context,
-        )?))
-    }
-    */
-
     pub(super) fn try_term_to_tl<'a, LS: LoadState<'a>>(
         &mut self,
         loader: &mut Loader<'a, LS>,
@@ -607,20 +592,4 @@ impl Preprocessor {
             }
         }
     }
-
-    /*
-    fn try_terms_to_tls<'a, I: IntoIterator<Item = Term>, LS: LoadState<'a>>(
-        &mut self,
-        loader: &mut Loader<'a, LS>,
-        terms: I,
-    ) -> Result<VecDeque<TopLevel>, CompilationError> {
-        let mut results = VecDeque::new();
-
-        for term in terms.into_iter() {
-            results.push_back(self.try_term_to_tl(loader, term)?);
-        }
-
-        Ok(results)
-    }
-    */
 }

src/machine/stack.rs

@@ -203,6 +203,12 @@ impl Stack {
         }
     }
 
+    pub(crate) fn top(&self) -> usize {
+        unsafe {
+            (*self.buf.ptr.get()) as usize - self.buf.base as usize
+        }
+    }
+
     pub(crate) fn allocate_or_frame(&mut self, num_cells: usize) -> usize {
         let frame_size = OrFrame::size_of(num_cells);
 

src/machine/system_calls.rs

@@ -1176,14 +1176,12 @@ impl Machine {
             CompilationTarget::Module(target) => target,
         };
 
-        let bp = self
+        let mut bp = self
             .indices
             .get_predicate_code_index(atom!("$clause"), 2, module_name)
             .and_then(|idx| idx.local())
             .unwrap();
 
-        let p = self.machine_st.iip as usize;
-
         macro_rules! extract_ptr {
             ($ptr: expr) => {
                 match $ptr {
@@ -1199,45 +1197,71 @@ impl Machine {
             };
         }
 
-        match &self.code[bp] {
+        loop {
-            Instruction::IndexingCode(ref indexing_code) => {
+            match &self.code[bp] {
-                let indexing_code_ptr = match &indexing_code[0] {
+                Instruction::IndexingCode(ref indexing_code) => {
-                    &IndexingLine::Indexing(IndexingInstruction::SwitchOnTerm(_, _, c, _, s)) => {
+                    let indexing_code_ptr = match &indexing_code[0] {
-                        if key.1 > 0 {
+                        &IndexingLine::Indexing(IndexingInstruction::SwitchOnTerm(_, _, c, _, s)) => {
-                            s
+                            if key.1 > 0 {
-                        } else {
+                                s
-                            c
+                            } else {
+                                c
+                            }
                         }
-                    }
+                        _ => {
-                    _ => {
+                            unreachable!()
-                        unreachable!()
+                        }
-                    }
+                    };
-                };
 
-                let boip = extract_ptr!(indexing_code_ptr);
+                    let boip = extract_ptr!(indexing_code_ptr);
 
-                let boip = match &indexing_code[boip] {
+                    let boip = match &indexing_code[boip] {
-                    IndexingLine::Indexing(IndexingInstruction::SwitchOnStructure(ref hm)) => {
+                        IndexingLine::Indexing(IndexingInstruction::SwitchOnStructure(ref hm)) => {
-                        boip + extract_ptr!(hm.get(&key).cloned().unwrap())
+                            boip + extract_ptr!(hm.get(&key).cloned().unwrap())
-                    }
+                        }
-                    IndexingLine::Indexing(IndexingInstruction::SwitchOnConstant(ref hm)) => {
+                        IndexingLine::Indexing(IndexingInstruction::SwitchOnConstant(ref hm)) => {
-                        boip + extract_ptr!(hm.get(&Literal::Atom(key.0)).cloned().unwrap())
+                            boip + extract_ptr!(hm.get(&Literal::Atom(key.0)).cloned().unwrap())
-                    }
+                        }
-                    _ => boip,
+                        _ => boip,
-                };
+                    };
 
-                match &indexing_code[boip] {
+                    match &indexing_code[boip] {
-                    IndexingLine::IndexedChoice(indexed_choice) => {
+                        IndexingLine::IndexedChoice(indexed_choice) => {
-                        return (
+                            let p = if self.machine_st.b > self.machine_st.e {
-                            skeleton.core.clause_clause_locs[p],
+                                // this means the last
-                            bp + indexed_choice[p].offset(),
+                                // self.machine_st.iip value has yet
-                        );
+                                // to be overwritten by the Trust
+                                // instruction. In this case, return
+                                // it.
+                                self.machine_st.iip as usize
+                            } else {
+                                // otherwise, read the '$clause'
+                                // choicepoint from the top of the
+                                // stack. this is very volatile in
+                                // that it depends on '$clause'
+                                // immediately preceding
+                                // '$get_clause_p', which cannot be
+                                // the last clause of the retract
+                                // helper to delay deallocation of its
+                                // environment frame.
+                                let clause_b = self.machine_st.stack.top();
+                                self.machine_st.stack.index_or_frame(clause_b).prelude.biip as usize
+                            };
+
+                            return (
+                                skeleton.core.clause_clause_locs[p],
+                                bp + indexed_choice[p].offset(),
+                            );
+                        }
+                        _ => unreachable!(),
                     }
-                    _ => unreachable!(),
                 }
-            }
+                &Instruction::RevJmpBy(offset) => {
-            _ => {
+                    bp -= offset;
-                return (skeleton.core.clause_clause_locs[p], bp);
+                }
+                _ => {
+                    return (skeleton.core.clause_clause_locs.back().cloned().unwrap(), bp);
+                }
             }
         }
     }