Fish Fillets - Next Generation: BadWindow in X_ChangeProperty (on GNOME + Xwayland)

[smcv]

Prerequisites:

• Debian testing (Debian 12 alpha)
• Video: GNOME 43 in Wayland mode (with Mesa 22.2.0 on AMD Vega, if it matters)
• Audio: Pipewire 0.3.59, with pipewire-pulse emulating PulseAudio
• apt install fillets-ng (Debian package version 1.0.1-4+b1)
• Some relevant libraries:
  • libsdl1.2debian (real SDL 1.2, with an odd package name for historical reasons) version 1.2.15+dfsg2-8
  • libsdl1.2-compat commit eba13ef, locally-built
  • libsdl2-2.0-0 version 2.24.1+dfsg-1

To reproduce:

• fillets
• LD_LIBRARY_PATH=.../sdl12-compat/_build fillets

Expected result: both work

Actual result: with sdl12-compat, it fails with:

X Error of failed request:  BadWindow (invalid Window parameter)
  Major opcode of failed request:  18 (X_ChangeProperty)
  Resource id in failed request:  0x0
  Serial number of failed request:  148
  Current serial number in output stream:  164

Workaround: SDL_VIDEODRIVER=wayland works.

[icculus]

This is calling SDL_GetWMInfo before SDL_SetVideoMode and that's upsetting things; it works if I return 0; from SDL_GetWMInfo immediately. Checking why.

[icculus]

This is what's triggering it in SDL2's x11 code:

https://github.com/libsdl-org/SDL/blob/e714d4d72437fb76a7d2d1b3e5e745032c840e54/src/video/x11/SDL_x11window.c#L260-L267

#ifdef X_HAVE_UTF8_STRING
    if (SDL_X11_HAVE_UTF8 && videodata->im) {
        data->ic =
            X11_XCreateIC(videodata->im, XNClientWindow, w, XNFocusWindow, w,
                       XNInputStyle, XIMPreeditNothing | XIMStatusNothing,
                       NULL);
    }
#endif

I'm not sure why, and this little SDL2 program doesn't trigger it:

#include "SDL.h"
#include "SDL_syswm.h"

int main(void)
{
    SDL_Window *w;
    SDL_SysWMinfo info20;
    SDL_Init(SDL_INIT_VIDEO);
    w = SDL_CreateWindow("Hello SDL", 100, 100, 100, 100, 0);
    SDL_zero(info20);
    SDL_VERSION(&info20.version);
    SDL_GetWindowWMInfo(w, &info20);
    SDL_DestroyWindow(w);
    w = SDL_CreateWindow("Hello SDL", 100, 100, 100, 100, 0);
    SDL_DestroyWindow(w);
    SDL_Quit();
    return 0;
}

[icculus]

Thanks to https://github.com/libsdl-org/SDL/commit/17322e2be649eefaf6a02e2fcaefc444722ccfbb we can see more exactly where SDL2 is getting called into.

I still don't know what part of this is upsetting X11, but there's lots of stuff going on in here that my non-triggering reproduction case isn't doing, so that's more to research still.

Also, it looks like it's not crashing with the X error, but cleanly shutting down after this cases SDL_CreateWindow() to fail, but I haven't looked more closely yet.

icculus@lucha:~/projects/sdl12-compat/cmake-build$ SDL_DYNAPI_LOG_CALLS=1 SDL12COMPAT_DEBUG_LOGGING=1 LD_LIBRARY_PATH=/home/icculus/projects/sdl12-compat/cmake-build gdb fillets
GNU gdb (Ubuntu 12.0.90-0ubuntu1) 12.0.90
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from fillets...
(No debugging symbols found in fillets)
(gdb) b SDL_SetVideoMode
Breakpoint 1 at 0xf170
(gdb) b SDL_GetWMInfo
Breakpoint 2 at 0xf140
(gdb) r
Starting program: /usr/games/fillets 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
INFO: SDL2CALL SDL_GetVersion
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_strtol
INFO: SDL2CALL SDL_LogMessageV
INFO: sdl12-compat 1.2.59, built on Oct 10 2022 at 12:44:11, talking to SDL2 2.25.0
INFO: SDL2CALL SDL_strrchr
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_Init
INFO: SDL2CALL SDL_GetCurrentVideoDriver
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_CreateMutex
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_EventState
INFO: SDL2CALL SDL_strcmp
INFO: SDL2CALL SDL_DelEventWatch
INFO: SDL2CALL SDL_AddEventWatch
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_ShowCursor
INFO: SDL2CALL SDL_GetNumDisplayModes
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_GetDisplayMode
INFO: SDL2CALL SDL_realloc
INFO: SDL2CALL SDL_calloc
INFO: SDL2CALL SDL_StopTextInput
INFO: SDL2CALL SDL_GetDesktopDisplayMode
INFO: SDL2CALL SDL_AllocFormat
INFO: SDL2CALL SDL_strcasecmp
INFO: SDL2CALL SDL_RWFromFile
INFO: SDL2CALL SDL_malloc
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_CreateRGBSurface
INFO: SDL2CALL SDL_malloc
INFO: SDL2CALL SDL_malloc
INFO: SDL2CALL SDL_malloc
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_GetColorKey
INFO: SDL2CALL SDL_GetSurfaceAlphaMod
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_GetSurfaceBlendMode
INFO: SDL2CALL SDL_SetColorKey
INFO: SDL2CALL SDL_GetColorKey
INFO: SDL2CALL SDL_free
INFO: SDL2CALL SDL_GetSurfaceBlendMode
INFO: SDL2CALL SDL_PixelFormatEnumToMasks
INFO: SDL2CALL SDL_CreateRGBSurface
INFO: SDL2CALL SDL_SetSurfaceBlendMode
INFO: SDL2CALL SDL_UpperBlit
INFO: SDL2CALL SDL_SetSurfaceBlendMode
INFO: SDL2CALL SDL_FreeSurface
INFO: SDL2CALL SDL_FreeSurface
INFO: SDL2CALL SDL_free
INFO: SDL2CALL SDL_free
INFO: SDL2CALL SDL_free

Breakpoint 2, SDL_GetWMInfo (info12=0x7fffffffd9b0) at /home/icculus/projects/sdl12-compat/src/SDL12_compat.c:7133
7133    {
(gdb) c
Continuing.
INFO: SDL2CALL SDL_CreateWindow
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_GetWindowWMInfo
INFO: SDL2CALL SDL_DestroyWindow

Breakpoint 1, SDL_SetVideoMode (width=640, height=480, bpp=32, flags12=805306368) at /home/icculus/projects/sdl12-compat/src/SDL12_compat.c:6146
6146        SetVideoModeInProgress = SDL_TRUE;
(gdb) c
Continuing.
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_WasInit
INFO: SDL2CALL SDL_GetCurrentDisplayMode
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_CreateWindow
X Error of failed request:  BadWindow (invalid Window parameter)
  Major opcode of failed request:  18 (X_ChangeProperty)
  Resource id in failed request:  0x0
  Serial number of failed request:  148
  Current serial number in output stream:  164
INFO: SDL2CALL SDL_WasInit
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_FreeSurface
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_free
INFO: SDL2CALL SDL_free
INFO: SDL2CALL SDL_free
INFO: SDL2CALL SDL_FreeFormat
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_DestroyMutex
INFO: SDL2CALL SDL_QuitSubSystem
INFO: SDL2CALL SDL_WasInit
INFO: SDL2CALL SDL_Quit
[Inferior 1 (process 823566) exited with code 01]
(gdb) q

[icculus]

The SDL_CreateRGBSurface call is SDL_image loading a surface before we've created a window, and then sdl12-compat creating a surface for SDL_WM_SetIcon (I believe from the png that SDL_image just loaded), in case that's relevant.

[icculus]

Also, it looks like it's not crashing with the X error, but cleanly shutting down after this cases SDL_CreateWindow() to fail, but I haven't looked more closely yet.

Nope, Xlib is calling exit() in that error handler, and the app has SDL_Quit in an atexit function.

[icculus]

(sorry for all the spam, I'm just making notes here as I dig.)

Digging deeper, I just have SDL_WM_SetIcon return immediately, since the crash still happens and that's just noise, then I run this 1.2 app with sdl12-compat and log the SDL2 calls:

#include "SDL.h"
#include "SDL_syswm.h"

int main(int argc, char **argv)
{
    SDL_SysWMinfo info12;
    SDL_Init(SDL_INIT_VIDEO);
    SDL_memset(&info12, '\0', sizeof (info12));
    SDL_VERSION(&info12.version);
    SDL_GetWMInfo(&info12);
    SDL_SetVideoMode(100, 100, 0, 0);
    SDL_Quit();
    return 0;
}

Then I do the same with fillets, and here's the diff between the two, up until it hits that fatal SDL_CreateWindow call:

 INFO: SDL2CALL SDL_GetDesktopDisplayMode
 INFO: SDL2CALL SDL_AllocFormat
 INFO: SDL2CALL SDL_strcasecmp
+INFO: SDL2CALL SDL_RWFromFile
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_CreateRGBSurface
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_GetColorKey
+INFO: SDL2CALL SDL_GetSurfaceAlphaMod
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_GetSurfaceBlendMode
+INFO: SDL2CALL SDL_SetColorKey
+INFO: SDL2CALL SDL_GetColorKey
+INFO: SDL2CALL SDL_free
+INFO: SDL2CALL SDL_FreeSurface
+INFO: SDL2CALL SDL_free
+INFO: SDL2CALL SDL_free
+INFO: SDL2CALL SDL_free
 INFO: SDL2CALL SDL_CreateWindow
 INFO: SDL2CALL SDL_memset
 INFO: SDL2CALL SDL_GetWindowWMInfo
@@ -104,77 +124,14 @@
 INFO: SDL2CALL SDL_getenv
 INFO: SDL2CALL SDL_getenv
 INFO: SDL2CALL SDL_CreateWindow

[icculus]

valgrind shows no memory errors in fillets, so it's not an obvious memory corruption. I guess I'll add SDL_image into the mix here.

[icculus]

This program gives (what appears to be) the exact same string of SDL2 calls but does not crash:

#include "SDL.h"
#include "SDL_syswm.h"
#include "SDL_image.h"

int main(int argc, char **argv)
{
    SDL_SysWMinfo info12;
    SDL_Surface *surface;
    SDL_RWops *io;
    SDL_Init(SDL_INIT_VIDEO);
    //printf("imginit=%d\n", IMG_Init(IMG_INIT_PNG));
    io = SDL_RWFromFile("/usr/share/games/fillets-ng/images/icon.png", "rb");
    //printf("io=%p\n", io);
    surface = IMG_LoadTyped_RW(io, 1, "PNG");
    //printf("surface=%p%s\n", surface, surface ? "" : IMG_GetError());
    SDL_WM_SetIcon(surface, NULL);
    SDL_FreeSurface(surface);
    SDL_memset(&info12, '\0', sizeof (info12));
    SDL_VERSION(&info12.version);
    SDL_GetWMInfo(&info12);
    SDL_SetVideoMode(100, 100, 0, 0);
    SDL_Quit();
    return 0;
}

The same functions are called down to the crashing SDL_CreateWindow, so there's something else happening here, or some specific of a given call, I don't know, but it's probably time to either give up or debug this down into Xlib and see what's upsetting it.

[icculus]

Ugh, the binary links directly to libX11, and calls Xutf8TextListToTextProperty, XSetWMName, and XFree between the calls to SDL_GetWMInfo and SDL_SetVideoMode.

Returning immediately with an error from SDL_GetWMInfo was a red herring, since it obviously wanted the Display pointer to call these Xlib functions directly, and went on without them, avoiding the crash.

I guess the solution now is to add a quirk that says "refuse to provide syswm info to this app," which could be useful for other apps that also do unnecessary X11 stuff like this but would otherwise work on Wayland.

(Although in this case, forcing SDL_VIDEODRIVER=wayland also works, since it presumably discovers it can't do X11 stuff and carries on.)

[icculus]

Okay, the quirk gets us past the X11 error, and the game seems largely playable now, but there are bursts of static in the first level from some sound it's not playing/converting correctly, so I'll check that before closing this issue.

[myself600]

Fedora fixed this https://bugzilla.redhat.com/show_bug.cgi?id=2020306

using this patch https://src.fedoraproject.org/rpms/fillets-ng/raw/rawhide/f/fillets-ng-1.0.1-f35-startup-crash.patch