Skip to content

Commit 229bd1e

Browse files
anematodevondele
authored andcommitted
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 #6410 No functional change
1 parent 3f2405b commit 229bd1e

File tree

3 files changed

+17
-3
lines changed

3 files changed

+17
-3
lines changed

src/position.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,8 @@ void Position::set_check_info() const {
336336
// The function is only used when a new position is set up
337337
void Position::set_state() const {
338338

339-
st->key = st->materialKey = 0;
340-
st->minorPieceKey = 0;
339+
st->key = 0;
340+
st->minorPieceKey = 0;
341341
st->nonPawnKey[WHITE] = st->nonPawnKey[BLACK] = 0;
342342
st->pawnKey = Zobrist::noPawns;
343343
st->nonPawnMaterial[WHITE] = st->nonPawnMaterial[BLACK] = VALUE_ZERO;
@@ -375,10 +375,15 @@ void Position::set_state() const {
375375
st->key ^= Zobrist::side;
376376

377377
st->key ^= Zobrist::castling[st->castlingRights];
378+
st->materialKey = compute_material_key();
379+
}
378380

381+
Key Position::compute_material_key() const {
382+
Key k = 0;
379383
for (Piece pc : Pieces)
380384
for (int cnt = 0; cnt < pieceCount[pc]; ++cnt)
381-
st->materialKey ^= Zobrist::psq[pc][8 + cnt];
385+
k ^= Zobrist::psq[pc][8 + cnt];
386+
return k;
382387
}
383388

384389

@@ -1447,6 +1452,9 @@ void Position::flip() {
14471452
}
14481453

14491454

1455+
bool Position::material_key_is_ok() const { return compute_material_key() == st->materialKey; }
1456+
1457+
14501458
// Performs some consistency checks for the position object
14511459
// and raise an assert if something wrong is detected.
14521460
// This is meant to be helpful when debugging.
@@ -1496,6 +1504,8 @@ bool Position::pos_is_ok() const {
14961504
assert(0 && "pos_is_ok: Castling");
14971505
}
14981506

1507+
assert(material_key_is_ok() && "pos_is_ok: materialKey");
1508+
14991509
return true;
15001510
}
15011511

src/position.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ class Position {
169169

170170
// Position consistency check, for debugging
171171
bool pos_is_ok() const;
172+
bool material_key_is_ok() const;
172173
void flip();
173174

174175
StateInfo* state() const;
@@ -180,6 +181,7 @@ class Position {
180181
private:
181182
// Initialization helpers (used while setting up a position)
182183
void set_castling_right(Color c, Square rfrom);
184+
Key compute_material_key() const;
183185
void set_state() const;
184186
void set_check_info() const;
185187

src/syzygy/tbprobe.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,8 @@ template<TBType Type>
12141214
void* mapped(TBTable<Type>& e, const Position& pos) {
12151215

12161216
static std::mutex mutex;
1217+
// Because TB is the only usage of materialKey, check it here in debug mode
1218+
assert(pos.material_key_is_ok());
12171219

12181220
// Use 'acquire' to avoid a thread reading 'ready' == true while
12191221
// another is still working. (compiler reordering may cause this).

0 commit comments

Comments
 (0)