Patch: Crash/access vio when frame_size is larger than source_rect

Celdecea · 2009-03-21 17:51:09

As of revision 18, specifying a frame_size larger than the source_rect in a brick file (sprite block) causes an access violation to occur. When compiled under Visual Studio .NET 2003, the error looks like this:

MSVC wrote:

Unhandled exception at 0x0042436e in opensonic.exe: 0xC0000005: Access violation reading location 0xfdfdfe01.

Call Stack wrote:

opensonic.exe!update_level_size() Line 2145 + 0x12 C
opensonic.exe!level_load(char * filepath=0x0048e170) Line 391 C
opensonic.exe!level_init() Line 570 + 0xa C
opensonic.exe!scenestack_push(scene_t * scn=0x0cf492a8) Line 112 + 0x7 C
opensonic.exe!_mangled_main(int argc=4, char * * argv=0x01192438) Line 186 + 0xb C
*Impertinent*

The two lines of the brick file that I can use to reproduce this are:

source_rect     70 10 128 128
frame_size      128 129

In src/sprite.c around line 222 a couple min() calls supposedly fix the frame size and it appears to work; dumping the locals in src/level.c from update_level_size() gives me the following:

level.c.18.locals.png

But the image variable (p->data->brick_ref->image) has no usable data. The only inconsistency that I see in src/sprite.c is the frame_count variable. This variable is set for the purposes of checking for valid number of frames (with advisory message) but then the values of frame_w and frame_h are altered without altering the frame_count again. Further on, frame_count is used in at least a mallocx() call.

There are several obvious ways to fix this. The simplest I can think of (though not a very good way) is to just reverse the ordering of the min() calls and the frame_count as with this patch. However, this takes away the benefit of the warning message. The second method is to set the frame_count variable a second time after the check. A possible third method is enable a dirty exit of the app like we did here, and just do a specific check for out-of-bounds frame sizes.

I am sure there are even better ways, but I'm tired this week.

EDIT: After further inspection, I don't think I have found the problem. Still playing with it...

Last edited by Celdecea (2009-03-22 02:34:01)

Alexandre · 2009-03-22 14:29:34

This should fix it (src/sprite.c around line 215 - SVN revision 18):

(...)
        else if(strcmp(identifier, "}") == 0) {
            spriteinfo_t *sprite;

            if(frame_w>rect_w || frame_h>rect_h) {
                logfile_message("Warning: read_sprite() incompatible source_rect (%d,%d) x frame_size (%d,%d)", rect_w, rect_h, frame_w, frame_h);
                frame_w = min(frame_w, rect_w);
                frame_h = min(frame_h, rect_h);
                logfile_message("frame_size is now (%d,%d)", frame_w, frame_h);
            }

            if(rect_w%frame_w > 0 || rect_h%frame_h > 0) {
                logfile_message("Warning: read_sprite() incompatible source_rect (%d,%d) x frame_size (%d,%d)", rect_w, rect_h, frame_w, frame_h);
                rect_w = (rect_w%frame_w > 0) ? (rect_w - rect_w%frame_w + frame_w) : rect_w;
                rect_h = (rect_h%frame_h > 0) ? (rect_h - rect_h%frame_h + frame_h) : rect_h;
                logfile_message("source_rect is now (%d,%d)", rect_w, rect_h);
            }

            frame_count = (rect_w/frame_w) * (rect_h/frame_h);

            sprite = spriteinfo_create();
            strcpy(sprite->source_file, source_file);
            sprite->rect_x = rect_x;
(...)

Last edited by Alexandre (2009-03-22 22:10:11)

Open Surge Forum

Announcement

#1 2009-03-21 17:51:09

Patch: Crash/access vio when frame_size is larger than source_rect

#2 2009-03-22 14:29:34

Re: Patch: Crash/access vio when frame_size is larger than source_rect

Board footer