[gnugo-devel] semeai crash (Re: Current results)

[Arend Bayer]

Gunnar wrote:

> Arend wrote:
> > I don't get that. Could you post the bug message?
>
> Some trouble with the owl stack, deep down. Below follows the output
> from abortgo() and after that the backtrace when stopped in abortgo().

This is caused by the owl_stack getting moved when realloc() is called.
I should have realized that this case might not have gotten testing --
in fact, with the usual settings I didn't succeed in having this happen
here, despite starting a couple of memory killing netscapes...

Could you possibly test the patch below? (This is not yet for CVS.)
The relevant change is in pop_owl().
Note that you should get a message "Stack has moved....", otherwise the
problem didn't even occur.

For CVS, I will also make the initial owl stack size larger if
experimental_semeai is selected.

Arend


Index: engine/owl.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/owl.c,v
retrieving revision 1.94
diff -u -r1.94 owl.c
--- engine/owl.c        24 Jun 2002 14:24:47 -0000      1.94
+++ engine/owl.c        26 Jun 2002 14:57:45 -0000
@@ -859,10 +859,6 @@
        && stackp < MAX_SEMEAI_DEPTH
        && semeai_trymove(mpos, color, moves[k].name, apos, bpos,
                          owl_phase, moves[k].value)) {
-      if (owl_phase && stackp <= SEMEAI_BRANCH_DEPTH + 1)
-       push_owl(&owla, &owlb);
-      else
-       memcpy(saved_goal, owla->goal, sizeof(saved_goal));
       if ((debug & DEBUG_SEMEAI) && verbose)
        dump_stack();
       if (board[bpos] == EMPTY) {
@@ -870,21 +866,29 @@
        this_resulta = ALIVE;
       }
       else {
+       if (owl_phase && stackp <= SEMEAI_BRANCH_DEPTH + 1)
+         push_owl(&owla, &owlb);
+       else
+         memcpy(saved_goal, owla->goal, sizeof(saved_goal));
+
        owl_update_goal(mpos, moves[k].same_dragon, owla, 1);
        owla->lunches_are_current = 0;
        owl_update_boundary_marks(mpos, owla);
+
        do_owl_analyze_semeai(bpos, apos, owlb, owla, komaster,
                              &this_resultb, &this_resulta, NULL, 0, owl_phase);
-      }
-
-      if (this_resultb == DEAD && this_resulta == ALIVE) {
+
        if (owl_phase && stackp <= SEMEAI_BRANCH_DEPTH + 1) {
          pop_owl(&owlb);
          pop_owl(&owla);
        }
        else
          memcpy(owla->goal, saved_goal, sizeof(saved_goal));
-       popgo();
+      }
+
+      popgo();
+
+      if (this_resultb == DEAD && this_resulta == ALIVE) {
        *resulta = ALIVE;
        *resultb = DEAD;
        if (move)
@@ -910,13 +914,6 @@
        best_move = mpos;
        best_move_k = k;
       }
-      if (owl_phase && stackp <= SEMEAI_BRANCH_DEPTH + 1) {
-       pop_owl(&owlb);
-       pop_owl(&owla);
-      }
-      else
-       memcpy(owla->goal, saved_goal, sizeof(saved_goal));
-      popgo();
     }
   }

@@ -4612,30 +4609,42 @@
  * semeai code; use NULL otherwise.
  *
  * If you use push_owl with two arguments, later call
- * pop_owl(owlb); pop_owl(owla);
+ * pop_owl(&owlb); pop_owl(&owla);
  * in this order!
+ *
+ * Note that the owl stack might get moved in this function. This means
+ * that all pointers to the owl stack will get invalid. Only the pointers
+ * owla (and owlb) at the current recursion depth get corrected immediately.
+ * All other pointers will get corrected when pop_owl() is called.
  */
 static void
 push_owl(struct local_owl_data **owla, struct local_owl_data **owlb)
 {
   /* Do we need to enlarge the stack? */
-  if (owl_stack_pointer == owl_stack_size - 1) {
+  if (owl_stack_pointer == owl_stack_size - 1
+      || (owl_stack_pointer == owl_stack_size - 2 && owlb)) {
     int num_a = (*owla)->number_in_stack;
     int num_b = 0;
-    if (owlb)
+    struct local_owl_data *old_stack_loc = owl_stack;
+    gg_assert(*owla == &(owl_stack[num_a]));
+    if (owlb) {
       num_b = (*owlb)->number_in_stack;
-    if (0)
+      gg_assert(*owlb == &(owl_stack[num_b]));
+    }
+    if (0) {
       gprintf("Have to enlarge owl stack! (size %d, owl_stack %d, stackp 
%d)\n",
              owl_stack_size, owl_stack_pointer, stackp);
-    if (owlb)
-      owl_stack_size += 2;
-    else
-      owl_stack_size++;
+      dump_stack();
+    }
+    /* Better reallocate too much, than to have reallocate more often: */
+    owl_stack_size += 2;
     owl_stack = realloc(owl_stack, owl_stack_size * sizeof(*owl_stack));
     gg_assert(owl_stack != NULL);
-    *owla = &owl_stack[num_a];
+    if (1 && (owl_stack != old_stack_loc))
+      gprintf("Stack has moved! New stack size %d.\n", owl_stack_size);
+    *owla = &(owl_stack[num_a]);
     if (owlb)
-      *owlb = &owl_stack[num_b];
+      *owlb = &(owl_stack[num_b]);
   }

   do_push_owl(owla);
@@ -4651,9 +4660,8 @@
 pop_owl(struct local_owl_data **owl)
 {
   int nodes = (*owl)->local_owl_node_counter;
-  gg_assert(*owl == &owl_stack[owl_stack_pointer]);

-  *owl = &owl_stack[(*owl)->restore_from];
+  *owl = &(owl_stack[owl_stack[owl_stack_pointer].restore_from]);

   owl_stack_pointer--;
   (*owl)->local_owl_node_counter = nodes;