diff --git a/src/engine.cpp b/src/engine.cpp index be0fe3c40..c26567854 100644 --- a/src/engine.cpp +++ b/src/engine.cpp @@ -195,21 +195,26 @@ void Engine::set_on_verify_networks(std::function&& f) { void Engine::wait_for_search_finished() { threads.main_thread()->wait_for_search_finished(); } -void Engine::set_position(const std::string& fen, const std::vector& moves) { +std::optional Engine::set_position(const std::string& fen, + const std::vector& moves) { // Drop the old state and create a new one - states = StateListPtr(new std::deque(1)); - pos.set(fen, options["UCI_Chess960"], &states->back()); + states = StateListPtr(new std::deque(1)); + auto err = pos.set(fen, options["UCI_Chess960"], &states->back()); + if (err.has_value()) + return err; for (const auto& move : moves) { auto m = UCIEngine::to_move(pos, move); if (m == Move::none()) - break; + return PositionSetError("Illegal move: " + move); states->emplace_back(); pos.do_move(m, states->back()); } + + return std::nullopt; } // modifiers diff --git a/src/engine.h b/src/engine.h index 92d6282dc..e22466991 100644 --- a/src/engine.h +++ b/src/engine.h @@ -68,7 +68,8 @@ class Engine { // blocking call to wait for search to finish void wait_for_search_finished(); // set a new position, moves are in UCI format - void set_position(const std::string& fen, const std::vector& moves); + std::optional set_position(const std::string& fen, + const std::vector& moves); // modifiers diff --git a/src/position.cpp b/src/position.cpp index daadf59ec..f9b94b5dd 100644 --- a/src/position.cpp +++ b/src/position.cpp @@ -162,9 +162,10 @@ void Position::init() { // Initializes the position object with the given FEN string. -// This function is not very robust - make sure that input FENs are correct, -// this is assumed to be the responsibility of the GUI. -Position& Position::set(const string& fenStr, bool isChess960, StateInfo* si) { +// The FEN string is strictly validated; if it is invalid or inconsistent, +// a PositionSetError describing the problem is returned, otherwise std::nullopt. +std::optional +Position::set(const string& fenStr, bool isChess960, StateInfo* si) { /* A FEN string defines a particular position using only the ASCII character set. @@ -200,9 +201,7 @@ Position& Position::set(const string& fenStr, bool isChess960, StateInfo* si) { incremented after Black's move. */ - unsigned char col, row, token; - size_t idx; - Square sq = SQ_A8; + unsigned char token; std::istringstream ss(fenStr); std::memset(reinterpret_cast(this), 0, sizeof(Position)); @@ -211,81 +210,214 @@ Position& Position::set(const string& fenStr, bool isChess960, StateInfo* si) { ss >> std::noskipws; + int numPieces = 0; + int file = FILE_A; + int rank = RANK_8; + // 1. Piece placement - while ((ss >> token) && !isspace(token)) + for (;;) { + if (!(ss >> token)) + return PositionSetError("Invalid FEN. Unexpected end of stream."); + + if (isspace(token)) + break; + if (isdigit(token)) - sq += (token - '0') * EAST; // Advance the given number of files - - else if (token == '/') - sq += 2 * SOUTH; - - else if ((idx = PieceToChar.find(token)) != string::npos) { + const int diff = (token - '0'); + if (diff < 1 || diff > 8) + return PositionSetError("Invalid FEN. Invalid number of squares to skip."); + + file += diff; + if (file > FILE_NB) + return PositionSetError("Invalid FEN. Invalid file reached."); + } + else if (token == '/') + { + if (file != FILE_NB) + return PositionSetError( + "Invalid FEN. Trying to end rank when not at the end of it."); + + --rank; + file = FILE_A; + + if (rank < RANK_1) + return PositionSetError("Invalid FEN. Invalid rank reached."); + } + else + { + if (file >= FILE_NB) + return PositionSetError("Invalid FEN. Invalid file reached."); + + const size_t idx = PieceToChar.find(token); + if (idx == string::npos) + return PositionSetError(std::string("Invalid FEN. Invalid piece: ") + + std::string(1, token)); + + if (++numPieces > 32) + return PositionSetError("Invalid FEN. More than 32 pieces on the board."); + + const Square sq = make_square(File(file), Rank(rank)); put_piece(Piece(idx), sq); - ++sq; + + ++file; } } + if (rank != RANK_1 || file != FILE_NB) + return PositionSetError("Invalid FEN. Board state encoding ended but cursor not at end."); + + if (pieces(PAWN) & (RANK_1 | RANK_8)) + return PositionSetError("Unsupported position. Pawns on the first or eighth rank."); + + if (count(WHITE) != 1 || count(BLACK) != 1) + return PositionSetError("Unsupported position. Incorrect number of kings."); + + const int wPawns = count(WHITE); + const int bPawns = count(BLACK); + if (wPawns > 8) + return PositionSetError("Unsupported position. WHITE has more than 8 pawns."); + if (bPawns > 8) + return PositionSetError("Unsupported position. BLACK has more than 8 pawns."); + + const int wAdditionalKnights = std::max((int) count(WHITE) - 2, 0); + const int bAdditionalKnights = std::max((int) count(BLACK) - 2, 0); + const int wAdditionalBishops = std::max((int) count(WHITE) - 2, 0); + const int bAdditionalBishops = std::max((int) count(BLACK) - 2, 0); + const int wAdditionalRooks = std::max((int) count(WHITE) - 2, 0); + const int bAdditionalRooks = std::max((int) count(BLACK) - 2, 0); + const int wAdditionalQueens = std::max((int) count(WHITE) - 1, 0); + const int bAdditionalQueens = std::max((int) count(BLACK) - 1, 0); + if (wAdditionalKnights + wAdditionalBishops + wAdditionalRooks + wAdditionalQueens > 8 - wPawns) + return PositionSetError("Unsupported position. Too many major pieces for WHITE."); + if (bAdditionalKnights + bAdditionalBishops + bAdditionalRooks + bAdditionalQueens > 8 - bPawns) + return PositionSetError("Unsupported position. Too many major pieces for BLACK."); + // 2. Active color - ss >> token; + if (!(ss >> token)) + return PositionSetError("Invalid FEN. Unexpected end of stream."); + if (token != 'w' && token != 'b') + return PositionSetError(std::string("Invalid FEN. Invalid side to move: ") + + std::string(1, token)); sideToMove = (token == 'w' ? WHITE : BLACK); - ss >> token; + if (!(ss >> token) || !isspace(token) || ss.eof()) + return PositionSetError("Invalid FEN. Expected whitespace after side to move."); // 3. Castling availability. Compatible with 3 standards: Normal FEN standard, // Shredder-FEN that uses the letters of the columns on which the rooks began // the game instead of KQkq and also X-FEN standard that, in case of Chess960, // if an inner rook is associated with the castling right, the castling tag is // replaced by the file letter of the involved rook, as for the Shredder-FEN. - while ((ss >> token) && !isspace(token)) + // + // NOTE: Due to the prevalnce of incorrect (or missing) castling rights the + // validation is less strict. However, incorrect castling rights are still sanitized. + int num_castling_rights = 0; + for (;;) { - Square rsq; + if (!(ss >> token)) + break; + + if (isspace(token)) + break; + + if (num_castling_rights == 0 && token == '-') + { + ss >> std::ws; + break; + } + + if (++num_castling_rights > 4) + return PositionSetError("Invalid FEN. Maximum of 4 castling rights can be specified."); + + Square rsq = SQ_NONE; + Square ksq = SQ_NONE; Color c = islower(token) ? BLACK : WHITE; Piece rook = make_piece(c, ROOK); + Piece king = make_piece(c, KING); token = char(toupper(token)); - if (token == 'K') - for (rsq = relative_square(c, SQ_H1); piece_on(rsq) != rook; --rsq) - {} - - else if (token == 'Q') - for (rsq = relative_square(c, SQ_A1); piece_on(rsq) != rook; ++rsq) - {} - + if (token == 'K' || token == 'Q') + { + const int dir = token == 'K' ? -1 : 1; + Square sq = relative_square(c, token == 'K' ? SQ_H1 : SQ_A1); + // Look for a rook and a king for the castling. King must come later. + // Only the first rook is noted. + // If the castling rights are available the king must always be between files 2 and 7 inclusive + // so there is no need to check the last square. + for (int i = 0; i < 7; ++i, sq = Square(sq + dir)) + { + const Piece pc = piece_on(sq); + if (pc == king) + { + ksq = sq; + break; + } + else if (pc == rook && rsq == SQ_NONE) + { + rsq = sq; + } + } + } else if (token >= 'A' && token <= 'H') - rsq = make_square(File(token - 'A'), relative_rank(c, RANK_1)); + { + const Square rsqCandidate = make_square(File(token - 'A'), relative_rank(c, RANK_1)); + ; + if (piece_on(rsqCandidate) == rook) + rsq = rsqCandidate; + // If the castling rights are available the king must always be between files 2 and 7 inclusive. + Square sq = relative_square(c, SQ_B1); + for (int i = 0; i < 6; ++i, ++sq) + { + if (piece_on(sq) == king) + ksq = sq; + } + } else - continue; + { + return PositionSetError(std::string("Invalid FEN. Expected castling rights. Got: ") + + std::string(1, token)); + } - set_castling_right(c, rsq); + // Only apply castling rights if they can be valid. + if (ksq != SQ_NONE && rsq != SQ_NONE) + set_castling_right(c, rsq); } // 4. En passant square. // Ignore if square is invalid or not on side to move relative rank 6. - bool enpassant = false, legalEP = false; - - if (((ss >> col) && (col >= 'a' && col <= 'h')) - && ((ss >> row) && (row == (sideToMove == WHITE ? '6' : '3')))) + bool enpassant = false, legalEP = false; + unsigned char col = '-', row; + ss >> col; + if (col != '-') { - st->epSquare = make_square(File(col - 'a'), Rank(row - '1')); + if (!(ss >> row)) + return PositionSetError("Invalid FEN. Unexpected end of stream."); - Bitboard pawns = attacks_bb(st->epSquare, ~sideToMove) & pieces(sideToMove, PAWN); - Bitboard target = (pieces(~sideToMove, PAWN) & (st->epSquare + pawn_push(~sideToMove))); - Bitboard occ = pieces() ^ target ^ st->epSquare; + if ((col >= 'a' && col <= 'h') && (row == (sideToMove == WHITE ? '6' : '3'))) + { + st->epSquare = make_square(File(col - 'a'), Rank(row - '1')); - // En passant square will be considered only if - // a) side to move have a pawn threatening epSquare - // b) there is an enemy pawn in front of epSquare - // c) there is no piece on epSquare or behind epSquare - enpassant = - pawns && target && !(pieces() & (st->epSquare | (st->epSquare + pawn_push(sideToMove)))); + Bitboard pawns = attacks_bb(st->epSquare, ~sideToMove) & pieces(sideToMove, PAWN); + Bitboard target = (pieces(~sideToMove, PAWN) & (st->epSquare + pawn_push(~sideToMove))); + Bitboard occ = pieces() ^ target ^ st->epSquare; - // If no pawn can execute the en passant capture without leaving the king in check, don't record the epSquare - while (pawns) - legalEP |= !(attackers_to(square(sideToMove), occ ^ pop_lsb(pawns)) - & pieces(~sideToMove) & ~target); + // En passant square will be considered only if + // a) side to move have a pawn threatening epSquare + // b) there is an enemy pawn in front of epSquare + // c) there is no piece on epSquare or behind epSquare + enpassant = pawns && target + && !(pieces() & (st->epSquare | (st->epSquare + pawn_push(sideToMove)))); + + // If no pawn can execute the en passant capture without leaving the king in check, don't record the epSquare + while (pawns) + legalEP |= !(attackers_to(square(sideToMove), occ ^ pop_lsb(pawns)) + & pieces(~sideToMove) & ~target); + } + else + return PositionSetError("Invalid FEN. Invalid en-passant square."); } if (!enpassant || !legalEP) @@ -294,6 +426,14 @@ Position& Position::set(const string& fenStr, bool isChess960, StateInfo* si) { // 5-6. Halfmove clock and fullmove number ss >> std::skipws >> st->rule50 >> gamePly; + // Normally values larger than 99 would be pointless but we do support ignoring 50 move rule for TB purposes. + // Limit at 2**15 as it's used multiplicativly with position evaluation during search. + if (st->rule50 < 0 || st->rule50 > 32767) + return PositionSetError("Unsupported position. Rule50 counter out of range."); + + if (gamePly < 0 || gamePly > 100000) + return PositionSetError("Unsupported position. Game ply out of range."); + // Convert from fullmove starting from 1 to gamePly starting from 0, // handle also common incorrect FEN with fullmove = 0. gamePly = std::max(2 * (gamePly - 1), 0) + (sideToMove == BLACK); @@ -301,9 +441,12 @@ Position& Position::set(const string& fenStr, bool isChess960, StateInfo* si) { chess960 = isChess960; set_state(); + if (attackers_to_exist(square(~sideToMove), pieces(), sideToMove)) + return PositionSetError("Unsupported position. King can be captured."); + assert(pos_is_ok()); - return *this; + return std::nullopt; } @@ -401,7 +544,7 @@ Key Position::compute_material_key() const { // Overload to initialize the position object with the given endgame code string // like "KBPKN". It's mainly a helper to get the material key out of an endgame code. -Position& Position::set(const string& code, Color c, StateInfo* si) { +std::optional Position::set(const string& code, Color c, StateInfo* si) { assert(code[0] == 'K'); diff --git a/src/position.h b/src/position.h index e02a400d3..693207aaa 100644 --- a/src/position.h +++ b/src/position.h @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include #include "bitboard.h" @@ -70,6 +72,12 @@ struct StateInfo { // elements are not invalidated upon list resizing. using StateListPtr = std::unique_ptr>; +// This error should be used whenever a position is suspected to be unsupported +// by the engine. In particular positions that may cause hard errors like segmentation fault. +struct PositionSetError: std::runtime_error { + using std::runtime_error::runtime_error; +}; + // Position class stores information regarding the board representation as // pieces, side to move, hash keys, castling info, etc. Important methods are // do_move() and undo_move(), used by the search to update node info when @@ -83,9 +91,9 @@ class Position { Position& operator=(const Position&) = delete; // FEN string input/output - Position& set(const std::string& fenStr, bool isChess960, StateInfo* si); - Position& set(const std::string& code, Color c, StateInfo* si); - std::string fen() const; + std::optional set(const std::string& fenStr, bool isChess960, StateInfo* si); + std::optional set(const std::string& code, Color c, StateInfo* si); + std::string fen() const; // Position representation Bitboard pieces() const; // All pieces diff --git a/src/syzygy/tbprobe.cpp b/src/syzygy/tbprobe.cpp index 9fe6df9dc..ba808ee4b 100644 --- a/src/syzygy/tbprobe.cpp +++ b/src/syzygy/tbprobe.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -386,7 +387,16 @@ TBTable::TBTable(const std::string& code) : StateInfo st; Position pos; - key = pos.set(code, WHITE, &st).material_key(); + auto err = pos.set(code, WHITE, &st); + // IMPORTANT: We cannot assert here because it WILL produce validation errors + // on some TB7 and higher positions due to the black king being attacked + // while white is to move. This is not fixable without significant changes. + // As using pos.set here is already a very hacky way to achieve the desired + // result here so we leave it for now. The validation checks that fail are + // done after the position is fully set up, so it's fine for now. + // assert(!err.has_value()); + (void) err; + key = pos.material_key(); pieceCount = pos.count(); hasPawns = pos.pieces(PAWN); @@ -404,7 +414,16 @@ TBTable::TBTable(const std::string& code) : pawnCount[0] = pos.count(c ? WHITE : BLACK); pawnCount[1] = pos.count(c ? BLACK : WHITE); - key2 = pos.set(code, BLACK, &st).material_key(); + err = pos.set(code, BLACK, &st); + // IMPORTANT: We cannot assert here because it WILL produce validation errors + // on some TB7 and higher positions due to the black king being attacked + // while white is to move. This is not fixable without significant changes. + // As using pos.set here is already a very hacky way to achieve the desired + // result here so we leave it for now. The validation checks that fail are + // done after the position is fully set up, so it's fine for now. + // assert(!err.has_value()); + (void) err; + key2 = pos.material_key(); } template<> diff --git a/src/uci.cpp b/src/uci.cpp index 927007686..54305bc31 100644 --- a/src/uci.cpp +++ b/src/uci.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -466,6 +467,8 @@ std::uint64_t UCIEngine::perft(const Search::LimitsType& limits) { } void UCIEngine::position(std::istringstream& is) { + const std::string fullCommand = is.str(); + std::string token, fen; is >> token; @@ -488,7 +491,11 @@ void UCIEngine::position(std::istringstream& is) { moves.push_back(token); } - engine.set_position(fen, moves); + auto err = engine.set_position(fen, moves); + if (err.has_value()) + { + terminate_on_critical_error(fullCommand, err->what()); + } } namespace { @@ -658,4 +665,12 @@ void UCIEngine::on_bestmove(std::string_view bestmove, std::string_view ponder) std::cout << sync_endl; } +void UCIEngine::terminate_on_critical_error(const std::string& fullCommand, + const std::string& message) { + sync_cout << "info string CRITICAL ERROR: Command `" << fullCommand + << "` failed. Reason: " << message << '\n' + << sync_endl; + std::exit(1); +} + } // namespace Stockfish diff --git a/src/uci.h b/src/uci.h index ebc04fc3c..85c74afd8 100644 --- a/src/uci.h +++ b/src/uci.h @@ -73,6 +73,9 @@ class UCIEngine { static void on_bestmove(std::string_view bestmove, std::string_view ponder); void init_search_update_listeners(); + + [[noreturn]] void terminate_on_critical_error(const std::string& fullCommand, + const std::string& message); }; } // namespace Stockfish