Position Causes Stockfish and Komodo To Crash

[syzygy]

The GUI will likely get terribly confused if it does not crash.

If the GUI can't deal with "info string" or "bestmove 0000", it isn't a UCI compliant GUI in the first place.

I see. I wasn't aware of "0000", which the spec does mention but does not explain. I'm afraid the spec as it is written is really terribly sloppy to the point of being useless. (So to know the real spec, you have to look at how it is implemented in practice, and then Komodo and Stockfish could be considered to lead the way.)

Does your engine also refuse to crash or freeze if you change the TT size or the number of search threads halfway through a search?

No crash here because that will be buffered until after search ends. If the buffer overflows, it will be overwritten - which is OK because the GUI shouldn't be sending such commands during search in the first place.

It seems the written spec requires the command to be ignored altogether. At least, that is one way to read the spec.

Unless we're dealing with defective hardware, any engine crash is a bug that needs to be fixed, that's the kind of robustness that I'm striving for.

Same for the C library and double free()s?

[syzygy]

There is a universal consensus, outside computer chess apparently, that a program should always handle illegal input gracefully. Would you want ld to crash if it does not recognize a file format?

I'm not aware of such a concensus for modules that are designed to work together. A program is not supposed to call free() twice on the same pointer. The C library is free to crash on the second free(). There are many such examples.

If you feed scripts you retrieve from the internet to the bash shell, you run the risk of wiping out your system, even if bash zealously checks the validity of the input. bash is dangerous, so use with care. A UCI chess engine is dangerous, too. Don't use it without a GUI unless you know what you're doing.

I think the real problem we have here is that the UCI spec is not a spec.

There is another point good which has already been mentioned. Since the concept of a legal chess position is not well defined (can we have 10 queens? 20? black and white pawns all on the 7th rank?) it seems better to let the engine decide if it can handle a fen and return an error (info string) if this is not so.

The concept of a legal chess position is well defined ("can occur in a legal game of chess"). But indeed it can be tricky to verify this.

Luckily, the "unreachable" positions with normal numbers of pieces and pawns are unlikely to pose a problem for any chess engine.
I suspect the trickiest positions are those with an in-check pattern that cannot legally occur, as the engine might make assumptions when generating evasions that are not valid in the illegal position. An example is Lc0, which can get confused if the king is attacked by two pawns at once:
https://github.com/LeelaChessZero/lc0/i ... -691946095
But still, this example is easily detected once you're aware of it.

What is missing is a clear statement in the "spec" what positions are supposed to be accepted. Then GUIs know what to check for and engine authors know what assumptions about the position they can make.

[jshriver]

If anyones curious I came across this while building a positions.txt for the Cerebrum NN library. It requires as input a FEN and both white and black eval in centipawns. So I took the TWIC database, ran through pgn-extract to build a massive fen list of every position from every game.

Since Cerebrum requires both black and white evals, i chopped off the "w - -/ b - -" and use the epd to test for both. Thats when I came across this position. I wrote a python script that interacts with any engine using UCI to automate the building process.

Seeing now some people have fen files with 200-300m or even 1B fen positions. Is there a tool to generate random legal FEN positions and dump to a file? I'm still workingon a bug-free legal move generator.

[Ras]

I wasn't aware of "0000", which the spec does mention but does not explain.

I understand the term "nullmove" as in "the engine doesn't have a move for whatever reason". With regular positions, that could be mates or stalemates.

I'm afraid the spec as it is written is really terribly sloppy to the point of being useless.

It isn't the best piece of technical documentation that I've seen, but it's workable.

It seems the written spec requires the command to be ignored altogether.

That would be the part of ignoring commands that are not supposed to come, like "setoption" when the engine isn't waiting. I implemented that in a more defensive way because if you don't discard commands, you can't discard them erroneously. Either way, a crash isn't allowed.

Same for the C library and double free()s?

The intricacies of C are no argument for being sloppy in one's own programs. If anything, it should be a motivation to do better.

[gbtami]

The GUI writers will tell you it is the responsibility of the engine or the user to detect bad data.

I consider myself a GUI writer. UCI spec is clear enough here I think. In https://www.pychess.org/editor/chess the user input is the position they construct. I never thought it is not my responsibility to check the FEN they create.

[gbtami]

Sure, you can have your opinion. But at the end of the day Marco Costalba decides

I guess you are wrong.

Sure. You can substitute current maintainers

[Sesse]

Sure. You can substitute current maintainers

Is there active resistance, or just “nobody has contributed a reasonable patch”?

[AndrewGrant]

Sure. You can substitute current maintainers

Is there active resistance, or just “nobody has contributed a reasonable patch”?

Typically, a person comes around and says "Look this crashes, fix it!" and everyone says no.
No one, to my knowledge, has actually showed up and presented a solution. So its laziness from the complainers too.

[MikeB]

A poorly constructed FEN caused SF to crash ...

Code: Select all

$ stockfish
Stockfish 121220 by the Stockfish developers (see AUTHORS file)
position fen 1B1b1k2/7p/1ppN3R/5Ppb/8/8/PP2rKPP/8
eval
info string NNUE evaluation using eval.bin enabled

     Term    |    White    |    Black    |    Total
             |   MG    EG  |   MG    EG  |   MG    EG
 ------------+-------------+-------------+------------
    Material |  ----  ---- |  ----  ---- |  0.47  1.04
   Imbalance |  ----  ---- |  ----  ---- | -0.12 -0.14
       Pawns |  0.22 -0.04 |  0.17  0.00 |  0.05 -0.04
     Knights | -0.15 -0.17 |  0.00  0.00 | -0.15 -0.17
     Bishops | -0.20 -0.35 | -0.20 -0.36 |  0.00  0.01
       Rooks |  0.00  0.00 |  0.23  0.13 | -0.23 -0.13
      Queens |  0.00  0.00 |  0.00  0.00 |  0.00  0.00
    Mobility |  0.34  0.64 |  0.57  0.94 | -0.23 -0.30
 King safety | -0.46 -0.24 | -1.10 -0.18 |  0.64 -0.06
     Threats |  1.26  1.54 |  0.98  0.86 |  0.28  0.68
      Passed |  0.37  0.29 |  0.00  0.00 |  0.37  0.29
       Space |  0.00  0.00 |  0.00  0.00 |  0.00  0.00
    Winnable |  ----  ---- |  ----  ---- |  0.00 -0.12
 ------------+-------------+-------------+------------
       Total |  ----  ---- |  ----  ---- |  1.08  1.06

Classical evaluation: 0.87 (white side)

NNUE evaluation:      2.63 (white side)

Final evaluation:     0.87 (white side)

d

 +---+---+---+---+---+---+---+---+
 |   | B |   | b |   | k |   |   | 8
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   | p | 7
 +---+---+---+---+---+---+---+---+
 |   | p | p | N |   |   |   | R | 6
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   | P | p | b | 5
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   |   | 4
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   |   | 3
 +---+---+---+---+---+---+---+---+
 | P | P |   |   | r | K | P | P | 2
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   |   | 1
 +---+---+---+---+---+---+---+---+
   a   b   c   d   e   f   g   h

Fen: 1B1b1k2/7p/1ppN3R/5Ppb/8/8/PP2rKPP/8 b - - 0 1
Key: 2A187A09FD59B8AF
Checkers:
go depth 1
info string NNUE evaluation using eval.bin enabled
Segmentation fault

and this should be news ....?

[bob]

Someone recently wasted their time writing up 20 paragraphs showing how they could "exploit" Stockfish into crashing....

If you send a chess engine garbage, you should expect garbage. If you don't know it is garbage, then maybe a GUI should be doing it for you.

The above has been affirmed many times in Stockfish PRs, where users come saying they have found a "bug"

have to add here... look at any software engineering textbook and find a discussion of "robustness". Hint: bad input should NEVER cause an engine to crash, produce garbage or cause demons to fly out of your nose. Why allow it? Senseless to me.