From fbb138cc1dd13da2f30129206cdcd3d128344f54 Mon Sep 17 00:00:00 2001 From: Scott Gasch Date: Tue, 8 Sep 2026 16:50:55 -0700 Subject: Retire asm GetAttacks, recogn.c/fen.c bugfixes, misc bugfixes verified at parity Confirmed self-play regression traced to a stale test_vs_head.sh reference binary (typhoon_allbitboards/550ea81, deleted): every "vs head" comparison since 92fc412 (Sep 4) was checking new work against that fixed Sep-4 snapshot, never against real HEAD or the working tree. Rebuilt clean reference binaries directly from git and re-verified everything from scratch. This commit lands only the pieces confirmed safe against clean 434fa04 (fast st1 match, ~30-40 games, score ~0.44-0.55, consistent with parity; plus a DEBUG-build smoke test pass): - recogn.c, fen.c: real bugfixes - data.c, draw.c, ics.c: whitespace only - x64.asm: retires the asm GetAttacks implementation now that chess.h's GetAttacks macro unconditionally selects the already-verified-faster _GetAttacksBB bitboard version instead of a three-way build-flag toggle (GETATTACKS_BITBOARD/CROUTINES/asm default) - see.c, testsee.c: SEE/test-harness updates supporting that default - root.c: per-tier eval-exit reporting (super-lazy counters currently always read 0 -- accurate, since no super-lazy exit exists yet) - main.c: startup banner update, InitEval() call, TestRecogn() added to the #ifdef TEST self-test sequence - command.c: InitEval() DNA-reload hook, new qsearchfutility diagnostic - dynamic.c: minor changes - chess.h: the GetAttacks default change above, three FUTILITY_BASE_MARGIN_* compatibility aliases (all still equal to the original flat FUTILITY_BASE_MARGIN -- search.c has not been split into per-tier margins here), placeholder super-lazy counters, and an EvalPasserRaces -> _EvalPasserRacesAgainstLoneKings rename (confirmed byte-identical body) to match recogn.c's call site - eval.c: the same rename, plus a no-op InitEval() stub (nothing to initialize until the ROOK_FULL_HALF_OPEN_BONUS cache below exists) Deliberately NOT included: the full eval.c overhaul (~1770 lines) and search.c's qsearch-futility rework (~650 lines), including yesterday's loosened SUPER_LAZY_MARGIN_BY_ARMY/FUTILITY_BASE_MARGIN_BY_SOURCE tables. Reverting just those two tables while keeping the rest of the eval.c overhaul still lost badly to 434fa04 (0.20 over 10 games), so the regression isn't fully explained by the margins alone -- the eval.c overhaul needs careful, incremental re-verification against this commit as the new baseline, not a bulk re-apply. Full original work preserved in git stash (stash@{0} as of this commit) for that follow-up. Note: two pre-existing, position/state-dependent assertion crashes were found during this verification (util.c:1093 WalkPV, recogn.c:1359 _SanityCheckRecognizers), both reproducing on unmodified 434fa04 -- not introduced by anything here, not yet root-caused. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ka9o3S2eKqh4jNfmxVZ6fH --- src/fen.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 73 insertions(+), 11 deletions(-) (limited to 'src/fen.c') diff --git a/src/fen.c b/src/fen.c index b42ee04..33810e6 100755 --- a/src/fen.c +++ b/src/fen.c @@ -588,32 +588,94 @@ Return value: **/ { - ULONG u = 0; + ULONG u = 1; + ULONG uChunkIndex = 0; int i; CHAR *q, *op; - + p->uFifty = 0; + // + // A FEN that omits the halfmove-clock/fullmove-number suffix + // entirely (routine in EPD test suites like tests/ecm.ep_ -- e.g. + // "... w - -" with nothing after the en passant field) makes + // szFifty NULL here (FindChunk(szCapturedFen, 5) found no 5th + // chunk at all). The original code happened to survive that + // silently because FindChunk(NULL, 0) hits its "0 means whole + // string" fast path and returns NULL without ever dereferencing + // sz; starting at chunk 1 below (see the comment further down) + // loses that accidental safety net since chunk-1 lookups always + // dereference sz. Handle it explicitly instead of relying on + // FindChunk's early-return shape again -- found live by + // precommit_check.sh's debug_smoke_test.sh crashing on the very + // first random ecm.ep_ position after the chunk-index fix below + // was added. + // + if (NULL == szFifty) + { + return; + } + // + // Start at chunk 1 (FindChunk's convention: 0 means "whole + // remaining string", 1 means "the first token"), not 0. Starting + // at 0 here made this loop visit the first token twice -- once via + // the u==0 "whole string" fetch below, again via the very first + // u==1 fetch inside the loop -- throwing off the uChunkIndex + // bookkeeping used to tell the halfmove clock (real chunk 1) apart + // from the fullmove number (real chunk 2) below. + // q = FindChunk(szFifty, u); u++; while(NULL != q) { //printf("%u: %s\n", u-1, q); - - i = atoi(q); - if ((i > 0) && (i < 100)) + + if (0 == uChunkIndex) + { + // + // In a real FEN string this chunk is always the halfmove + // (fifty-move-rule) clock -- unlike every later chunk, 0 is + // a legitimate value here (a fresh game, or right after a + // capture/pawn move), not "absent". The generic i>0 check + // below would skip a genuine 0 and fall through to the + // *next* chunk (the fullmove number) instead, silently + // assigning that to uFifty. Confirmed live: the engine's + // own hardcoded starting-position FEN ("... - 0 1") was + // parsed as uFifty=1 (the fullmove number) rather than 0. + // + i = atoi(q); + if ((i >= 0) && (i < 100)) + { + p->uFifty = (ULONG)i; + } + } + else if (1 == uChunkIndex) { - p->uFifty = (ULONG)i; + // + // The second chunk is the fullmove number, not used by + // this engine -- just consumed here so the generic + // EPD-opcode scan below doesn't misread it as a repeated + // (and wrong) fifty-move value. + // } else { - if (!STRCMPI(q, "bm")) + i = atoi(q); + if ((i > 0) && (i < 100)) { - op = FindChunk(szFifty, u); - u++; - if (NULL == op) break; + p->uFifty = (ULONG)i; + } + else + { + if (!STRCMPI(q, "bm")) + { + op = FindChunk(szFifty, u); + u++; + if (NULL == op) break; + } + } - } + uChunkIndex++; q = FindChunk(szFifty, u); u++; } -- cgit v1.3