Re: [gnugo-devel] Arraybound violation in 3.1.16

[Gunnar Farneback]

Dan wrote:
> Apparently this is what was intended (see patch, untested).

Sorry I didn't review this closer earlier.

> Index: engine/reading.c
> ===================================================================
> RCS file: /cvsroot/gnugo/gnugo/engine/reading.c,v
> retrieving revision 1.38
> diff -u -r1.38 reading.c
> --- engine/reading.c  10 Dec 2001 17:22:40 -0000      1.38
> +++ engine/reading.c  12 Dec 2001 17:04:41 -0000
> @@ -895,12 +895,10 @@
>  
>         for (r = -1; r < max_points; r++) {
>           /* -1 is used only when stackp > 0. */
> -       if (stackp > 0) {
> -         if (r == -1)
> -           bb = dd;
> -         else
> -           break;
> -       }
> +       if (r == -1 && stackp > 0)
> +         break;
> +       if (r == -1)
> +         bb = dd;

This is definitely not what Inge had intended. When stackp>0 this
breaks out of the loop immediately, so there would be no point even
entering the loop with stackp>0. The appended patch fixes this problem
and another serious bug, namely that attack_and_defend() must be
called rather than attack() and find_defense() separately in order for
the defense point to be reliable. Finally it fixes up some weird
indentation.

Btw, there was an NNGS crash earlier today. This was caused by the
original arraybound violation because it in turn caused trymove() to
be called at NO_MOVE (i.e. 0).

/Gunnar

Index: engine/reading.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/reading.c,v
retrieving revision 1.39
diff -u -r1.39 reading.c
--- engine/reading.c    13 Dec 2001 17:04:29 -0000      1.39
+++ engine/reading.c    13 Dec 2001 19:55:33 -0000
@@ -889,33 +889,36 @@
   for (k = 0; k < num_adj; k++) {
     int bb;
     int dd;  /* Defense point of weak neighbor. */
-
-    if (attack(adjs[k], NULL) != 0
-       && find_defense(adjs[k], &dd) != 0) {
-
-       for (r = -1; r < max_points; r++) {
-         /* -1 is used only when stackp > 0. */
-         if (r == -1 && stackp > 0)
-           break;
+    int acode;
+    int dcode;
+    attack_and_defend(adjs[k], &acode, NULL, &dcode, &dd);
+    if (acode != 0 && dcode != 0) {
+      for (r = -1; r < max_points; r++) {
+       /* -1 is used only when stackp > 0. */
+       if (stackp > 0) {
          if (r == -1)
            bb = dd;
-         else {
-           if (worm[adjs[k]].defend_codes[r] == 0)
-             break;
-           bb = worm[adjs[k]].defense_points[r];
-         }
-
-         if (trymove(bb, other, "attack_threats-C", pos, EMPTY, NO_MOVE)) {
-           int acode;
-           if (board[pos] == EMPTY)
-             acode = WIN;
-           else
-             acode = attack(pos, NULL);
-           if (acode != 0)
-             movelist_change_point(bb, acode, max_points, moves, codes);
-           popgo();
-         }
+         else
+           break;
+       }
+       else {
+         if (r == -1)
+           continue;
+         if (worm[adjs[k]].defend_codes[r] == 0)
+           break;
+         bb = worm[adjs[k]].defense_points[r];
+       }
+       
+       if (trymove(bb, other, "attack_threats-C", pos, EMPTY, NO_MOVE)) {
+         if (board[pos] == EMPTY)
+           acode = WIN;
+         else
+           acode = attack(pos, NULL);
+         if (acode != 0)
+           movelist_change_point(bb, acode, max_points, moves, codes);
+         popgo();
        }
+      }
     }
   }