We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 1116 - [patch] Fix segfault in SDL_CreateWindowTexture()
Summary: [patch] Fix segfault in SDL_CreateWindowTexture()
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: HG 2.0
Hardware: x86_64 Linux
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-12 12:28 UTC by Martin Decky
Modified: 2011-02-13 14:05 UTC (History)
0 users

See Also:

Attachments
Proposed patch (1.15 KB, patch)
2011-02-12 12:28 UTC, Martin Decky 		Details | Diff
Fix local variable initialization and scope (1.36 KB, patch)
2011-02-13 04:00 UTC, Martin Decky 		Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Decky 2011-02-12 12:28:00 UTC 
Running QEMU with the latest SDL 1.3 and specifically with the OpenGL renderer causes a segmentation fault with the following stack trace:

#0  0x00007ffff5a6350c in free () from /lib64/libc.so.6
#1  0x00007ffff65e3ffc in SDL_CreateWindowTexture (_this=<value optimized out>, window=0x1346410, format=0x7fffffffddfc, pixels=0x7fffffffddd8, pitch=0x7fffffffddf8) at src/video/SDL_video.c:266
#2  0x00007ffff65e4b01 in SDL_CreateWindowFramebuffer (window=0x1346410) at src/video/SDL_video.c:1602
#3                        SDL_GetWindowSurface (window=0x1346410) at src/video/SDL_video.c:1623
#4  0x00007ffff6570216 in SDL_SetVideoMode (width=640, height=480, bpp=32, flags=150994945) at src/SDL_compat.c:552
#5  0x000000000049c241 in do_sdl_resize (new_width=<value optimized out>, new_height=<value optimized out>, bpp=32) at ui/sdl.c:112
#6  0x000000000049cca6 in sdl_create_displaysurface (width=640, height=480) at ui/sdl.c:201
#7  0x0000000000461aac in register_displayallocator (ds=0x1174e70, da=0x1346d30) at console.c:1380
#8  0x000000000049da45 in sdl_display_init (ds=0x1174e70, full_screen=0, no_frame=<value optimized out>) at ui/sdl.c:865
#9  0x000000000055b162 in main (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>) at /home/martin/qemu/vl.c:3054

The root cause of the segmentation fault is roughly following:

1. In the SDL_SetVideoMode() an initial X11 window is created.

2. Later in SDL_CreateWindowTexture() we first allocate the SDL_WindowTextureData structure and set it as SDL_WINDOWTEXTUREDATA.

3. Still in SDL_CreateWindowTexture() we call SDL_CreateRenderer() which eventually causes the X11 to be recreated in SDL_RecreateWindow(). However, in SDL_RecreateWindow() the original SDL_WINDOWTEXTUREDATA is removed and the SDL_WindowTextureData structure is deallocated (see "_this->DestroyWindowFramebuffer(_this, window);" on line 1231 in SDL_video.c).

4. Finally as we return back to SDL_CreateWindowTexture(), the local "data" pointer points to freed memory and the segfaults happens soon while trying to deallocate data->texture in SDL_DestroyTexture().


The attached patch fixes the problem by explicitly refreshing the pointer to the SDL_WindowTextureData structure after returning from SDL_CreateRenderer().
Comment 1 Martin Decky 2011-02-12 12:28:45 UTC 
Created attachment 566 [details]
Proposed patch
Comment 2 Sam Lantinga 2011-02-12 17:53:30 UTC 
I fixed it slightly differently in this patch:
http://hg.libsdl.org/SDL/rev/bad04e4710f6

Thanks!
Comment 3 Martin Decky 2011-02-13 03:57:19 UTC 
OK. But then you should probably add at least the patch which I am going to attach in a minute to make it really work.
Comment 4 Martin Decky 2011-02-13 04:00:37 UTC 
Created attachment 570 [details]
Fix local variable initialization and scope

(Oops, sorry, originally attached to a wrong bug ..)

Rationale: The local variable "renderer" needs to be initialized to NULL,
otherwise the code in the if (!data) { ... } branch might fail to actually
create a renderer (note the second for-loop guarded by (!renderer)).

Secondly, move the "renderer" variable scope into the branch to make sure that
the code actually uses data->renderer outside the branch (which holds the only
provably correct value in both cases).
Comment 5 Sam Lantinga 2011-02-13 14:05:54 UTC 
Got it, thanks!