Potential for integer overflow and crash in SortingRendererClass::Flush_Sorting_Pool

[Caball009]

With an insane number of units or effects, it's possible to get the polygon count to overflow a 16 bit limit and subsequently crash the game. This needs fixing, but the scenario is somewhat contrived because the game is so slow it's completely unplayable by the time it overflows and crashes. There's an assertion that'll get triggered in debug mode.

GeneralsGameCode/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp

Lines 527 to 542 in fc36efc

	unsigned polygonAllocCount = overlapping_polygon_count;
	if ((unsigned)(DynamicIBAccessClass::Get_Default_Index_Count()/3) < DEFAULT_SORTING_POLY_COUNT)
	polygonAllocCount = DEFAULT_SORTING_POLY_COUNT; //make sure that we force the DX8 index buffer to maximum size
	if (overlapping_polygon_count > polygonAllocCount)
	polygonAllocCount = overlapping_polygon_count;
	WWASSERT(DEFAULT_SORTING_POLY_COUNT <= 1 || polygonAllocCount <= DEFAULT_SORTING_POLY_COUNT);
	DynamicIBAccessClass dyn_ib_access(BUFFER_TYPE_DYNAMIC_DX8,polygonAllocCount*3);
	{
	DynamicIBAccessClass::WriteLockClass lock(&dyn_ib_access);
	ShortVectorIStruct* sorted_polygon_index_array=(ShortVectorIStruct*)lock.Get_Index_Array();
	for (unsigned a=0;a<overlapping_polygon_count;++a) {
	sorted_polygon_index_array[a]=tis[a].tri;
	}
	}

A couple of issues here:

1. polygonAllocCount*3 (L534) is cast to an unsigned short by DynamicIBAccessClass's constructor, so it must below 2 ^ 16 considering it's used later for the iteration.
2. a<overlapping_polygon_count (L539) should be a<polygonAllocCount.

[Caball009]

For easier reproduction:

1. Set the particle count to something ridiculous like 50000.
2. Drop 3 adjacent or overlapping cluster mines.
3. Launch a nuclear missile in the center, and keep the mines on screen.
4. The game crashes because of the effects of the exploding mines.

This scenario is slightly different than what's described in the first post and less contrived, because the only outlier here is the high particle count. The point where the max particle count becomes an issue may differ per GPU and / or GPU driver.

Live crash caused by this bug:

https://www.youtube.com/live/Q34Zi8lr6OY?t=16396s

[Mauller]

It looks like these 16bit index buffers are a hold over from directx7, DX8 supports 32bit index buffers so we can just try changing all unsigned shorts to unsigned ints.

[Caball009]

Debug build crashes here already with the above reproduction:

GeneralsGameCode/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp

Lines 438 to 441 in fc36efc

	// If you have a crash in here and "dest_verts" points to illegal memory area,
	// it is because D3D is in illegal state, and the only known cure is rebooting.
	// This illegal state is usually caused by Quake3-engine powered games such as MOHAA.
	memcpy(dest_verts, src_verts, sizeof(VertexFormatXYZNDUV2)*state->vertex_count);

Despite EA's comment this is more likely to be caused by a 16 bit integer overflow here as well.

[xezon]

Literally munkee fudge everywhere

[stephanmeesters]

#2783 reduced the risk of crashing by allowing around ~50% more polygons (from ~22k polys to ~32k).

This does not fully fix the issue though when bumping the particle limit, because the sorting renderer still uses 16-bit indices into one combined vertex buffer. A full fix would need batching the vertex buffer too, or adding 32-bit index buffers for this path. With the upcoming BGFX renderer this should become much easier to solve properly.