Fixed #35 - fixed crash in tactical benchmark.

cosinekitty · cosinekitty · commit f9047e768f4a · 2022-12-28T11:22:34.000-05:00
The tactical benchmark could crash due to accessing
invalid chess move in ChessBoard::NumberOfRepetitions().
That function tries to repeat making moves on the board,
but the "moves" are not always valid chess moves.

NumberOfRepetitions() already had a safety check to
make sure the board was not edited before doing anything.
The safety check did not work in the case of the benchmark
because the benchmark didn't set an initial FEN
(Forsyth Edwards Notation) string. A non-null FEN string
is required to indicate that a position has been edited.

The fix was to make sure the benchmark initializes
the FEN string for the starting position before
doing each min-max search.
diff --git a/src/board.cpp b/src/board.cpp
@@ -1487,15 +1487,21 @@ void ChessBoard::SaveSpecialMove ( Move special )
 
     initialPlyNumber = ++ply_number;        // Remember the spot right after the last pseudo-move was saved
 
-    char fen [200];
-    if (GetForsythEdwardsNotation (fen, sizeof(fen)))
-    {
-        ReplaceString (initialFen, fen);
-    }
+    MarkInitialPosition();
+}
+
+
+void ChessBoard::MarkInitialPosition()
+{
+    // Record the current position as the initial position for this game.
+    // This has the side-effect of indicating that the board has been edited.
+    // This is needed to prevent crashes in functions like NumberOfRepetitions().
+
+    char fen[200];
+    if (GetForsythEdwardsNotation(fen, sizeof(fen)))
+        ReplaceString(initialFen, fen);
     else
-    {
-        ChessFatal ("ChessBoard::SaveSpecialMove: Unable to save initial FEN");
-    }
+        ChessFatal("ChessBoard::MarkInitialPosition: Unable to save initial FEN");
 }
 
 
diff --git a/src/chess.h b/src/chess.h
@@ -1278,7 +1278,8 @@ class ChessBoard
     bool  IsDefiniteDraw ( int *numReps = 0 );      // Does NOT find stalemate!
     int NumberOfRepetitions();
 
-    void   SaveSpecialMove ( Move );
+    void SaveSpecialMove ( Move );
+    void MarkInitialPosition();
 
     void   MakeWhiteMove (
         Move       &move,     // can have side-effects!
diff --git a/src/uiwin32.cpp b/src/uiwin32.cpp
@@ -2158,6 +2158,7 @@ double ChessUI_win32_gui::runTacticalBenchmark()
     tboard.SetSquareContents ( BROOK, 0, 7 );
     tboard.SetSquareContents ( BQUEEN, 3, 7 );
     tboard.SetSquareContents ( BROOK, 4, 7 );
+    tboard.MarkInitialPosition();
     benchmarkExpectedMove.source = 0;
     benchmarkExpectedMove.dest = 0;
     TheBoardDisplayBuffer.update (tboard);
@@ -2180,6 +2181,7 @@ double ChessUI_win32_gui::runTacticalBenchmark()
     tboard.SetSquareContents ( BPAWN, 0, 6 );
     tboard.SetSquareContents ( BPAWN, 1, 6 );
     tboard.SetSquareContents ( BBISHOP, 3, 6 );
+    tboard.MarkInitialPosition();
     TheBoardDisplayBuffer.update (tboard);
     TheBoardDisplayBuffer.freshenBoard();
     benchmarkExpectedMove.source = 0;
@@ -2210,6 +2212,7 @@ double ChessUI_win32_gui::runTacticalBenchmark()
     tboard.SetSquareContents ( BPAWN, 7, 5 );
     tboard.SetSquareContents ( BKNIGHT, 4, 6 );
     tboard.SetSquareContents ( BROOK, 5, 7 );
+    tboard.MarkInitialPosition();
     move.source = OFFSET(9,3);
     move.dest = OFFSET(8,3);
     tboard.MakeMove ( move, unmove );
@@ -2248,6 +2251,7 @@ double ChessUI_win32_gui::runTacticalBenchmark()
     tboard.SetSquareContents ( BKING, 0, 7 );
     tboard.SetSquareContents ( BROOK, 2, 7 );
     tboard.SetSquareContents ( BROOK, 4, 7 );
+    tboard.MarkInitialPosition();
     benchmarkExpectedMove.source = 0;
     benchmarkExpectedMove.dest = 0;
     benchmarkPlayer->SetSearchDepth (4);
@@ -2292,6 +2296,7 @@ double ChessUI_win32_gui::runTacticalBenchmark()
     tboard.SetSquareContents ( BROOK, 4, 7 );
     tboard.SetSquareContents ( BQUEEN, 3, 7 );
     tboard.SetSquareContents ( BROOK, 0, 7 );
+    tboard.MarkInitialPosition();
     benchmarkExpectedMove.source = 0;
     benchmarkExpectedMove.dest = 0;
     benchmarkPlayer->SetSearchDepth (4);
