Possible slider_blockers() bug?

[daniloarcidiacono]

Hello, I think I've found a problem with the slider_blockers() routine. Consider the position 8/8/8/8/8/Q2r1R1k/8/2K5 b - - 0 1 (view on lichess).

According to the following test code (I've just removed the UCI::loop() call in main()):

int main(int argc, char* argv[]) {
  ...      
  std::string fen = "8/8/8/8/8/Q2r1R1k/8/2K5 b - - 0 1";
  StateInfo si;
  Position pos;
  pos.set(fen, false, &si, Threads.main());
  std::cout << "Blockers for black king on square " << UCI::square(pos.square<KING>(Color::BLACK)) << std::endl;
  std::cout << Bitboards::pretty(pos.blockers_for_king(Color::BLACK)) << std::endl;

  std::cout << "Pinners for black pieces" << std::endl;
  std::cout << Bitboards::pretty(pos.pinners(Color::WHITE)) << std::endl;

  // UCI::loop(argc, argv);

  Threads.set(0);
  return 0;
}

Stockfish thinks that the black rook on D3 is pinned to the black king on H3 by the white queen on A3.

C:\workspace_cpp\Stockfish-master\cmake-build-debug\Stockfish_14d-master.exe
Stockfish 310721 by the Stockfish developers (see AUTHORS file)
Blockers for black king on square h3
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 8
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 7
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 6
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 5
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 4
+---+---+---+---+---+---+---+---+
|   |   |   | X |   |   |   |   | 3
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 2
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 1
+---+---+---+---+---+---+---+---+
  a   b   c   d   e   f   g   h

Pinners for black pieces
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 8
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 7
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 6
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 5
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 4
+---+---+---+---+---+---+---+---+
| X |   |   |   |   |   |   |   | 3
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 2
+---+---+---+---+---+---+---+---+
|   |   |   |   |   |   |   |   | 1
+---+---+---+---+---+---+---+---+
  a   b   c   d   e   f   g   h

Process finished with exit code 0

Of course, this is not true, since it's actually the white rook on F3 that is checking the king, so the black rook can freely move.

By looking at the code we have:

si->blockersForKing[BLACK] = slider_blockers(pieces(WHITE), square<KING>(BLACK), si->pinners[WHITE]);

and

/// Position::slider_blockers() returns a bitboard of all the pieces (both colors)
/// that are blocking attacks on the square 's' from 'sliders'. A piece blocks a
/// slider if removing that piece from the board would result in a position where
/// square 's' is attacked. For example, a king-attack blocking piece can be either
/// a pinned or a discovered check piece, according if its color is the opposite
/// or the same of the color of the slider.    
Bitboard Position::slider_blockers(Bitboard sliders, Square s, Bitboard& pinners) const 

which, for our actual parameters, would "translate" to

_Position::sliderblockers() returns a bitboard of all the pieces (both colors) that are blocking attacks on the black king square from the white pieces. A piece blocks a slider if removing that piece from the board would result in a position where the black king square is attacked.

One could argue that if we remove the black rook the king is still attacked, but not by the queen! Is this the intended behaviour or indeed a problem? If so, I think it can be solved by doing another "round" when calculating the snipers:

Bitboard Position::slider_blockers(Bitboard sliders, Square s, Bitboard& pinners) const {

  Bitboard blockers = 0;
  pinners = 0;

  // Snipers are sliders that attack 's' when a piece and other snipers are removed
  Bitboard snipers = (  (attacks_bb<  ROOK>(s) & pieces(QUEEN, ROOK))
                      | (attacks_bb<BISHOP>(s) & pieces(QUEEN, BISHOP))) & sliders;

  // FIX: Second round to remove x-raying snipers
  snipers = (  (attacks_bb<  ROOK>(s, snipers) & pieces(QUEEN, ROOK))
             | (attacks_bb<BISHOP>(s, snipers) & pieces(QUEEN, BISHOP))) & sliders;
  // FIX: Second round to remove x-raying snipers

  Bitboard occupancy = pieces() ^ snipers;

What do you think?

Edit: forgot to add that I've tested this on the master branch a few hours ago. Edit 2: Fixed the FEN which was illegal

[vondele]

the fen is illegal as both kings are in check, which can't happen in chess?

[daniloarcidiacono]

Woops... sorry... however by moving the white king one square to the left the position should become legal... I'll edit the post

Come to think of it, it's a slight inaccuracy in the pinners bitmask but, depending on how pinners are used in the rest of the code, that inaccuracy may not (and probably does not) cause any problems. Adding complexity to handle this scenario is probably not worth the loss in speed, since if I'm not mistaken slider_blockers() is called a lot.