More crash fixes #17399
More crash fixes #17399
Conversation
… and skip if not. buildfix
…d as badly as in that crash report though...
GPU/GPUCommonHW.cpp
Outdated
| @@ -819,6 +820,7 @@ void GPUCommonHW::FastRunLoop(DisplayList &list) { | |||
| if (flags & (FLAG_EXECUTE | FLAG_EXECUTEONCHANGE)) { | |||
| downcount = dc; | |||
| (this->*info.func)(op, diff); | |||
| // TODO: Check pc here, and break if invalid, as the func can change it. Might have a performance impact though. | |||
There was a problem hiding this comment.
I'd rather check it when modifying the PC, rather than after every func (which only a small % of them can change the PC.)
-[Unknown]
There was a problem hiding this comment.
Right, we're currently not even using the return value of the func, which would be cheap to check. So we could do the check inside where needed and return an error, which we could branch on here. Changing the todo.
| @@ -365,7 +365,7 @@ class GameInfoWorkItem : public Task { | |||
| } | |||
|
|
|||
| // In case of a remote file, check if it actually exists before locking. | |||
| if (!info_->GetFileLoader()->Exists()) { | |||
| if (!info_->GetFileLoader() || !info_->GetFileLoader()->Exists()) { | |||
There was a problem hiding this comment.
Hm. I think we now schedule these on multiple threads. There might be a race condition now where DisposeFileLoader() could be run by one thread while another is starting up for the very same file (i.e. with additional want flags.)
-[Unknown]
There was a problem hiding this comment.
Ah, that's a good catch, very likely what happens..
| @@ -272,7 +272,9 @@ class PopupMultiChoiceDynamic : public PopupMultiChoice { | |||
|
|
|||
| protected: | |||
| void PostChoiceCallback(int num) override { | |||
| *valueStr_ = choices_[num]; | |||
| if (valueStr_) { | |||
There was a problem hiding this comment.
Hm, if anything I'd be worried about a lifetime issue, i.e. the std::string passed to PopupMultiChoiceDynamic could be dead and this could be a use-after-free if the callback somehow happens late.
-[Unknown]
| @@ -62,6 +62,7 @@ class GPUCommonHW : public GPUCommon { | |||
|
|
|||
| void Execute_TexFlush(u32 op, u32 diff); | |||
|
|
|||
| // TODO: Have these return an error code if they jump to a bad address. If bad, stop the FastRunLoop. | |||
There was a problem hiding this comment.
Or just set gpuState = GPU_ERROR; and set downcount = 0; to force a recheck.
-[Unknown]
There was a problem hiding this comment.
Oh yeah, that's probably the slickest solution.
There was a problem hiding this comment.
Actually it seems we already effectively do that in many cases through UpdateState. But not in fast memory mode.
I am going to remove Execute_JumpFast and Execute_CallFast. I don't believe they are worth it.
Fix issues found looking at 1.15.1 reports, documented in #17364