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

Common/CodeBlock: Call ResetCodePtr when decreasing region_size #10447

Merged
merged 1 commit into from Feb 13, 2022

Conversation

JosJuice
Copy link
Member

Fixes https://bugs.dolphin-emu.org/issues/12827.

A description of what was going wrong:

JitArm64::Init first calls CodeBlock::AllocCodeSpace, after which CodeBlock and Arm64Emitter consider us to have 96 MB of code space available. JitArm64::Init then calls AddChildCodeSpace, which is supposed to take 64 MiB of that space and give it to m_far_code. CodeBlock's view of how much space there is gets updated from 96 MiB to 32 MiB, but due to the missing call, Arm64Emitter keeps thinking that it has 96 MiB of space available.

The last thing JitArm64::Init does is to call ResetFreeMemoryRanges. This function asks Arm64Emitter how much code space is available and stores a range of that size in m_free_ranges_near, meaning that m_free_ranges_near ends up being backed by both nearcode and farcode! This is a ticking time bomb; as soon as we grab memory from m_free_ranges_near which is backed by farcode, we're in trouble. The crash I ran into in my testing was caused by fastmem code being allocated in farcode (our backpatch handler only handles accesses made from nearcode), but you may as well get errors caused by code intended for nearcode overwriting code intended for farcode or vice versa.

So why did NBA Live 2005 crash when most games had no problems, and why was the bug bisected to the commit that increased the size of far code from 16 MiB to 64 MiB? Well, as long as we're only using the first 32 MiB of the big 96 MiB range, everything works. What happens with NBA Live 2005 (I have not investigated exactly through what mechanism this happens) is that at some point the range in m_free_ranges_near gets split into two ranges, one which is backed by nearcode and one which is backed by farcode. Dolphin prefers to select the biggest range available (we don't want to pick a tiny 1 KiB range that may not be able to fit the whole block we're about to emit, after all), and after increasing the size of farcode to 64 MiB, farcode is bigger than nearcode.

Fixes https://bugs.dolphin-emu.org/issues/12827.

A description of what was going wrong:

JitArm64::Init first calls CodeBlock::AllocCodeSpace, after which
CodeBlock and Arm64Emitter consider us to have 96 MB of code space
available. JitArm64::Init then calls AddChildCodeSpace, which is
supposed to take 64 MiB of that space and give it to m_far_code.
CodeBlock's view of how much space there is gets updated from 96 MiB
to 32 MiB, but due to the missing call, Arm64Emitter keeps thinking
that it has 96 MiB of space available.

The last thing JitArm64::Init does is to call ResetFreeMemoryRanges.
This function asks Arm64Emitter how much code space is available and
stores a range of that size in m_free_ranges_near, meaning that
m_free_ranges_near ends up being backed by both nearcode and farcode!
This is a ticking time bomb; as soon as we grab memory from
m_free_ranges_near which is backed by farcode, we're in trouble.
The crash I ran into in my testing was caused by fastmem code being
allocated in farcode (our backpatch handler only handles accesses made
from nearcode), but you may as well get errors caused by code intended
for nearcode overwriting code intended for farcode or vice versa.

So why did NBA Live 2005 crash when most games had no problems,
and why was the bug bisected to the commit that increased the size
of far code from 16 MiB to 64 MiB? Well, as long as we're only
using the first 32 MiB of the big 96 MiB range, everything works.
What happens with NBA Live 2005 (I have not investigated exactly
through what mechanism this happens) is that at some point the range
in m_free_ranges_near gets split into two ranges, one which is
backed by nearcode and one which is backed by farcode. Dolphin
prefers to select the biggest range available (we don't want to
pick a tiny 1 KiB range that may not be able to fit the whole block
we're about to emit, after all), and after increasing the size of
farcode to 64 MiB, farcode is bigger than nearcode.
@AdmiralCurtiss
Copy link
Contributor

I will note that the x64 JIT didn't run into the issue because it manually called ResetCodePtr() after the child allocations.

But yeah, seems fine, as long as AllocChildCodeSpace() is only called before code is already generated (which I believe is true) this works.

@JMC47
Copy link
Contributor

JMC47 commented Feb 13, 2022

Merge this for now to get the crashing to stop. We can standardize how this works later between the different JITs if necessary.

@JMC47 JMC47 merged commit 86dbf76 into dolphin-emu:master Feb 13, 2022
10 checks passed
@JosJuice JosJuice deleted the resetcodeptr-on-decrement branch February 13, 2022 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants