Bug#1062969: doomsday: launching doomsday (2.3.1+ds1-1+b2) causes segfault in libsdl2-2.0-0 (2.30.0+dfsg-1)

[Simon McVittie]

Control: clone -1 -2
Control: reassign -1 libsdl2 2.30.0+dfsg-1
Control: reassign -2 doomsday 2.3.1+ds1-1
Control: retitle -1 libsdl2: SDL_JoystickName() doesn't tolerate NULL joystick object since 2.30
Control: retitle -2 doomsday: shouldn't rely on being able to call methods on a NULL SDL object

On Sun, 04 Feb 2024 at 12:31:35 +0000, Simon McVittie wrote:
> When I try the command above, on GNOME in Wayland mode, I get what
> appears to be a different crash

Now reported separately as #1065709, which I now see was a duplicate
of #1057620 (I'll merge them).

> which makes me wonder whether there is some conflict between the way
> libsdl2 is using X11, and the way the doomsday engine is using X11
> internally?

Yes. A workaround for this crash is to run it like:

SDL_VIDEODRIVER=x11 QT_QPA_PLATFORM=xcb doomsday -game doom2

and with that command I can reproduce a backtrace similar to the one
you provided.

> This might either be a regression in libsdl2, or an unintended interaction
> with the new libsdl2 exposing a bug in doomsday; at this stage it's hard
> to tell which.

I think it's really both, so I'm cloning the bug.

If you don't have a joystick or gamepad attached, Doomsday relies on being
able to call SDL_JoystickName(nullptr).

On one hand, this is fairly clearly a programming error, because
SDL_JoystickName() is documented to take a pointer to a SDL_Joystick as
its parameter, and nullptr isn't a joystick (more like the absence of a
joystick). So doomsday should be doing more like this:

 de::String Joystick_Name()
 {
 #ifndef DENG_NO_SDL
-    return SDL_JoystickName(joy);
+    return (joy ? SDL_JoystickName(joy) : "");
 #else
     return "";
 #endif
 }

or perhaps:

 de::String Joystick_Name()
 {
 #ifndef DENG_NO_SDL
-    return SDL_JoystickName(joy);
+    return (joyAvailable ? SDL_JoystickName(joy) : "");
 #else
     return "";
 #endif
 }

Let's use a cloned bug to represent that.

But on the other hand, SDL has historically tolerated NULL parameters to
most of its public API, returning an error code, so the fact that 2.30
no longer does that for this particular function is technically a
regression I suppose. I'll use #1062969 to represent the SDL regression.

    smcv