Skip to content
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

JitCache: Fix potentially dangling pointer to fast block map. #12150

Merged
merged 2 commits into from Sep 2, 2023

Conversation

AdmiralCurtiss
Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss commented Sep 1, 2023

Whenever JitBaseBlockCache::Clear() got called, it threw away the memory mapping for the fast block map and created a new one. This new mapping typically got mapped at the same address at the old one, but this is not guaranteed. The pointer to the mapping gets embedded in the generated dispatcher code in Jit64AsmRoutineManager::Generate(), which is only called once on game boot, so if the new mapping ended up at a different address than the old one, the pointer in the ASM pointed at garbage, leading to a crash.

This fixes the issue by guaranteeing that the new mapping is mapped at the same address.


@krnlyng You might be interested in this.

… memory region whose pages are only allocated on first access.
Whenever JitBaseBlockCache::Clear() got called, it threw away the memory mapping for the fast block map and created a new one. This new mapping typically got mapped at the same address at the old one, but this is not guaranteed. The pointer to the mapping gets embedded in the generated dispatcher code in Jit64AsmRoutineManager::Generate(), which is only called once on game boot, so if the new mapping ended up at a different address than the old one, the pointer in the ASM pointed at garbage, leading to a crash.

This fixes the issue by guaranteeing that the new mapping is mapped at the same address.
@AdmiralCurtiss
Copy link
Contributor Author

Someone needs to test if the Linux and Android implementations work as expected. Boot a game, play for a minute, then do something that causes a JIT cache flush (saving and then reloading a savestate should do it) and see if anything breaks.

@JMC47
Copy link
Contributor

JMC47 commented Sep 2, 2023

I can test that to see what happens.

@AdmiralCurtiss AdmiralCurtiss changed the title [WIP] JitCache: Fix potentially dangling pointer to fast block map. JitCache: Fix potentially dangling pointer to fast block map. Sep 2, 2023
@AdmiralCurtiss AdmiralCurtiss marked this pull request as ready for review September 2, 2023 02:48
@JMC47 JMC47 merged commit 6beaee0 into dolphin-emu:master Sep 2, 2023
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the fast-block-cache-fix branch September 2, 2023 16:41
@theofficialgman
Copy link

theofficialgman commented Sep 6, 2023

@AdmiralCurtiss @JosJuice I have bisected crashes on launch of all games with both opengl/vulkan and jit and interpreter back to this commit.

For reference, commit 6beaee078baeaa80f9f1c21ce372ce28fd8273b6 on master (the merge of this PR) crashes and the previous commit that can be checked out 5e5887a378db28324a8fc8825f21539525412e12 (which is the commit directly before the merge of this PR) does not crash.

Please revert as soon as possible. Confirmed on mulitple systems (all ARM64 Linux)

edit: for clarity, dolphin itself fully crashes, the game does not just fail to launch.

@theofficialgman
Copy link

CC: @JMC47 since I forgot to in the intial post ^

@AdmiralCurtiss
Copy link
Contributor Author

^ Please test if #12173 fixes it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants