#86 Crash on during evaluation #91
Conversation
| @@ -1124,7 +1123,6 @@ void _droidzebra_throw_engine_error(JNIEnv* env, const char* msg) | |||
|
|
|||
| void _droidzebra_undo_stack_push(int val) | |||
| { | |||
| assert(s_undo_stack_pointer<64); | |||
There was a problem hiding this comment.
s_undo_stack size is 64, so if this assert is triggered here, it's definitely a bug. If we remove it, it will just silently overflow the buffer, there's no logic to grow that buffer here.
There was a problem hiding this comment.
I think, that the stack size is too small, without the asserts everything works perfectly. But I am just trying to find out how often this is increased.
There was a problem hiding this comment.
without the asserts everything works perfectly
This can be very misleading - it just means that the value is written somewhere in the memory and the program has no problems writing to it - that doesn't mean that the memory is valid. A little change in the code can move the array somewhere else in memory and cause a bug, either a crash if it's on page boundary, or something more problematic by overriding unrelated data.
I think it makes sense to increase the stack size, but I'd definitely not remove the assert, because it guards a real bug.
There was a problem hiding this comment.
Stack size is now 120
There are 60 White and 60 Black moves maximum (each move can be a pass)
There was a problem hiding this comment.
To clarify - it's possible that the assert went away only because you've also added the undoall text and that changed the layout of the code, it may not even be related to that assert.
That would also make sense, because asserts are usually completely removed in production builds. I'm not sure if we do that here, you can test this by adding assert(false) somewhere and run a production build.
There was a problem hiding this comment.
I deleted the assert. I reintrocuded it.
64 worked until 4 Passes were made. After that the assert always failed.
I think the asserts were disabled in production or before some c refactoring. And the Application crashed when doing redo. In my current version it crashed at undo at the assert
There was a problem hiding this comment.
Stack size is now 120 There are 60 White and 60 Black moves maximum (each move can be a pass)
That sounds plausible, good catch. Does the game from the bug report have a lot of passes?
There was a problem hiding this comment.
like 7 or 8, Lots of passes.
No description provided.