lichess-org / mobile

Lichess mobile app v2
GNU General Public License v3.0
1.36k stars 196 forks source link

[bug] app crash when you add any pieces on the board #1005

Closed olivertzeng closed 1 month ago

olivertzeng commented 2 months ago

Latest lichess beta on iOS

tom-anders commented 2 months ago

Tried reproducing on my Android phone, but even adding a pawn to every single square did not crash. Is this 100% reproducible for you?

olivertzeng commented 2 months ago

Tried reproducing on my Android phone, but even adding a pawn to every single square did not crash. Is this 100% reproducible for you?

Yes let me record this

olivertzeng commented 2 months ago

Tried reproducing on my Android phone, but even adding a pawn to every single square did not crash. Is this 100% reproducible for you?

https://imgur.com/a/BCx51zX

tom-anders commented 2 months ago

Ah, so it only crashes once you enter the analysis screen, got it! Then I can reproduce as well

tom-anders commented 2 months ago

Most minimal reproduction steps:

1) Open Board Editor 2) Add an additional white pawn to e4 (or any other square apparently) 3) Open Analysis board

It looks like stockfish crashes (see stacktrace below). Interestingly, the same reproduction steps also crash the old lichess app. Not sure if this is actually a stockfish issue, or if we're giving stockfish wrong/unexpected inputs. @veloce any ideas?

I/flutter ( 3443): Engine state: EngineState.loading
I/flutter ( 3443): Engine state: EngineState.computing
F/libc    ( 3443): Fatal signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x748d40340c00 in tid 3813 (DartWorker), pid 3443 (.mobileV2.debug)
*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
Build fingerprint: 'google/sdk_gphone64_x86_64/emu64xa:15/AP31.240617.003/12088229:user/release-keys'
Revision: '0'
ABI: 'x86_64'
Timestamp: 2024-09-10 08:32:58.701874929+0200
Process uptime: 17s
Cmdline: org.lichess.mobileV2.debug
pid: 3443, tid: 3813, name: DartWorker  >>> org.lichess.mobileV2.debug <<<
uid: 10201
signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x0000748d40340c00
    rax 0000748d15200000  rbx 0000748d148308c0  rcx 00000000158a0000  rdx 0000000000006fcc
    r8  0000748d1483a300  r9  0000748d15200c00  r10 00000000ffffffb3  r11 0000748fe72485f8
    r12 0000748d213eb0c8  r13 0000748d148308c0  r14 0000748d148308c0  r15 0000748d148308c0
    rdi 0000748d14828350  rsi 00000000ffffffff
    rbp 0000748d148283f0  rsp 0000748d14828310  rip 0000748d1edcede9
11 total frames
backtrace:
      #00 pc 0000000002751de9  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (void Stockfish::Eval::NNUE::FeatureTransformer::update_accumulator_refresh<(Stockfish::Color)0>(Stockfish::Position const&) const+329) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #01 pc 0000000002752a09  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (void Stockfish::Eval::NNUE::FeatureTransformer::update_accumulator<(Stockfish::Color)0>(Stockfish::Position const&) const+249) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #02 pc 0000000002750c17  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (Stockfish::Eval::NNUE::FeatureTransformer::transform(Stockfish::Position const&, unsigned char*, int) const+39) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #03 pc 000000000274f189  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (Stockfish::Eval::NNUE::evaluate(Stockfish::Position const&, bool, int*)+185) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #04 pc 0000000002732a73  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (Stockfish::Eval::evaluate(Stockfish::Position const&)+275) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #05 pc 0000000002763614  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #06 pc 0000000002761433  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (Stockfish::Thread::search()+2819) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #07 pc 00000000027918a6  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (Stockfish::Thread::idle_loop()+310) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #08 pc 0000000002796519  /data/app/~~LucKNvJJBZyjEEeoNv9fFA==/org.lichess.mobileV2.debug-5P87t8riTcwJ2OsXXwl_Wg==/base.apk!libstockfish.so (offset 0x7d66000) (void* Stockfish::start_routine<Stockfish::Thread, std::__ndk1::pair<Stockfish::Thread*, void (Stockfish::Thread::*)()>>(void*)+105) (BuildId: ef8ea4ea7db75e97494535e846c46975098d6654)
      #09 pc 000000000006d62a  /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+58) (BuildId: eb58b4d427279994f00c0e1818477e4f)
      #10 pc 0000000000060348  /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+56) (BuildId: eb58b4d427279994f00c0e1818477e4f)
veloce commented 2 months ago

It is possible that stockfish would fail with weird positions. We could add more safeguards. It is also possible that these exists on the website, so we could reuse the logic.

olivertzeng commented 2 months ago

It is possible that stockfish would fail with weird positions. We could add more safeguards. It is also possible that these exists on the website, so we could reuse the logic.

圖片

tom-anders commented 2 months ago

@olivertzeng I think that is actually a unrelated issue caused by the recent stockfish 17 integration, I same a similar issue o. For me https://lichess.org/analysis/rnbqkbnr/pppppppp/8/8/8/4P3/PPPPPPPP/RNBQKBNR_w_KQkq_-_0_1?color=white works on web (both stockfish 16 and 17 actually)

So to me it looks like stockfish should handle this position just fine

olivertzeng commented 2 months ago

Update: this issue also produces if you put an extra knight on the board as well

tom-anders commented 2 months ago

Update: this issue also produces if you put an extra knight on the board as well

And the extra knight crashes the old app as well.

olivertzeng commented 2 months ago

Update: this issue also produces if you put an extra knight on the board as well

And the extra knight crashes the old app as well.

Nice find!

TBestLittleHelper commented 2 months ago

Stockfish's "limited" scope is real chess positions. This means that if you have more then 32 pices stockfish can crash. I think, you can achive a bit more support by using fary-stockfish ( variants ). But there would still be positions that can crash that too.

Probably, it would be smart to have a "error" screen similar to the web. And maybe allow using fariy stockfish ( at least if fairy stockfish will be added anyway, for variants support. Not sure if dart-stockfish is fariy based or not).

Since many illegal positions still work fine, at least I think the best approach is the error screen that the web shows.

tom-anders commented 2 months ago

Stockfish's "limited" scope is real chess positions. This means that if you have more then 32 pices stockfish can crash. I think, you can achive a bit more support by using fary-stockfish ( variants ). But there would still be positions that can crash that too.

Probably, it would be smart to have a "error" screen similar to the web. And maybe allow using fariy stockfish ( at least if fairy stockfish will be added anyway, for variants support. Not sure if dart-stockfish is fariy based or not).

Since many illegal positions still work fine, at least I think the best approach is the error screen that the web shows.

The website does not crash on these positions, so probably they’re using fairy Stockfish for these positions as well..?

We already disable the analysis button for other illegal positions, and AFAIK we don’t have variant support yet (but I think there’s already an issue related to fairy stockfish support). So for now, we should probably simply disable the analysis button if there are more than 32 pieces on the board

tom-anders commented 2 months ago

(Not sure how easy it would be to add an Error message if Stockfish segfaults, @veloce probably has better insight into how the FFI works and if we can even stop it from crashing the whole app)

veloce commented 2 months ago

I can investigate that. But we should also disable stockfish evaluation if the position contains more than 32 pieces.

TBestLittleHelper commented 2 months ago

Maybe I should have been a bit more spesific btw. It's not really as simple as more then 32 pieces. It's " fens with illegal positions " that get's us. It's just that in practice, from what I see, people usually run into a crash when dragging lots of pices onto the board editior. But other weird fens from a board editor can crash it too. ( ie. you can have only a king and 15 pawns versus a king and presumebly risk a crash ). I think you avoid 99% of the crashes with the 32 rule through.

veloce commented 1 month ago

Closing as we now check the number of pieces.