summaryrefslogtreecommitdiff
path: root/src/gamelist.c
diff options
context:
space:
mode:
authorScott Gasch <[email protected]>2026-10-15 11:12:37 -0700
committerScott Gasch <[email protected]>2026-10-15 11:12:37 -0700
commitef2c2f667c4fc2c0303f56ea487183916f82fb67 (patch)
treed781a71ea958ca421521b21c93eb9c2ce59df65c /src/gamelist.c
parentd9419c104e9fe193649931cee167b819044c6ca3 (diff)
Fix several latent search/move-generation bugs found via NNUE annotation tooling
- probe.c: EGTB win/loss scores now render as +-MATE100 (literal +-NMATE) instead of a misleading float, matching how a real forced mate prints. - hash.c: fix a sign error in StoreExactScore's loss-side hash bound (iValue +NMATE paired with HASH_FLAG_UPPER, should be -NMATE) -- dormant until the probe.c fix made it reachable every EGTB loss. - san.c: a checking castle move (O-O+/O-O-O+) printed as +-O/+-O-O -- the check-suffix pointer was captured before strcpy filled the buffer, so '+' overwrote the leading 'O' instead of appending. - search.c/searchsup.c: MATEMOVE/DRAWMOVE/RECOGNMOVE/EGTBMOVE now record into the PV unconditionally instead of only when the ambient alpha-beta window happens to strictly contain the score -- these are exact, game-theoretic results, not heuristic bounds, so a PVS scout-window tie (common once a line's value is already known) shouldn't suppress them. This is why <#>/<=>/<REC>/<EGTB> almost never showed up in a printed PV. - util.c: WalkPV now reconstructs a still-missing final move/sentinel directly from the board when the recorded PV runs out one ply short of (or exactly at) a forced mate/stalemate/draw, instead of leaving the printed PV silently truncated. - gamelist.c/board.c/testsup.c: SetRootPosition now parses into a local staging POSITION and validates IsBoardLegal + VerifyPositionConsistency (both moved/enabled for all build profiles, not just TEST/EVAL_DUMP) before committing to g_RootPosition. Previously a rejected setboard (e.g. a hand-built FEN leaving the side not on move in check) still left the partially-parsed, illegal position in g_RootPosition for the next `go` to search -- GetRootPosition() hands out a stable pointer straight to it with no defensive copy -- causing a crash instead of the clean "Error (malformed FEN)" rejection already printed. All verified via precommit_check.sh (TEST=1 self-test suite + DEBUG smoke test) in addition to targeted manual repros for each bug. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Diffstat (limited to 'src/gamelist.c')
-rw-r--r--src/gamelist.c40
1 files changed, 39 insertions, 1 deletions
diff --git a/src/gamelist.c b/src/gamelist.c
index 3af04a8..a409f07 100644
--- a/src/gamelist.c
+++ b/src/gamelist.c
@@ -247,8 +247,46 @@ Return value:
**/
{
- if (TRUE == FenToPosition(&g_RootPosition, szFen))
+ POSITION sCandidate;
+
+ // 2026-10-15: parse into a local staging POSITION first, instead
+ // of straight into g_RootPosition, and only commit on success.
+ // FenToPosition is deliberately just a syntax parser -- it accepts
+ // any piece arrangement that parses, legal chess position or not
+ // (GenerateRandomLegalFenString's own round-trip tests rely on
+ // that, see testfen.c), so legality is checked here, the one real
+ // "load a position to actually search" entry point, via the
+ // existing IsBoardLegal (testsup.c -- already used the same way
+ // by TestLegalMoveGenerator: parse, then separately check
+ // legality). This also fixes a real crash: GetRootPosition()
+ // hands out a stable pointer straight to g_RootPosition with no
+ // defensive copy (removing one was a deliberate, confirmed-correct
+ // fix for a different bug -- see that function's own comment), so
+ // committing straight into g_RootPosition before validating meant
+ // a rejected setboard (e.g. a hand-built FEN leaving the side not
+ // on move in check -- confirmed reachable: a pseudo-legal "capture
+ // the enemy king" move, generated because nothing stops the
+ // generator from producing one, overflows _ScoreAllMoves's
+ // PIECE_VALUE(KING)-based MVV/LVA score past VALUE_KING) still
+ // left the bad position in place for the next `go` to search
+ // regardless of this function correctly reporting failure.
+ //
+ // VerifyPositionConsistency checks a different, complementary
+ // thing than IsBoardLegal: internal POSITION-struct consistency
+ // (piece lists, material counts, signatures -- could be wrong via
+ // a FenToPosition bug even on a legal-looking FEN), not chess-rule
+ // legality. Every other call site in the codebase gates it behind
+ // #ifdef DEBUG since it's rebuild-from-scratch expensive and most
+ // of those sites are hot per-move paths (MakeMove/UnmakeMove); this
+ // one isn't -- loading a root position happens once per game/
+ // setboard, not per node -- so it's worth the cost unconditionally,
+ // in release builds too, at the one place a bad position would
+ // otherwise go on to a real search instead of just a test harness.
+ if ((TRUE == FenToPosition(&sCandidate, szFen)) &&
+ (TRUE == IsBoardLegal(&sCandidate)) &&
+ (TRUE == VerifyPositionConsistency(&sCandidate, TRUE)))
{
+ g_RootPosition = sCandidate;
ResetGameList();
g_uToMove = g_RootPosition.uToMove;
TellGamelistThatIPlayColor(FLIP(g_RootPosition.uToMove));