Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDL_CreateRenderer segfaults on windows with dummy videodriver after updating to SDL 2.24.0 #6356

Closed
ankith26 opened this issue Oct 8, 2022 · 6 comments

Comments

@ankith26
Copy link
Contributor

ankith26 commented Oct 8, 2022

Till 2.0.22 SDL_CreateRenderer didn't segfault in the same configuration. This is a bug noticed downstream in the pygame unit tests running on CI (hence the dummy driver)

This bug is not reproducible locally when using a non-dummy driver, or using the dummy driver and setting SDL_RENDER_DRIVER to software.

(I know this is a bit of a weird usecase, considering that it does not make sense to be using a renderer when there's no actual window, but I just reported the bug because it shouldn't segfault)

@ankith26
Copy link
Contributor Author

ankith26 commented Oct 8, 2022

I tested this locally by compiling debug SDL binaries. This is fixed on main (probably) as a side affect of #6112 (IDK whether it was indented or not) but is still broken on the 2.24.x branch at the time of writing of this comment. This regression (probably) came from #5778

I'd appreciate it if a fix (if it's not much of a hassle) was backported to the 2.24.x branch so we can get to using that earlier.

image

Anyways, here is the stack trace (using VSC). The internal function at the root of this segfault was entirely replaced in that PR

@sezero
Copy link
Contributor

sezero commented Oct 8, 2022

So, the following for release-2.24.x branch, then?

diff --git a/src/video/windows/SDL_windowswindow.c b/src/video/windows/SDL_windowswindow.c
index eb536bb..3fd3290 100644
--- a/src/video/windows/SDL_windowswindow.c
+++ b/src/video/windows/SDL_windowswindow.c
@@ -1408,10 +1408,16 @@ WIN_SetWindowOpacity(_THIS, SDL_Window * window, float opacity)
 void
 WIN_GetDrawableSize(const SDL_Window *window, int *w, int *h)
 {
-    const SDL_WindowData *data = ((SDL_WindowData *)window->driverdata);
-    HWND hwnd = data->hwnd;
+    HWND hwnd;
     RECT rect;
 
+    if (!window || !(window->driverdata)) {
+        *w = 0;
+        *h = 0;
+        return;
+    }
+
+    hwnd = ((SDL_WindowData *)window->driverdata)->hwnd;
     if (GetClientRect(hwnd, &rect)) {
         *w = rect.right;
         *h = rect.bottom;

@ankith26
Copy link
Contributor Author

ankith26 commented Oct 8, 2022

Yes, I think this should do the trick. I'm not familiar with SDL internals so I can't say for sure whether this is the only thing that needs updating or whether there are more places like this that need fixing for this issue

@sezero
Copy link
Contributor

sezero commented Oct 8, 2022

@slouken: Is the patch above good for release-2.24.x branch?
Can there be any other places that need touching?

@slouken
Copy link
Collaborator

slouken commented Oct 8, 2022

Yes, that's fine. I don't know of any other places that would need touching.

@sezero
Copy link
Contributor

sezero commented Oct 8, 2022

OK, the patch is in the release-2.24.x branch.

@sezero sezero closed this as completed Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants