From 229bd1e2e350b4b67ed5a962ae99d939c0a27d29 Mon Sep 17 00:00:00 2001 From: Timothy Herchen Date: Tue, 11 Nov 2025 06:09:26 -0800 Subject: [PATCH] check for material key validity in tbprobe During recent work on threat inputs, there was a hard-to-debug crash in tbprobe caused by incorrectly computing st->materialKey. This PR adds correctness checking to pos_is_ok and a faster check in tbprobe that will actually run in CI (because pos_is_ok checking is disabled even in debug mode, for performance purposes). closes https://github.com/official-stockfish/Stockfish/pull/6410 No functional change --- src/position.cpp | 16 +++++++++++++--- src/position.h | 2 ++ src/syzygy/tbprobe.cpp | 2 ++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/position.cpp b/src/position.cpp index 347bbcfc8..c34ceb400 100644 --- a/src/position.cpp +++ b/src/position.cpp @@ -336,8 +336,8 @@ void Position::set_check_info() const { // The function is only used when a new position is set up void Position::set_state() const { - st->key = st->materialKey = 0; - st->minorPieceKey = 0; + st->key = 0; + st->minorPieceKey = 0; st->nonPawnKey[WHITE] = st->nonPawnKey[BLACK] = 0; st->pawnKey = Zobrist::noPawns; st->nonPawnMaterial[WHITE] = st->nonPawnMaterial[BLACK] = VALUE_ZERO; @@ -375,10 +375,15 @@ void Position::set_state() const { st->key ^= Zobrist::side; st->key ^= Zobrist::castling[st->castlingRights]; + st->materialKey = compute_material_key(); +} +Key Position::compute_material_key() const { + Key k = 0; for (Piece pc : Pieces) for (int cnt = 0; cnt < pieceCount[pc]; ++cnt) - st->materialKey ^= Zobrist::psq[pc][8 + cnt]; + k ^= Zobrist::psq[pc][8 + cnt]; + return k; } @@ -1447,6 +1452,9 @@ void Position::flip() { } +bool Position::material_key_is_ok() const { return compute_material_key() == st->materialKey; } + + // Performs some consistency checks for the position object // and raise an assert if something wrong is detected. // This is meant to be helpful when debugging. @@ -1496,6 +1504,8 @@ bool Position::pos_is_ok() const { assert(0 && "pos_is_ok: Castling"); } + assert(material_key_is_ok() && "pos_is_ok: materialKey"); + return true; } diff --git a/src/position.h b/src/position.h index c95697d19..711c4e444 100644 --- a/src/position.h +++ b/src/position.h @@ -169,6 +169,7 @@ class Position { // Position consistency check, for debugging bool pos_is_ok() const; + bool material_key_is_ok() const; void flip(); StateInfo* state() const; @@ -180,6 +181,7 @@ class Position { private: // Initialization helpers (used while setting up a position) void set_castling_right(Color c, Square rfrom); + Key compute_material_key() const; void set_state() const; void set_check_info() const; diff --git a/src/syzygy/tbprobe.cpp b/src/syzygy/tbprobe.cpp index cbf8dce5e..657866ba4 100644 --- a/src/syzygy/tbprobe.cpp +++ b/src/syzygy/tbprobe.cpp @@ -1214,6 +1214,8 @@ template void* mapped(TBTable& e, const Position& pos) { static std::mutex mutex; + // Because TB is the only usage of materialKey, check it here in debug mode + assert(pos.material_key_is_ok()); // Use 'acquire' to avoid a thread reading 'ready' == true while // another is still working. (compiler reordering may cause this).