barrage: segfault on startup

[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 barrage (Debian package version 1.0.5-1)
• Some relevant libraries:
  • libsdl1.2-compat either 1.2.58-1 (packaged in Debian experimental) or commit 63e4393 (locally built)
  • libsdl2-2.0-0 version 2.24.1+dfsg-1
  • libsdl-mixer1.2 version 1.2.12-17+b2
  • libsdl1.2debian (real SDL 1.2) version 1.2.15+dfsg2-8

To reproduce:

• barrage
• LD_LIBRARY_PATH=.../sdl12-compat/_build barrage
• SDL_VIDEODRIVER=wayland LD_LIBRARY_PATH=.../sdl12-compat/_build barrage

Expected result: it runs

Actual result: real SDL 1.2 works. sdl12-compat segfaults on startup:

Thread 1 (Thread 0x7f22f3526080 (LWP 16145)):
#0  0x0000558e4c36f627 in data_load () at data.c:218
#1  0x0000558e4c369729 in main (argc=1, argv=<optimized out>) at main.c:825

This is dereferencing SDL_Surface *screen which is NULL. It looks like this:

    screen = SDL_CreateRGBSurface( SDL_SWSURFACE, 640, 480,
            vsurf->format->BitsPerPixel,
            vsurf->format->Rmask, vsurf->format->Bmask,
            vsurf->format->Gmask, vsurf->format->Amask );

must have failed.

[icculus]

This was fixed by eaeaf932ba7439ee9a75c1729ff8bd4b446e1b1e.

[slouken]

What is vsurf->format in this case? It seems odd that we wouldn't be able to create a surface in the format of the screen, which is what I assume is happening here?

[icculus]

The failing call was:

SDL_CreateRGBSurface (flags12=0, width=640, height=480, depth=16, Rmask=63488, Gmask=31, Bmask=2016, Amask=0)

But I haven't looked further than a bisect to see what fixed it...those masks should be correct, though. Hmm.

[icculus]

Okay, I'm reopening just to sanity check this.

[icculus]

Actually, those masks aren't correct. They made the Ruth Bader Ginsburg mistake: they asked for a 16-bit RBG layout, but they meant RGB.

Honestly, I do this all the time, too.

Either 1.2 would support this, or 1.2 would just treat these as bits to copy and it didn't matter, but SDL2 rejects it and when we added the fallback for bogus 16-bit color masks, it fixed this game.

This happens to render correctly because the game makes the same mistake in other places, so it's blitting between two surfaces with the same wrong format, and for the final presentation, the game memcpy()'s the surface data to the actual video surface without worrying about the layout, so it happens to work out.

The only part it gets it wrong (and this happens with classic SDL 1.2, too) is when it does the fadeout when quitting the game. I was wondering why it turns purplish when fading, and when I fixed the masks, it fades correctly.

Anyhow, I think we're good to go here.

[icculus]

(This game is still actively maintained, I'll send a patch upstream, since it's a legit bug, but we handle this game correctly with or without the game fix.)

[slouken]

Ah, great sleuthing!

[smcv]

Confirmed fixed by https://github.com/libsdl-org/sdl12-compat/commit/67f8b3a85b782eefb4db90f34d5b0742ef2cb5fc, 1.2.58 + 29 commits