Re: [gnugo-devel] Assertion failure

[Gunnar Farneback]

I wrote:
> The appended game caused an assertion failure on NNGS. The problem is
> that too many move reasons are generated, which we are aware of may
> happen. The relevant code can be found around line 413 in
> move_reasons.c. 
> 
>   /* Reason not found, add it if there is place left in both lists.
>    *
>    * FIXME: We should just drop the move reason silently in stable
>    *        releases if some list is full.
>    */
>   gg_assert(k < MAX_REASONS);
>   gg_assert(next_reason < MAX_MOVE_REASONS);
> 
> I think we should implement the FIXME also for development versions.
> It shouldn't matter all that much if a couple of the last induced move
> reasons are missing. When huge amounts of move reasons are generated
> the correct move can be assumed to have a sufficient number already.

We've had another crash caused by the same bug. The appended patch
solves the problem and also another bug (causing segmentation
violation) which previously was extremely unlikely to be triggered but
after the patch became easily triggered.

- drop move reasons in add_move_reason() when the lists become full
- bugfix in value_move_reasons()
- MAX_REASONS increased to 120

/Gunnar

Index: engine/move_reasons.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/move_reasons.c,v
retrieving revision 1.108
diff -u -r1.108 move_reasons.c
--- engine/move_reasons.c       2 Jan 2003 00:23:28 -0000       1.108
+++ engine/move_reasons.c       27 Jan 2003 21:20:23 -0000
@@ -406,12 +406,21 @@
   }
 
   /* Reason not found, add it if there is place left in both lists.
-   *
-   * FIXME: We should just drop the move reason silently in stable
-   *        releases if some list is full.
+   * Otherwise drop it.
    */
-  gg_assert(k < MAX_REASONS);
-  gg_assert(next_reason < MAX_MOVE_REASONS);
+  if (k >= MAX_REASONS) {
+    DEBUG(DEBUG_MOVE_REASONS,
+         "Move reason at %1m (type=%d, what=%d) dropped because list full.\n",
+         pos, type, what);
+    return;
+  }
+
+  if (next_reason >= MAX_MOVE_REASONS) {
+    DEBUG(DEBUG_MOVE_REASONS,
+         "Move reason at %1m (type=%d, what=%d) dropped because global list 
full.\n",
+         pos, type, what);
+    return;
+  }
 
   /* Add a new entry. */
   move[pos].reason[k] = next_reason;
Index: engine/move_reasons.h
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/move_reasons.h,v
retrieving revision 1.31
diff -u -r1.31 move_reasons.h
--- engine/move_reasons.h       2 Jan 2003 00:23:28 -0000       1.31
+++ engine/move_reasons.h       27 Jan 2003 21:20:24 -0000
@@ -78,7 +78,7 @@
 #define REDUNDANT               (TERRITORY_REDUNDANT | STRATEGICALLY_REDUNDANT)
 #define SECONDARY               4
 
-#define MAX_REASONS 100
+#define MAX_REASONS 120
 
 #define MAX_TRACE_LENGTH  160
 
Index: engine/value_moves.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/value_moves.c,v
retrieving revision 1.79
diff -u -r1.79 value_moves.c
--- engine/value_moves.c        12 Jan 2003 15:26:53 -0000      1.79
+++ engine/value_moves.c        27 Jan 2003 21:20:45 -0000
@@ -2497,7 +2497,7 @@
      * the reasons for different moves in the trace outputs.
      */
     num_reasons = 0;
-    while (move[pos].reason[num_reasons] >= 0)
+    while (move[pos].reason[num_reasons] >= 0 && num_reasons < MAX_REASONS)
       num_reasons++;
     gg_sort(move[pos].reason, num_reasons, sizeof(move[pos].reason[0]),
            compare_move_reasons);