Internal assert is hit, which looks like an integer overflow bug

[simonvanbernem]

Hi,

I have been playtesting my game a little, and have observed crashes, which originate from an assert in solver.c:

box2d/src/solver.c

Line 221 in c05c487

B2_ASSERT( startIndex <= endIndex );

It crashes with this callstack:

spaced.exe!b2IntegratePositionsTask(int startIndex, int endIndex, b2StepContext * context) Line 221 (c:\Workspaces\spaced\source\modules\box2d\src\solver.c:221)
spaced.exe!b2ExecuteBlock(b2SolverStage * stage, b2StepContext * context, b2SolverBlock * block, int workerIndex) Line 901 (c:\Workspaces\spaced\source\modules\box2d\src\solver.c:901)
spaced.exe!b2ExecuteStage(b2SolverStage * stage, b2StepContext * context, int previousSyncIndex, int syncIndex, int workerIndex) Line 965 (c:\Workspaces\spaced\source\modules\box2d\src\solver.c:965)
spaced.exe!b2ExecuteMainStage(b2SolverStage * stage, b2StepContext * context, unsigned int syncBits) Line 1025 (c:\Workspaces\spaced\source\modules\box2d\src\solver.c:1025)
spaced.exe!b2SolverTask(int startIndex, int endIndex, unsigned int threadIndexIgnore, void * taskContext) Line 1156 (c:\Workspaces\spaced\source\modules\box2d\src\solver.c:1156)
spaced.exe!b2DefaultAddTaskFcn(void(*)(int, int, unsigned int, void *) task, int count, int minRange, void * taskContext, void * userContext) Line 80 (c:\Workspaces\spaced\source\modules\box2d\src\physics_world.c:80)
spaced.exe!b2Solve(b2World * world, b2StepContext * stepContext) Line 1806 (c:\Workspaces\spaced\source\modules\box2d\src\solver.c:1806)
spaced.exe!b2World_Step(b2WorldId worldId, float timeStep, int subStepCount) Line 799
...

In the crash, endIndex has the value -32768, which looks like a wrapped int16. At solver.c:965 in b2ExecuteStage, blocks contains a single block with startIndex = 0 and count = -32768.

When the crash happens, the game has created and destroyed many thousands of bodies and shapes (they are particles simulating the exhaust of a rocket engine), so it seems plausible that some int16 counter got incremented per shape/body created/destroyed and wrapped around this way.

Additionally, I had a bug where I didn't destroy the particle bodies properly. At the time of the crash, there are 6100 shapes but 131079 bodies (most of them dynamic bodies with no shape).

Let me know if this is enough information for a fix, or if I should dig deeper. I have only reproduced the bug with a release build, which makes stuff somewhat awkward to debug, but if necessary, I could probably get it with a debug build and provide a minidump and so on.

[simonvanbernem]

Absolute mad props btw for creating a physics engine where I simply didn't notice that I had 131k bodies around, because it was still fast :D.

[erincatto]

Might be this:

block->count = (int16_t)bodyBlockSize;

I'll probably change this to uint16_t and add some earlier asserts. In this case it sounds like the assert caught a problem on the app side.

[simonvanbernem]

Yeah, seems to makes sense with this:

// A block is a range of tasks, a start index and count as a sub-array.
// Each worker receives at most M blocks of work. The workers may receive less blocks if there is not sufficient work.
// Each block of work has a minimum number of elements (block size). This in turn may limit the number of blocks.
// If there are many elements then the block size is increased so there are still at most M blocks of work per worker.
// M is a tunable number that has two goals:
// 1. keep M small to reduce overhead
// 2. keep M large enough for other workers to be able to steal work
// The block size is a power of two to make math efficient.

Maybe the blocksize got increased more and more to stay within maxBlockCount with so many bodies areound. I'm not sure what the tradeoff is here, but sound like the tunable M should have some value it tries to achieve, but then go up if needed, to not exceed int16_max block size?

[simonvanbernem]

If there was an assert that checked for too many bodies, which could be calculated using M and the data type capacity, that would be helpful!

[erincatto]

It is not the number of bodies, it is the number of awake dynamic bodies that is the limitation.

[erincatto]

I've changed the block count to use a uint16_t. I think this the most I will do for now. It is important for b2SolverBlock to be small. I have never hit this limit myself and I've pushed the engine to its limits.