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

Unmap Fixes #2080

Merged
merged 5 commits into from
Jan 8, 2025
Merged

Conversation

StevenMiller123
Copy link
Contributor

@StevenMiller123 StevenMiller123 commented Jan 7, 2025

This PR fixes several memory issues I've noticed.

  1. When games try to unmap reserved memory, this would cause address_space asserts on Windows. This is because reserved memory was never mapped in address_space to begin with. My fix is to only unmap in address_space if the unmapped VMA had type != VMAType::Reserved.
  2. Some games also attempt to unmap free memory. This causes address_space asserts on Windows, and also messes up later memory use calculations. My solution is adding an early return if the VMA to unmap has type == VMAType::Free.
  3. UnmapMemoryImpl doesn't properly handle pooled memory. PoolReserved memory can either be reserved or decommitted memory, either case isn't mapped in address_space. Additionally, Pooled memory is GPU mapped. My solution is adding those relevant checks so we don't run into any strange Windows-specific issues.
  4. When games overwrite memory with reserved pages, our check for vma.type == VMAType::Free would fail. This is because, while the UnmapMemoryImpl call modifies vma_map, the vma retrieved before that call is not modified. To fix this, I add an extra FindVMA call after the unmap.

This should fix cases of:

[Debug] <Critical> address_space.cpp:operator():268: Assertion Failed!
Invalid address/size given to unmap.

And some cases of:

[Debug] <Critical> memory.cpp:operator():217: Assertion Failed!

Credits to @red-prig for providing relevant information.

@Missake212
Copy link

The Last of Us Remastered goes from crashing on launch to now hanging on a black screen.
shad_log.txt

@StevenMiller123
Copy link
Contributor Author

@Missake212 Pretty sure TLOU Remastered hangs on main too, this PR shouldn't impact it. Could you test main to verify?

@Missake212
Copy link

@Missake212 Pretty sure TLOU Remastered hangs on main too, this PR shouldn't impact it. Could you test main to verify?

TLOU Remastered crashes on main for me, here's the log
shad_log.txt

@StevenMiller123
Copy link
Contributor Author

Well, an improvement is an improvement, I won't complain about that.

@Inui-senpai
Copy link

DDLC+ now crashes on Existing mapping does not contain requested unmap range.
shad_log.txt

@georgemoralis georgemoralis merged commit fc50567 into shadps4-emu:main Jan 8, 2025
10 checks passed
@georgemoralis
Copy link
Collaborator

let's get dangerous :D

@StevenMiller123 StevenMiller123 deleted the fix-reserved-unmaps branch January 9, 2025 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants