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 5222 - Crash when running with -DHIDAPI=ON
Summary: Crash when running with -DHIDAPI=ON
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: joystick (show other bugs)
Version: HG 2.0
Hardware: x86_64 Linux
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords: target-2.0.14
Depends on:
Blocks:
 
Reported: 2020-07-03 10:06 UTC by Sven-Hendrik Haase
Modified: 2020-12-09 03:04 UTC (History)
4 users (show)

See Also:

Attachments
fix error checking and initialisation in SDL_hidapi.c (5.32 KB, patch)
2020-07-05 12:03 UTC, Mathieu Eyraud 		Details | Diff
fix error checking and initialisation in SDL_hidapi.c (no log) (5.33 KB, patch)
2020-07-06 09:01 UTC, Mathieu Eyraud 		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 Sven-Hendrik Haase 2020-07-03 10:06:09 UTC 
Applications that init the joystick subsystem crash after I use compile my SDL with -DHIDAPI=ON.

Example running blobby volley (SDL2) with on recent tip with -DHIDAPI=ON:

#0  0x0000000000000000 in  ()
#1  0x00007ffff7e3e555 in LIBUSB_hid_init () at ../src/hidapi/libusb/hid.c:460
#2  hid_init () at ../src/hidapi/SDL_hidapi.c:491
#3  0x00007ffff7e40435 in hid_init () at ../src/hidapi/SDL_hidapi.c:454
#4  0x00007ffff7e3cb99 in HIDAPI_JoystickInit () at ../src/joystick/hidapi/SDL_hidapijoystick.c:582
#5  0x00007ffff7e01eb4 in SDL_JoystickInit () at ../src/joystick/SDL_joystick.c:238
#6  0x00007ffff7d2a2b5 in SDL_InitSubSystem_REAL (flags=16944) at ../src/SDL.c:234
#7  0x0000555555669348 in main ()

I compile my SDL2 like this:

  cmake . \
      -Bbuild \
      -GNinja \
      -DCMAKE_INSTALL_PREFIX=/usr \
      -DSDL_STATIC=OFF \
      -DSDL_DLOPEN=ON \
      -DARTS=OFF \
      -DESD=OFF \
      -DNAS=OFF \
      -DALSA=ON \
      -DHIDAPI=ON \
      -DPULSEAUDIO_SHARED=ON \
      -DVIDEO_WAYLAND=ON \
      -DRPATH=OFF \
      -DCLOCK_GETTIME=ON \
      -DJACK_SHARED=ON
  ninja -C build
Comment 1 Sven-Hendrik Haase 2020-07-03 10:07:14 UTC 
Some more system information:

Arch Linux
cmake 3.17.3
gcc 10.1.0
Comment 2 Sven-Hendrik Haase 2020-07-03 10:09:50 UTC 
Our downstream bug report about this: https://bugs.archlinux.org/task/67174
Comment 3 Mathieu Eyraud 2020-07-05 12:03:26 UTC 
Created attachment 4402 [details]
fix error checking and initialisation in SDL_hidapi.c

SDL dynamically loads libusb but does not check the return value of 'SDL_LoadFunction'.

Also libusb is loaded and initialized several time because 'SDL_hidapi_wasinit' is never set to true.

I made a patch if you want to test:
  - check that 'hid_init' is called once and only once,
  - check return value of 'hid_init',
  - check return value of 'SDL_LoadFunction',
  - check return value of 'SDL_malloc',
  - add some debug logging.
Comment 4 Sven-Hendrik Haase 2020-07-05 12:22:48 UTC 
(In reply to meyraud705 from comment #3)
> Created attachment 4402 [details]
> fix error checking and initialisation in SDL_hidapi.c
> 
> SDL dynamically loads libusb but does not check the return value of
> 'SDL_LoadFunction'.
> 
> Also libusb is loaded and initialized several time because
> 'SDL_hidapi_wasinit' is never set to true.
> 
> I made a patch if you want to test:
>   - check that 'hid_init' is called once and only once,
>   - check return value of 'hid_init',
>   - check return value of 'SDL_LoadFunction',
>   - check return value of 'SDL_malloc',
>   - add some debug logging.

Doesn't compile for me:

FAILED: CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o 
/usr/bin/cc -DSDL2_EXPORTS -DSDL_USE_IME -DUSING_GENERATED_CONFIG_H -Iinclude -I../include -Iwayland-generated-protocols -I/usr/include/libdrm -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -I/usr/include/ibus-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/libmount -I/usr/include/blkid -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -idirafter /build/sdl2/src/SDL2-2.0.12/src/video/khronos -I/usr/include/libusb-1.0 -I/build/sdl2/src/SDL2-2.0.12/src/hidapi/hidapi -DHAVE_LINUX_VERSION_H -I/usr/include/libdrm -I/usr/include -D_REENTRANT -msse3 -msse2 -msse -m3dnow -mmmx -Wshadow -fvisibility=hidden -Wdeclaration-after-statement -Werror=declaration-after-statement -fno-strict-aliasing -Wall -march=x86-64 -mtune=generic -O2 -pipe -fno-plt  -fPIC -MD -MT CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o -MF CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o.d -o CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o   -c ../src/hidapi/SDL_hidapi.c
In file included from ../src/hidapi/../SDL_internal.h:45,
                 from ../src/hidapi/SDL_hidapi.c:31:
../src/hidapi/SDL_hidapi.c: In function ‘hid_init’:
../src/hidapi/../dynapi/SDL_dynapi_overrides.h:35:21: warning: implicit declaration of function ‘SDL_LogWarn_REAL’; did you mean ‘SDL_CondWait_REAL’? [-Wimplicit-function-declaration]
   35 | #define SDL_LogWarn SDL_LogWarn_REAL
      |                     ^~~~~~~~~~~~~~~~
../src/hidapi/SDL_hidapi.c:501:13: note: in expansion of macro ‘SDL_LogWarn’
  501 |             SDL_LogWarn(SDL_LOG_CATEGORY_INPUT, SDL_LIBUSB_DYNAMIC " found but could not load function.");
      |             ^~~~~~~~~~~
../src/hidapi/SDL_hidapi.c:501:25: error: ‘SDL_LOG_CATEGORY_INPUT’ undeclared (first use in this function)
  501 |             SDL_LogWarn(SDL_LOG_CATEGORY_INPUT, SDL_LIBUSB_DYNAMIC " found but could not load function.");
      |                         ^~~~~~~~~~~~~~~~~~~~~~
Comment 5 Mathieu Eyraud 2020-07-06 09:01:48 UTC 
Created attachment 4403 [details]
fix error checking and initialisation in SDL_hidapi.c (no log)

Same patch with problematic line commented out (you will not get logging in case SDL_LoadFunction fail).

I don't understand why it doesn't compile for you. SDL_hidapi.c includes SDL_internals.h which include SDL_Log.h, and SDL_log.h defines SDL_LOG_CATEGORY_INPUT.
Comment 6 Sven-Hendrik Haase 2020-07-06 10:28:31 UTC 
That seems to be working well for me, cool!
Comment 7 Sven-Hendrik Haase 2020-07-10 23:01:23 UTC 
A user reports that while it doesn't crash, it also doesn't work. I'll try to get them to participate here.
Comment 8 Dom 2020-07-13 20:06:24 UTC 
This looks like an important patch to make using HIDAPI less risky
Comment 9 Sam Lantinga 2020-12-09 03:04:31 UTC 
Patch added, thanks!
https://hg.libsdl.org/SDL/rev/26a87784645c