-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RetroArch segfaults when loading cores from command line when no content is specified #4493
Comments
Can't reproduce this at all following your step 1 in 'Steps to reproduce the bug'. Please be more specific so that I can actually reproduce this with (say) the SNES9x core. Also, this is not necessarily a RetroArch issue, but more a core-by-core issue. The issue there is that we don't check if 'game' is NULL there. If it's NULL, it should return false inside retro_load_game, that would allow RetroArch to cleanly handle the error. So it's a core bug. |
Its not a core problem as every core does it (Except content-less cores like 2048). Unless you are saying all of the cores are broken? All I have to do to reproduce that is that command, for example...
The backtrace was arbitrarily done with snes9x-libretro, if any other core might produce a more helpful backtrace I can make one. I also asked a friend who was running a RetroArch commit from November and he was also able to reproduce it. |
It is a core problem. Reason - snes9x no longer crashes like that anymore after I updated it. Go ahead, do a git pull on the repo, recompile it and try again. It will now cleanly exit and return you back to the command prompt. |
Well since you put it that way I have to agree you are right, thanks! snes9x no longer segfaults now. I guess this can be addressed in each core as its encountered and this issue can be closed. For reference. |
Can you make a listing of each core this happens with so I can go down the list and get most of them fixed? |
Sure, I will compile one by the latest tomorrow. |
@twinaphex Here is a list of cores that segfaults when started with no content for me.
|
Isn't the point of libretro to remain backwards and forwards compatible forever, including old cores that assume the retro_load_game argument can be dereferenced? Are we going to break that promise? |
I'm going to reopen this issue until a solution can be agreed upon. See this PR. libretro/bsnes-mercury#30 |
Let's take the discussion here. Per my post in that PR, retro_load_game(NULL) is not supported by that core, or most other cores. Any reasonable definition of "unsupported" means "the callee can assume this never happens", as evidenced by the number of cores you're "fixing". If we're suddenly defining what this specific unsupported scenario does, then it is by definition supported. Making a previously-unsupported scenario supported, without there being properly documented fallbacks in place for old cores/fronts, is a backwards incompatible change and requires incrementing RETRO_API_VERSION. Unless you're doing that, I will not tolerate unexplained, undocumented, forwards incompatible changes in an API documented as stable. If you want to increment that integer, go ahead. Otherwise, the only correct solution is not calling retro_load_game(NULL) unless SET_SUPPORT_NO_GAME is used; even with your proposed changes, the frontend can already know that it will just return false, which is a waste of time. |
@Alcaro When you have a chance can you look at the other linked issues to make sure they are all core bugs? |
No, two of those are still frontend bugs; rarch is calling retro_unload_game despite retro_load_game not returning true. That's better handled in #4493. Reicast looks like it's mixing up retro_unload_game and retro_deinit in some strange way, which is a core bug. |
The issues that still occur and seem like frontend issues are... libretro/nxengine-libretro#29 |
My understanding is this is a combination of a frontend bug in RetroArch and a core bug in lutro. RetroArch attempts to unload content even when it was never loaded and lutro is deiniting content in retro_deinit instead of retro_unload_core. Please see: libretro/RetroArch#4493 libretro/RetroArch#7102 Note that this fix in lutro is enough without also requiring the fix in RetroArch which is needed for other cores.
My understanding is this is a combination of a frontend bug in RetroArch and a core bug in hatari. RetroArch attempts to unload content even when it was never loaded and hatari is deiniting content in retro_deinit instead of retro_unload_core. Please see: libretro/RetroArch#4493 libretro/RetroArch#7102 Note that this fix in hatari also requires the fix in RetroArch. This also removes the executive permissions of 0755 from Makefile.libretro to the correct permissions of 0644.
Please see: libretro/RetroArch#4493 libretro/RetroArch#7102 Note this does not require changes in RetroArch. This also removes the executive permissions of 0755 from Makefile.libretro to the correct permissions of 0644.
Note this does not require changes in RetroArch. Please see: libretro/RetroArch#4493 libretro/RetroArch#7102 This also removes the executive permissions of 0755 from Makefile.libretro to the correct permissions of 0644.
Note this does not require changes in RetroArch. Please see: libretro/RetroArch#4493 libretro/RetroArch#7102
I guess this can be closed now. |
Description
RetroArch will segfault when a core is loaded from the command line with
-L
while failing to specify any content. It seems to affect all cores with content while no content cores such as 2048 work fine.Expected behavior
RetroArch should fail safely if no content is found and maybe print a message explaining that no content was found.
Actual behavior
This is the backtrace I receive when tested with snes9x-libretro.
Full GDB log - http://pastebin.com/dJv1VT6y
Steps to reproduce the bug
retroarch -L /path/to/core_libretro.so
Bisect Results
This occurs with 1.3.6 too and I have not tried any earlier commits.
Version/Commit
snes9x-libretro-2017.01.23_690d475_master-x86_64-1_git
RetroArch-2017.01.24_7aaf19381_master-x86_64-1_git
Environment information
Slackware64-current
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
The text was updated successfully, but these errors were encountered: