search

libretro / tyrquake

Libretro port of Tyrquake (Quake 1 engine)
GNU General Public License v2.0
42 stars 46 forks source link

(libretro.c) Clean up file path handling + fix segfault when loading content from 'non-standard' directories #108

Closed jdgleaver closed 3 years ago

jdgleaver commented 3 years ago

At present, the core will segfault on certain platforms when loading PAK files from non-standard directories (anything other than id1, quoth, hipnotic, rogue) due to illegal usage of strncpy(). This can be observed most easily by attempting to run the shareware version of Quake from RetroArch's online content downloader.

This PR fixes the issue, and also cleans up all file path handling in libretro.c (using libretro-common routines). A number of libretro-common files have also been updated to the latest version.

inactive123 commented 3 years ago

This still crashes for me on my end.

[INFO] [Playlist]: Loading favorites file: [C:\retroarch\content_favorites.lpl].
[INFO] [Environ]: GET_USERNAME: "".

Thread 1 received signal SIGSEGV, Segmentation fault.
R_BeginEdgeFrame () at common/r_edge.c:92
92         surfaces[1].spans = NULL;    // no background spans yet
(gdb) bt
#0  R_BeginEdgeFrame () at common/r_edge.c:92
#1  0x00007ffc1c9386d9 in R_EdgeDrawing () at common/r_main.c:1020
#2  0x00007ffc1c93879f in R_RenderView_ () at common/r_main.c:1066
#3  0x00007ffc1c9388a4 in R_RenderView () at common/r_main.c:1106
#4  0x00007ffc1c95eac2 in V_RenderView () at common/view.c:906
#5  0x00007ffc1c946f64 in SCR_UpdateScreen () at common/screen.c:691
#6  0x00007ffc1c90ebe8 in _Host_Frame (time=0.00834167562)
    at common/host.c:712
#7  0x00007ffc1c90ec46 in Host_Frame (time=0.00834167562) at common/host.c:727
#8  0x00007ffc1c95a971 in retro_run () at common/libretro.c:907
#9  0x00007ff7dc9cfcb1 in core_run ()
#10 0x00007ff7dc9d354c in runloop_iterate ()
#11 0x00007ff7dc9d4868 in rarch_main ()
#12 0x00007ff7dd0b5daa in main_getcmdline ()
#13 0x00007ff7dc9b13b1 in __tmainCRTStartup ()
    at C:/_/M/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:321
#14 0x00007ff7dc9b14c6 in WinMainCRTStartup ()
    at C:/_/M/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:176

It might be an unrelated issue from the one you found with ASAN. As such, I could merge this, but it doesn't prevent the segfault at startup on Windows at least on my machine, as I was hoping for.

jdgleaver commented 3 years ago

Yes, your segfault has absolutely nothing to do with the illegal usage of strncpy(). Not sure if I can debug this surfaces issue on Linux (there is no error on this platform), but I will have a look next week...

bslenul commented 3 years ago

Thanks for this, not being limited to specific names for the folders is a pretty nice addition! 👍

For what it's worth, no issue here on Windows 10 / i5-4670K / GTX 970 with GL/Vulkan/D3D11. Tested with the shareware from the online updater and the full game. No idea if the version of the game matters, but for the full game I tested with 1.06 (PAK0.PAK - MD5: 5906E5998FC3D896DDAF5E6A62E03ABB + PAK1.PAK - D76B3E5678F0B64AC74CE5E340E6A685) and the 2021 re-release (PAK0.PAK - 457D9221804B0346FD8F42923B2A7879).

jdgleaver commented 3 years ago

@bslenul Thanks for the feedback :)

I tracked the issue down to some unsafe pointer casts (i.e. assumptions that certain data types have certain widths - not true on 64bit Windows!), so hopefully this PR puts it to bed: https://github.com/libretro/tyrquake/pull/109