From 41d00c8dc65686a596dfd87e7449f9f0bf130bce Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sun, 30 Apr 2023 14:42:52 -0700 Subject: [PATCH] SignalDelegator: Make sure to save and restore `InSyscallInfo` Fixes #2560 This was a forgotten member of the context that needs to be saved and restored when jumping around the signal state. When FEX was receiving signals back to back, there was a chance that the signals would ride the edge of having set `InSyscallInfo` in the JIT, which meant the SRA state would get saved once, then another signal would occur with the previous SRA data, saving SRA again. Then when unwinding the frames it would corrupt the SRA registers. This would result in trying to load SRA state that was no longer valid, looking like a crash in the JIT that was hard to see what happens. Should also make Mono games a little less crash happy. Cleans up CPUState memcpy as well, since it was a little weird looking. --- .../LinuxSyscalls/ArchHelpers/MContext.h | 2 ++ .../LinuxSyscalls/SignalDelegator.cpp | 20 +++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Source/Tools/FEXLoader/LinuxSyscalls/ArchHelpers/MContext.h b/Source/Tools/FEXLoader/LinuxSyscalls/ArchHelpers/MContext.h index bc4232430..1026942b1 100644 --- a/Source/Tools/FEXLoader/LinuxSyscalls/ArchHelpers/MContext.h +++ b/Source/Tools/FEXLoader/LinuxSyscalls/ArchHelpers/MContext.h @@ -38,6 +38,7 @@ struct X86ContextBackup { uint64_t GPRs[23]; FEXCore::x86_64::_libc_fpstate FPRState; uint64_t sa_mask; + uint16_t InSyscallInfo; bool FaultToTopAndGeneratedException; // Guest state @@ -64,6 +65,7 @@ struct ArmContextBackup { uint32_t FPCR; __uint128_t FPRs[32]; uint64_t sa_mask; + uint16_t InSyscallInfo; bool FaultToTopAndGeneratedException; // Guest state diff --git a/Source/Tools/FEXLoader/LinuxSyscalls/SignalDelegator.cpp b/Source/Tools/FEXLoader/LinuxSyscalls/SignalDelegator.cpp index 8dcbe6274..905bb2ab7 100644 --- a/Source/Tools/FEXLoader/LinuxSyscalls/SignalDelegator.cpp +++ b/Source/Tools/FEXLoader/LinuxSyscalls/SignalDelegator.cpp @@ -249,7 +249,7 @@ namespace FEX::HLE { // Save guest state // We can't guarantee if registers are in context or host GPRs // So we need to save everything - memcpy(&Context->GuestState, Thread->CurrentFrame, sizeof(FEXCore::Core::CPUState)); + memcpy(&Context->GuestState, &Thread->CurrentFrame->State, sizeof(FEXCore::Core::CPUState)); // Set the new SP ArchHelpers::Context::SetSp(ucontext, NewSP); @@ -258,6 +258,7 @@ namespace FEX::HLE { Context->FPStateLocation = 0; Context->UContextLocation = 0; Context->SigInfoLocation = 0; + Context->InSyscallInfo = 0; // Store fault to top status and then reset it Context->FaultToTopAndGeneratedException = Thread->CurrentFrame->SynchronousFaultData.FaultToTopAndGeneratedException; @@ -350,7 +351,7 @@ namespace FEX::HLE { ArchHelpers::Context::RestoreContext(ucontext, Context); // Reset the guest state - memcpy(Thread->CurrentFrame, &Context->GuestState, sizeof(FEXCore::Core::CPUState)); + memcpy(&Thread->CurrentFrame->State, &Context->GuestState, sizeof(FEXCore::Core::CPUState)); if (Context->UContextLocation) { auto Frame = Thread->CurrentFrame; @@ -384,6 +385,10 @@ namespace FEX::HLE { // If the guest modified the RIP then we need to take special precautions here if (Context->OriginalRIP != guest_uctx->uc_mcontext.gregs[FEXCore::x86_64::FEX_REG_RIP] || Context->FaultToTopAndGeneratedException) { + + // Restore previous `InSyscallInfo` structure. + Frame->InSyscallInfo = Context->InSyscallInfo; + // Hack! Go back to the top of the dispatcher top // This is only safe inside the JIT rather than anything outside of it ArchHelpers::Context::SetPc(ucontext, Config.AbsoluteLoopTopAddressFillSRA); @@ -456,6 +461,9 @@ namespace FEX::HLE { // If the guest modified the RIP then we need to take special precautions here if (Context->OriginalRIP != guest_uctx->sc.ip || Context->FaultToTopAndGeneratedException) { + // Restore previous `InSyscallInfo` structure. + Frame->InSyscallInfo = Context->InSyscallInfo; + // Hack! Go back to the top of the dispatcher top // This is only safe inside the JIT rather than anything outside of it ArchHelpers::Context::SetPc(ucontext, Config.AbsoluteLoopTopAddressFillSRA); @@ -539,6 +547,10 @@ namespace FEX::HLE { // If the guest modified the RIP then we need to take special precautions here if (Context->OriginalRIP != guest_uctx->uc.uc_mcontext.gregs[FEXCore::x86::FEX_REG_EIP] || Context->FaultToTopAndGeneratedException) { + + // Restore previous `InSyscallInfo` structure. + Frame->InSyscallInfo = Context->InSyscallInfo; + // Hack! Go back to the top of the dispatcher top // This is only safe inside the JIT rather than anything outside of it ArchHelpers::Context::SetPc(ucontext, Config.AbsoluteLoopTopAddressFillSRA); @@ -1166,6 +1178,10 @@ namespace FEX::HLE { SpillSRA(Thread, ucontext, IgnoreMask); ContextBackup->Flags |= ArchHelpers::Context::ContextFlags::CONTEXT_FLAG_INJIT; + + // We are leaving the syscall information behind. Make sure to store the previous state. + ContextBackup->InSyscallInfo = Thread->CurrentFrame->InSyscallInfo; + Thread->CurrentFrame->InSyscallInfo = 0; } else { if (!IsAddressInDispatcher(OldPC)) { // This is likely to cause issues but in some cases it isn't fatal