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

R5900 Recompiler: Export reset flag (To use with eemem) #10143

Closed
wants to merge 2 commits into from

Conversation

F0bes
Copy link
Member

@F0bes F0bes commented Oct 18, 2023

This is just an idea.
Currently EE instruction memory is page protected (I assume PAGE_READONLY?) for the JIT. This prevents you from writing to it using WriteProcessMemory. This is an issue when you depend on the EEMem pointer.
You can bypass this by changing the page protection

DWORD dwOldProtect;
// Change the protection to writeable
VirtualProtectEx(hProcess, (PVOID)addressToWrite, sizeToWrite, PAGE_READWRITE, &dwOldProtect);

// Write your data
WriteProcessMemory(hProcess, (PVOID)addressToWrite, &newOp, sizeToWrite, nullptr);

// Restore the protection
VirtualProtectEx(hProcess, (PVOID)addressToWrite, sizeToWrite, dwOldProtect, nullptr);

There is an issue with this though, you don't trip the page protection in PCSX2. Your newly written instruction data wont be recompiled :(
(I do wonder if an external process is able to trip the protection, that would be a much better solution)

This is where this PR comes in.
We can export eeRecNeedsReset and do the following to clear our old instruction blocks after we've written to the guest memory

// Get the address of eeRecNeedsReset
uintptr_t recExitRequested = GetProcAddressEx(hProcess, hPCSX2, "eeRecNeedsReset");
bool resetTrue = true;
// Set eeRecNeedsReset to true
WriteProcessMemory(hProcess, (PVOID)eeRecNeedsReset, &resetTrue, 1, nullptr)

There is no easy equivalent for the IOP JIT (It can just be added to an event test I think), but I was wondering if there were any thoughts on this?

@F0bes
Copy link
Member Author

F0bes commented Oct 19, 2023

I've expanded this to support resetting the IOP recompiler. I don't plan on doing the VUs.

@stenzek
Copy link
Contributor

stenzek commented Oct 19, 2023

Not really a fan of adding another check to an already-hot function (event test). I am going to add memory protection for code changes for the IOP at some point (writes are very slow ATM), but not just yet.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Oct 19, 2023

Not really a fan of adding another check to an already-hot function (event test). I am going to add memory protection for code changes for the IOP at some point (writes are very slow ATM), but not just yet.

but isn't the problem? that memory protection doesn't trigger from external programs, which is what started this stuff, people using pine/cheat engine to modify stuff in the recompiled code and it not clearing the block.

@stenzek
Copy link
Contributor

stenzek commented Oct 20, 2023

If the concern is just to write to memory externally, then I'd propose mapping another copy EEMem for the exports which doesn't have the page protection changed. But that won't invalidate rec pages if you change code.

Alternatively, as I've suggested before, the shared memory file can be mapped by other processes to bring PCSX2's memory into their address space, then you don't need to mess with other processes' memory in the first place.

IMO, exporting the flags isn't a great idea. It's error prone because the whole thing is a race, the EE thread could be doing anything while the variable changes - recompiling a block or otherwise. Same deal with changing the page's protection - if you're making it read/write, and the game is running, then we might miss a page fault from the game itself. Or PCSX2 might be switching from protected pages to manual block checks, and in the meantime, you've flicked it back to read-only. You could pause the PCSX2 process while mutating the memory, but then how do you know you haven't paused it in the middle of either of these things happening?

At least when using PINE/IPC, we have some control over when the writes occur (although it may be on another thread currently, if so, it should be queued to happen on the EE thread instead, or the socket read moved to the EE thread).

@F0bes
Copy link
Member Author

F0bes commented Oct 20, 2023

I'm aware there are dangers behind it. If we don't want to provide a way to flush the blocks externally then maybe it's not wise to export EEMem anymore. There are otherwise no indicators that "hey, this works for data but not code."

@stenzek
Copy link
Contributor

stenzek commented Oct 20, 2023

Yeah, it's a tricky one. I'm not really sure what the "ideal" solution is, other than to go through IPC.

If I remember correctly, having the memory export was a consolation prize for those who used Cheat Engine and previously relied on PCSX2 always mapping memory at a fixed location (back in 1.6). Whether you used the fixed location, or the export, you'd still run into issues with the memory being write-protected.

Funnily enough, the current behaviour at least tells you if you're writing to a code page, and it's not going to recompile, since the WriteProcessMemory() call should fail.

Personally, I'd be all for dropping the export and making everyone use the safe mechanisms. Better than having some third-party thing messing with our state, then the users blaming us when PCSX2 crashes. And since it's not going to be a module which gets injected into our process, it won't show up in crash logs, making support very difficult, unless users are upfront about it (plot spoiler, they're not). But if we did remove it, there'd be outrage.

So I can see three possible solutions:

  1. Drop the exports, make everyone use IPC, get raged at and abused.
  2. Map a second copy of EEmem for export, and have an option to always use manual protection for rec blocks. That way any code changes will be visible, and the appropriate blocks will be recompiled the next time they're executed, rather than resetting the whole rec. Display a warning on startup, because doing so will hurt performance (difficult to guess how much, I'd wager maybe 10% tops, but would have to benchmark). Also can say "no support if you enable this ;)". Price you pay for being able to use Cheat Engine.
  3. Make the memory export valid contingent on the above option, otherwise leave it as NULL, that way we don't end up with the can-change-data-but-not-code situation.

@F0bes
Copy link
Member Author

F0bes commented Oct 20, 2023

Not really a fan of adding another check to an already-hot function (event test). I am going to add memory protection for code changes for the IOP at some point (writes are very slow ATM), but not just yet.

Just wanted to touch on this. 99.9% of the time iopRecNeedsReset is going to be false. I wouldn't expect it to be that hot, assuming the user has a CPU with a functional branch predictor. I could even add [[unlikely]], although I don't know how much that'll change the codegen.

Racing is still an issue though.

@F0bes F0bes closed this Mar 7, 2024
@lightningterror lightningterror deleted the eemem-write-reset branch March 12, 2024 14:51
@xCENTx
Copy link
Contributor

xCENTx commented May 14, 2024

Hope this is of no trouble commenting on a closed PR. I would like to include additional information and how this can be fixed by developers of their individual resources.

it is possible to call recResetEE to trigger a recompile of EE. Though a user would need to create a remote thread to execute this function as well as provide the function prototype and address of the function.

I personally am not a fan of using PINE to access process memory , please do not force the usage of pine for process memory operations, pretty please. That being said , so long as EEMemory is made a static variable .. if the export is removed , users will still be able to obtain an instance of EEmemory

Example of Resetting EEMemory via C++ DLL
(this can also be done externally a developer just needs to create a thread in the process to execute the function and then exit thread. can also be done with cheat engine using lua)

// Declare function prototype
typedef void(__fastcall* recResetEE_stub)();

// Example Function 
void ResetEE()
{
	static __int64 pAddr = Memory::SignatureScan("80 3D ? ? ? ? ? 75 ? C6 05 ? ? ? ? ? C6 05 ? ? ? ? ? 80 3D");
	if (!pAddr)
		return;

	static recResetEE_stub fn = reinterpret_cast<recResetEE_stub>(pAddr);

        // Execute function
	fn();
}

@JordanTheToaster
Copy link
Member

I personally am not a fan of using PINE to access process memory , please do not force the usage of pine for process memory operations, pretty please.

If you do not wish to use PINE you can quite easily make a fork and do it your own way.

@xCENTx
Copy link
Contributor

xCENTx commented May 14, 2024

I personally am not a fan of using PINE to access process memory , please do not force the usage of pine for process memory operations, pretty please.

If you do not wish to use PINE you can quite easily make a fork and do it your own way

Nah , I'd just get a static reference to EE memory and be on my way lol. No need to deal with another library when WriteProcessMemory and ReadProcessMemory exist. Too much overhead when most of us using EEmem do is access and manipulate memory. EEmem is a static variable that can be obtained whether it is exported or not. Its a variable that has a predefined address in the data segment of the module, which for the time being also lives in the export table.

@TheLastRar
Copy link
Contributor

TheLastRar commented May 14, 2024

IIRC, A fair amount of effort was put into optimising PINE to be fast
Have you run into an issue where the overhead was causing problems?

Edit: You would probably have to use the batching functions to get the most performance out of PINE

@xCENTx
Copy link
Contributor

xCENTx commented May 14, 2024

IIRC, A fair amount of effort was put into optimising PINE to be fast Have you run into an issue where the overhead was causing problems?

I've just found it to be easier to compile a dll to manage memory 1:1 with the executing process as opposed to all the hoops one needs to jump through to get an instance of pine working. There's also the issue with builds requiring knowledge of binary ninja and meson, which tbh is not very straightforward , not examples available that I could find.

Pine very well could be fast but its never be explained to me how it does anything I cant already do with direct reference to memory address in the process and I've just had countless issues trying to get an instance to build or run. I also do not think it will come close to matching the data exchange speed you'd get as an module internal the process but I have not managed to get an instance of pine to run so that I could do a direct comparison of clock speeds in regards to how fast memory is read and wrote via pine. I'm not saying pine is bad, I'm just saying I don't use it and providing the reasons as to why. Im a game hacker and most of us deal with memory in a typical fashion. Open handle to process , get module base, apply offset to module base, profit.

Anyways, to bring this discussion back to the the meat of my comment, I was offering a method of recompiling EE so that patches applied "externally" via EEmem will stick as the memory gets recompiled, Which is something I do in my framework. I will not make mention of pine again , it seems to be a soft spot around here, i was just hopeful that pine enforcement was not being considered ( as in hiding member variables so they are no longer accessible without use of pine )

@stenzek
Copy link
Contributor

stenzek commented May 15, 2024

Sigh.

(this can also be done externally a developer just needs to create a thread in the process to execute the function and then exit thread. can also be done with cheat engine using lua)

No, no, and no. This is a massive race condition and will corrupt memory at best, or just outright crash unless the EE thread happens to be asleep when you do so.

@xCENTx
Copy link
Contributor

xCENTx commented May 15, 2024

Sigh.

(this can also be done externally a developer just needs to create a thread in the process to execute the function and then exit thread. can also be done with cheat engine using lua)

No, no, and no. This is a massive race condition and will corrupt memory at best, or just outright crash unless the EE thread happens to be asleep when you do so.

I've personally never once crashed and I reset EEmem at the click of an ImGui widget button. That being said, You are much more knowledgeable than I and respect the work you do so I will not argue the point. Thank you for explaining to me something I did not fully understand previously. I was however under the impressesion that recResetEE() verifies if EE is currently executing and would manage it in a clean and efficient way as to not disrupt execution flow. Not arguing the point , really just trying to understand better :)

static void recResetEE()
{
if (eeCpuExecuting)
{
// get outta here as soon as we can
eeRecNeedsReset = true;
recSafeExitExecution();
return;
}
recResetRaw();
}

@stenzek
Copy link
Contributor

stenzek commented May 15, 2024

The only thread you can call that function from is the EE thread. It's also static (internal), so subject to both inlining, and it's not meant for external consumption.

@xCENTx
Copy link
Contributor

xCENTx commented May 15, 2024

The only thread you can call that function from is the EE thread. It's also static (internal), so subject to both inlining, and it's not meant for external consumption.

To be fair I don't think anything in process memory is meant for external consumption in terms of game hacking , at the end of the day its up to the cheat / mod developer to find their own entry point and manage the process the way they best see fit. I personally haven't attempted to call the function via lua in cheat engine so maybe I should not have mentioned that. I have externally injected a dll, called the function, exited thread and watched as my patch did nothing until memory was recompiled. The patch showed in memory but was not reflected in the game world. I only have my hands on experience to go on as I do not understand the complete internals of the emulator like the devs who work on this every day.

There's also a few settings one can toggle in the settings menu to natively invoke this function I suppose ( this is how I came to find the function responsible for applying patches or recompiling memory after I changed memory )

I guess this is why it's called game hacking , because the dev team responsible for releasing the product would cry in agony seeing some of the ways people hook and utilize in game functions just to apply changes

again , I appreciate your insight and hope that this discussion is not bothering you in any way

@stenzek
Copy link
Contributor

stenzek commented May 15, 2024

I have externally injected a dll, called the function, exited thread and watched as my patch did nothing until memory was recompiled.

That's great, until you hit the small window of time when both the EE thread and your thread are executing that function, modifying state, and making a dog's breakfast of it.

Anyway. I'm done here, do whatever, so long as we don't get users complaining about random memory corruption-caused crashes, due to your sloppy patches.

@PCSX2 PCSX2 locked as off-topic and limited conversation to collaborators May 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants