diff --git a/Source/Windows/ARM64EC/Module.cpp b/Source/Windows/ARM64EC/Module.cpp index d15d0676c..5e35f565c 100644 --- a/Source/Windows/ARM64EC/Module.cpp +++ b/Source/Windows/ARM64EC/Module.cpp @@ -91,6 +91,7 @@ uint64_t WineNtProtectVirtualMemorySyscallId; NTSTATUS NtContinueNative(ARM64_NT_CONTEXT* NativeContext, BOOLEAN Alert); NTSTATUS NtAllocateVirtualMemoryNative(HANDLE, PVOID*, ULONG_PTR, SIZE_T*, ULONG, ULONG); NTSTATUS NtProtectVirtualMemoryNative(HANDLE, PVOID*, SIZE_T*, ULONG, ULONG*); +static fextl::string AppConfigName {}; [[noreturn]] void JumpSetStack(uintptr_t PC, uintptr_t SP); @@ -583,12 +584,12 @@ NTSTATUS ProcessInit() { FEX::Windows::InitCRTProcess(); FEX::Windows::SetupThreadHandlers(); const auto ExecutablePath = FEX::Windows::GetExecutableFilePath(); - const auto ExecutableName = FEX::Windows::BaseName(ExecutablePath); - FEX::Config::LoadConfig(fextl::string {ExecutableName}, _environ, FEX::ReadPortabilityInformation()); + AppConfigName = FEX::Windows::BaseName(ExecutablePath); + FEX::Config::LoadConfig(AppConfigName, _environ, FEX::ReadPortabilityInformation()); FEXCore::Config::ReloadMetaLayer(); FEX::Windows::Logging::Init(); FEXCore::Config::Set(FEXCore::Config::CONFIG_APP_FILENAME, ExecutablePath); - FEXCore::Config::Set(FEXCore::Config::CONFIG_APP_CONFIG_NAME, ExecutableName); + FEXCore::Config::Set(FEXCore::Config::CONFIG_APP_CONFIG_NAME, AppConfigName); FEXCore::Config::Set(FEXCore::Config::CONFIG_IS64BIT_MODE, "1"); @@ -644,8 +645,8 @@ NTSTATUS ProcessInit() { StatAllocHandler = fextl::make_unique(FEXCore::SHMStats::AppType::WIN_ARM64EC); } - if (StartupSleep() && (StartupSleepProcName().empty() || ExecutableName == StartupSleepProcName())) { - LogMan::Msg::IFmt("[{}][{}] Sleeping for {} seconds", GetCurrentProcessId(), ExecutableName, StartupSleep()); + if (StartupSleep() && (StartupSleepProcName().empty() || AppConfigName == StartupSleepProcName())) { + LogMan::Msg::IFmt("[{}][{}] Sleeping for {} seconds", GetCurrentProcessId(), AppConfigName, StartupSleep()); std::this_thread::sleep_for(std::chrono::seconds(StartupSleep())); } @@ -894,22 +895,50 @@ void BTCpu64NotifyReadFile(HANDLE Handle, void* Address, SIZE_T Size, BOOL After return; } - auto& InLockedRWXRead = GetFrontendThreadData(ThreadState)->InLockedRWXRead; - if (!After) { - ThreadCreationMutex.lock(); - CTX->GetCodeInvalidationMutex().lock(); - if (InvalidationTracker->BeginUntrackedWriteLocked(reinterpret_cast(Address), static_cast(Size))) { - InLockedRWXRead = true; + // On file read this can cause JIT code invalidations. FEX previously used to hold locks on `CodeInvalidationMutex` and `ThreadCreationMutex` during + // this. This was a problem that file reads are blocking and may necessarily never return. A simple unit-test that holds a blocking read, then tries + // to create threads or JIT code would immediately hang. + // + // - PhysX msiexec installer (From Dishonored AppID 205100) did exactly this and hung forever. + // + // The alternative approach is to let the read progress as normal and only notify invalidation when the read actually returns. This introduces an + // unavoidable race which by my logic is fine. + // + // 1) If a thread is already jitting code from the address range then in a "real" x86 environment it may see old, new, or a combination of both. + // - NtReadFile isn't guaranteed to return new data atomically, in order, or anything of the sort. + // 2) The calling thread only ever sees the before code, and the after code because `NtReadFile` never leaves "kernel" space before invalidation. + // + // The only "gotcha" is that FEX could have write-protected the memory region when a JIT executed the code, and once the `NtReadFile` tries + // returning data, it faults. This requires coordination between Wine and FEX in order to handle this edge case. + // + // Additionally, async reads are still completely untracked with invalidation, which never worked before anyway. + + if (AppConfigName == "P5R.exe") { + // Use the locking mechanism for (Persona 5 Royal AppID 1687950) which does exactly the problem described above. + auto& InLockedRWXRead = GetFrontendThreadData(ThreadState)->InLockedRWXRead; + if (!After) { + ThreadCreationMutex.lock(); + CTX->GetCodeInvalidationMutex().lock(); + if (InvalidationTracker->BeginUntrackedWriteLocked(reinterpret_cast(Address), static_cast(Size))) { + InLockedRWXRead = true; + } else { + CTX->GetCodeInvalidationMutex().unlock(); + ThreadCreationMutex.unlock(); + } } else { - CTX->GetCodeInvalidationMutex().unlock(); - ThreadCreationMutex.unlock(); - } - } else { - if (InLockedRWXRead) { - InLockedRWXRead = false; - CTX->GetCodeInvalidationMutex().unlock(); - ThreadCreationMutex.unlock(); + if (InLockedRWXRead) { + InLockedRWXRead = false; + CTX->GetCodeInvalidationMutex().unlock(); + ThreadCreationMutex.unlock(); + } } + + return; + } + + if (After && Status == STATUS_SUCCESS) { + std::scoped_lock Lock(ThreadCreationMutex); + InvalidationTracker->InvalidateAlignedInterval(reinterpret_cast(Address), static_cast(Size), false); } } diff --git a/Source/Windows/WOW64/Module.cpp b/Source/Windows/WOW64/Module.cpp index 0fca8574d..b936ce303 100644 --- a/Source/Windows/WOW64/Module.cpp +++ b/Source/Windows/WOW64/Module.cpp @@ -1034,22 +1034,15 @@ void BTCpuNotifyUnmapViewOfSection(void* Address, BOOL After, ULONG Status) { } void BTCpuNotifyReadFile(HANDLE Handle, void* Address, SIZE_T Size, BOOL After, NTSTATUS Status) { - auto& InLockedRWXRead = GetFrontendThreadData(GetTLS().ThreadState())->InLockedRWXRead; - if (!After) { - ThreadCreationMutex.lock(); - CTX->GetCodeInvalidationMutex().lock(); - if (InvalidationTracker->BeginUntrackedWriteLocked(reinterpret_cast(Address), static_cast(Size))) { - InLockedRWXRead = true; - } else { - CTX->GetCodeInvalidationMutex().unlock(); - ThreadCreationMutex.unlock(); - } - } else { - if (InLockedRWXRead) { - InLockedRWXRead = false; - CTX->GetCodeInvalidationMutex().unlock(); - ThreadCreationMutex.unlock(); - } + auto* ThreadState = GetTLS().ThreadState(); + if (!InvalidationTracker || !ThreadState) { + return; + } + + // See arm64ec `BTCpu64NotifyReadFile` for why this like this. + if (After && Status == STATUS_SUCCESS) { + std::scoped_lock Lock(ThreadCreationMutex); + InvalidationTracker->InvalidateAlignedInterval(reinterpret_cast(Address), static_cast(Size), false); } }