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

Fixed SIGSEGV in PPCDebugInterface When Reading Too Early in The Boot Process #6292

Merged
merged 1 commit into from Jan 13, 2018

Conversation

2 participants
@gyroninja
Contributor

gyroninja commented Jan 7, 2018

I was encountering a weird issue when starting dolphin with the debug flag. It would immediately crash when trying to open a game UNLESS I had dolphin's window set to fullscreen.
It turns out that I was encountering a multithreading issue where the DR bit of MSR was not yet set. This caused a segfault when it was trying to read address 0. It is trivial to confirm that this is a multithreading issue because if you add a sleep before reading the DR bit it is properly set.
In order to fix this I simply changed PPCDebugInterface's isAlive() function to use Core::IsRunningAndStarted() instead of Core::IsRunning() which it was using previously. I also want to point out that Core::IsRunning() is kind of weird. It unnecessarily checks s_hardware_initialized.
While not necessary to fix the crash I also moved the code for setting that it is finished initializing / booting to after CBoot::BootUp happens (which initializes MSR).

There are a few other solutions I tried which worked:
Delaying before reading DR: Bad
Returning false when trying to read when DR is 0: Doesn't address the root of the problem
Changing IsRunning to s_hardware_initialized && !s_is_stopping: This might not be what IsRunning is meant to do.

It might also be worth checking if the comment on line 20 of PPCDebugInterface is still valid with this patch (my changes don't seem to be relevant to shutting down so it probably still doesn't work).

@leoetlino

This comment has been minimized.

Show comment
Hide comment
@leoetlino

leoetlino Jan 12, 2018

Member

I think we consider the console to be initialised and "booted" before CBoot::BootUp is called because the code in CBoot is supposed to mimic the console boot process (IOS / BS2). CBoot stuff (like on console) runs after all the other hardware is already initialised, so I'd suggest removing the second commit.

Member

leoetlino commented Jan 12, 2018

I think we consider the console to be initialised and "booted" before CBoot::BootUp is called because the code in CBoot is supposed to mimic the console boot process (IOS / BS2). CBoot stuff (like on console) runs after all the other hardware is already initialised, so I'd suggest removing the second commit.

@gyroninja

This comment has been minimized.

Show comment
Hide comment
@gyroninja

gyroninja Jan 12, 2018

Contributor

Okay, I went ahead and reverted that commit. The reason for the commit was in case some other part of the codebase was relying on the assumption that the MSR had been setup. I guess there wouldn't be many places this could happen though as the calls would need to come from the UI like the memory pane does.

Contributor

gyroninja commented Jan 12, 2018

Okay, I went ahead and reverted that commit. The reason for the commit was in case some other part of the codebase was relying on the assumption that the MSR had been setup. I guess there wouldn't be many places this could happen though as the calls would need to come from the UI like the memory pane does.

@leoetlino leoetlino merged commit 27b6a2e into dolphin-emu:master Jan 13, 2018

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment