From 829dc815d10a9c2353c91fb6f44c6edf836159be Mon Sep 17 00:00:00 2001 From: kuba Date: Tue, 29 Sep 2026 23:19:20 +0200 Subject: [PATCH 1/2] [CPU] Serialize import stub and lazy TLS site patches Patching a guest code site unprotects the page it lives on, writes, and restores the old protection. Two threads patching sites that share a page therefore race: one restores the page to read-execute while the other is still writing into it, and the second write faults or is lost. Both happen in practice -- a runtime module load sets up import stubs while guest threads are running, and guest threads patch their own lazy TLS access sites as they first reach them, several of which land on one page. Every import stub patch and every lazy TLS site patch now runs under a single gate, so the unprotect/write/restore sequence is never interleaved. --- .../Cpu/Native/DirectExecutionBackend.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs b/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs index 3af1142..5638f07 100644 --- a/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs +++ b/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs @@ -2505,7 +2505,20 @@ public sealed unsafe partial class DirectExecutionBackend : INativeCpuBackend, I } } + // Patches unprotect and re-protect whole pages: two threads patching code that shares a page + // (runtime module loads, lazy TLS site patches on guest threads) would restore the other's page + // to read-execute while it is still writing, so every patch runs under one gate. + private static readonly object ImportStubPatchGate = new(); + private unsafe bool PatchImportStub(nint address, nint trampoline) + { + lock (ImportStubPatchGate) + { + return PatchImportStubLocked(address, trampoline); + } + } + + private unsafe bool PatchImportStubLocked(nint address, nint trampoline) { uint flNewProtect = default(uint); if (!VirtualProtect((void*)address, 16u, 64u, &flNewProtect)) @@ -3695,6 +3708,16 @@ public sealed unsafe partial class DirectExecutionBackend : INativeCpuBackend, I } private static unsafe bool WriteTlsInstruction(nint address, ReadOnlySpan replacement) + { + // Guest threads patch their TLS sites as they first reach them; two sites on one page would + // otherwise race the unprotect/restore pair exactly like the import stubs. + lock (ImportStubPatchGate) + { + return WriteTlsInstructionLocked(address, replacement); + } + } + + private static unsafe bool WriteTlsInstructionLocked(nint address, ReadOnlySpan replacement) { if (replacement.Length is < 1 or > 15 || VirtualQuery((void*)address, out var information, (nuint)sizeof(MEMORY_BASIC_INFORMATION64)) == 0 || From ae7ca8dfbe774d99f79213eb8e19af5d0d41fb69 Mon Sep 17 00:00:00 2001 From: kuba Date: Wed, 30 Sep 2026 00:11:15 +0200 Subject: [PATCH 2/2] [CPU] Reuse import trampolines and leave patched stubs alone on module reloads Every runtime module load sets up the import stubs of all loaded modules again, and each setup built a fresh trampoline and rewrote the stub's 16 bytes -- under guest threads that may be executing exactly those bytes. A trampoline only encodes its import index, so the trampolines are now cached by index and reused, and a stub that already jumps to the trampoline it would be given is left untouched, with no unprotect/write/restore cycle at all. A reload of a module whose stubs are already patched therefore writes no guest code. The trampoline ABI test builds its backend with GetUninitializedObject, which skips field initializers, so it has to supply the new cache; both fields now go through one helper that names a field it cannot find. --- .../Cpu/Native/DirectExecutionBackend.cs | 39 +++++++++++++++++++ .../Cpu/ImportTrampolineAbiTests.cs | 19 ++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs b/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs index 5638f07..a09f3f6 100644 --- a/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs +++ b/src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs @@ -2329,7 +2329,31 @@ public sealed unsafe partial class DirectExecutionBackend : INativeCpuBackend, I } } + // A trampoline only encodes its import index, so a runtime module load that sets up every + // module's stubs again reuses them: the stubs then keep their bytes and are not rewritten + // under guest threads that may be executing them. + private readonly Dictionary _importHandlerTrampolineByIndex = new(); + private unsafe nint CreateImportHandlerTrampoline(int importIndex) + { + lock (ImportStubPatchGate) + { + if (_importHandlerTrampolineByIndex.TryGetValue(importIndex, out var existing)) + { + return existing; + } + + var created = CreateImportHandlerTrampolineCore(importIndex); + if (created != 0) + { + _importHandlerTrampolineByIndex[importIndex] = created; + } + + return created; + } + } + + private unsafe nint CreateImportHandlerTrampolineCore(int importIndex) { const uint stubSize = 1024u; void* ptr = VirtualAlloc(null, stubSize, 12288u, 64u); @@ -2520,6 +2544,20 @@ public sealed unsafe partial class DirectExecutionBackend : INativeCpuBackend, I private unsafe bool PatchImportStubLocked(nint address, nint trampoline) { + // Every runtime module load sets up the stubs of all loaded modules again; a stub that + // already jumps to this trampoline needs no unprotect/write/restore cycle. + Span patch = stackalloc byte[16]; + patch[0] = 0x48; + patch[1] = 0xB8; + System.Buffers.Binary.BinaryPrimitives.WriteInt64LittleEndian(patch[2..], trampoline); + patch[10] = 0xFF; + patch[11] = 0xE0; + patch[12..].Fill(0x90); + if (new ReadOnlySpan((void*)address, 16).SequenceEqual(patch)) + { + return true; + } + uint flNewProtect = default(uint); if (!VirtualProtect((void*)address, 16u, 64u, &flNewProtect)) { @@ -2556,6 +2594,7 @@ public sealed unsafe partial class DirectExecutionBackend : INativeCpuBackend, I } } _importHandlerTrampolines.Clear(); + _importHandlerTrampolineByIndex.Clear(); } private unsafe void CreateTlsHandler() diff --git a/tests/SharpEmu.Libs.Tests/Cpu/ImportTrampolineAbiTests.cs b/tests/SharpEmu.Libs.Tests/Cpu/ImportTrampolineAbiTests.cs index 5574abf..16d7990 100644 --- a/tests/SharpEmu.Libs.Tests/Cpu/ImportTrampolineAbiTests.cs +++ b/tests/SharpEmu.Libs.Tests/Cpu/ImportTrampolineAbiTests.cs @@ -81,13 +81,13 @@ public sealed class ImportTrampolineAbiTests private static unsafe byte[] CreateTrampolineBytes() { + // GetUninitializedObject skips the field initializers, so every reference + // field the emitter touches has to be supplied here, including the + // per-index trampoline cache. var backend = (DirectExecutionBackend)RuntimeHelpers.GetUninitializedObject( typeof(DirectExecutionBackend)); - var trampolineList = typeof(DirectExecutionBackend).GetField( - "_importHandlerTrampolines", - BindingFlags.Instance | BindingFlags.NonPublic); - Assert.NotNull(trampolineList); - trampolineList.SetValue(backend, new List()); + SetPrivateField(backend, "_importHandlerTrampolines", new List()); + SetPrivateField(backend, "_importHandlerTrampolineByIndex", new Dictionary()); var createTrampoline = typeof(DirectExecutionBackend).GetMethod( "CreateImportHandlerTrampoline", @@ -106,6 +106,15 @@ public sealed class ImportTrampolineAbiTests } } + private static void SetPrivateField(DirectExecutionBackend backend, string name, object value) + { + var field = typeof(DirectExecutionBackend).GetField( + name, + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.True(field is not null, $"DirectExecutionBackend has no field '{name}'."); + field!.SetValue(backend, value); + } + private static void AssertContains(ReadOnlySpan code, ReadOnlySpan expected) { Assert.True(