This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Exhibits bug in SDL_FillRect. (fred.c, text/plain, 2015-02-22 23:44:46 +0000, 710 bytes)
• fix for bug 2868 (SDL_FillRect bug) (SDL_bug_2868-FillRect_crash.c, text/plain, 2015-02-23 01:37:48 +0000, 341 bytes)
• simplified test program for SDL_FillRect() crash (SDL_bug_2868-FillRect_crash.c, text/x-csrc, 2015-02-23 01:38:31 +0000, 341 bytes)
• fix for bug 2868 (SDL_FillRect bug) (SDL_bug_2868-fixed.patch, text/plain, 2015-02-23 03:08:41 +0000, 840 bytes)

Reported in version: 2.0.3
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2015-02-05 17:05:31 +0000, skaller wrote:

I have code which is doing some drawing using at least SDL_RenderDrawLine on the surface of a window. If the window is resized so it it is too small, the program crashes, but not every time.

On thinking about this it would seem to be a serious design bug in SDL.
SDL deletes the surface of a window when the window is resized.

It appears this happens asynchronously, and can delete the surface whilst
it is in use. SDL_MUSTLOCK is NOT being set (so I'm not locking it).

This does not seem to bother bliting or filling, only line drawing,
and only when the line ends up being drawn off the edge of the window.

This happened with 2.0.0 so I upgraded to 2.0.3 and it seems to happen less
often but it still happens.

On 2015-02-22 23:44:46 +0000, skaller wrote:

Created attachment 2041
Exhibits bug in SDL_FillRect.

Definite bug in SDL_FillRect.c exhibited by this attachment.

On 2015-02-22 23:49:57 +0000, skaller wrote:

Here is the fix:

AFTER Line 255 of SDL_FillRect.c add this line:

if(SDL_RectEmpty(rect)) return 1;

This prevents negative heights and widths or out of bounds starting
pixel for fill crashing the subsequent calculations.

Please also examine ALL similar code: blits, line drawing
etc. Sorry, I cannot build SDL from source (OSX 10.6.8).

This patch has been tested on Linux and prevents the test
code from crashing.

I would really like a fixed up SDL2.dmg for OSX since I cannot
compile it myself.

On 2015-02-23 01:37:48 +0000, David Ludwig wrote:

Created attachment 2042
fix for bug 2868 (SDL_FillRect bug)

On 2015-02-23 01:38:31 +0000, David Ludwig wrote:

Created attachment 2043
simplified test program for SDL_FillRect() crash

On 2015-02-23 01:38:43 +0000, David Ludwig wrote:

Hi Sam, Ryan, etc.

I'm able to reproduce this bug on iOS + Win32. My guess, from looking at SDL's code, is that it occurs on other platforms as well.

In summary, I think the bug can be condensed to the following: SDL_FillRect() can crash if the surface's clip-rect is completely out-of-bounds

To reproduce:

1. set an SDL_Surface's clip-rect to something completely outside the bounds of the surface (via SDL_SetClipRect()).
2. call SDL_FillRect() on the surface, telling it to fill the entire rect (by passing in NULL as the 'rect/2nd-param).
3. crash!

John's suggestion for a patch looks good. I've tested it, and encoded it into a patch. It takes the following statement from SDL_SetClipRect's documentation:

'If the clip rectangle doesn't intersect the surface, the function will return SDL_FALSE and blits will be completely clipped.'

... and applies it to SDL_FillRect(). I.e. if and when a surface's clip-rect is out-of-bounds, 'SDL_FillRect(surface, NULL, color)' does nothing.

If you all like, I'd be happy to push this fix out to Mercurial.

In addition to the patch, I've attached a simplified test case, which should reproduce on any platform.

Cheers!
-- David L.

On 2015-02-23 03:08:41 +0000, David Ludwig wrote:

Created attachment 2044
fix for bug 2868 (SDL_FillRect bug)

Doh, I posted the simplified test program twice. Here's the patch!

On 2015-02-24 04:33:13 +0000, Ryan C. Gordon wrote:

Looks good. David, go ahead and push this and resolve the bug.

Thanks!

--ryan.

On 2015-02-24 04:44:46 +0000, David Ludwig wrote:

Fixed via https://hg.libsdl.org/SDL/rev/b577c4753421