Global Ring Count overruns points buffer

I observed intermittent crashes related to m_posDel while running the mapping undo test and bombarding Whorld with random MIDI input. It looked like a cascading delete issue but this was misdirection. Closer inspection found garbage in all CWhorldThread members above the point array (m_aPt), while members below m_aPt were intact.

This clearly indicated that the draw loop was not respecting the the point array size (MAX_SIDES). And sure enough, a search for MAX_SIDES revealed that max sides clamping is done at the start of AddRings, BEFORE the global nPolySides is added. Thus if the ring count is at or near maximum, AND curves are enabled, AND Fill is enabled, AND the global ring count is one or more, the point array overflows, generally causing havoc, and specifically overwriting m_posDel which will certainly cause Whorld to crash on the next cascading delete, due to an invalid list position.

Demo patch: points buffer overrun set Global Ring Sides 51.whp

More succinctly, the points buffer is overrun when:

1. Ring Sides = 50
2. Curves = true
3. Fill = true
4. Ring count >= 2
5. Global Ring Sides >= 1

OR

1. Ring Sides = 50
2. Curves = true
3. Global Ring Sides >= 51

The bug is more probable when Fill is enabled, because in that case the points buffer must accommodate TWO rings rather than only one. And in the Fill case, it's the memcpy that overruns, while copying the previous ring's points to the end of the points buffer.

The following assertion just before the memcpy catches the overrun:

ASSERT((nPoints + nPrevPoints) * sizeof(D2D_POINT_2F) < sizeof(m_aPt));

Adding a canary member variable after the points buffer and checking it after the ring iteration also catches the overrun:

D2D_POINT_2F m_aPt[MAX_POINTS * 2]; // enough for two rings [OR NOT!]
UINT_PTR m_nCanary; // guard band for detecting buffer overrun
m_nCanary = 0x1234abcd4321dcba; // init in WhorldThread ctor
ASSERT(m_nCanary == 0x1234abcd4321dcba); // detects overrun

This bug was introduced on January 15, 2008, with the following comment: "add globals for line width and poly sides".

The fix is trivial. In OnDraw, simply replace this line:

nSides = max(nSides, 1);

with this one:

nSides = CLAMP(nSides, 1, MAX_SIDES);

Prior to the 2008 revision, MAX_SIDES was only enforced in AddRings because there was no need to enforce it per ring, and doing so anyway would have wasted precious time. But implementing global Ring Sides changed the situation: it became necessary to enforce MAX_SIDES inside the ring-drawing loop, because nSides is the sum of the ring's side count and global Ring Sides, and latter can change at any time.

nSides = ring.nSides + m_globRing.nPolySides; // the source of the bug
nSides = max(nSides, 1); // no longer enough, must enforce MAX_SIDES too!