Merge #939 (pr/import-patch-serialization)

src/SharpEmu.Core/Cpu/Native/DirectExecutionBackend.cs

@@ -2339,7 +2339,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<int, nint> _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);
@@ -2515,8 +2539,35 @@ 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)
+	{
+		// 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<byte> 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<byte>((void*)address, 16).SequenceEqual(patch))
+		{
+			return true;
+		}
+
 		uint flNewProtect = default(uint);
 		if (!VirtualProtect((void*)address, 16u, 64u, &flNewProtect))
 		{
@@ -2553,6 +2604,7 @@ public sealed unsafe partial class DirectExecutionBackend : INativeCpuBackend, I
 			}
 		}
 		_importHandlerTrampolines.Clear();
+		_importHandlerTrampolineByIndex.Clear();
 	}
 
 	private unsafe void CreateTlsHandler()
@@ -3705,6 +3757,16 @@ public sealed unsafe partial class DirectExecutionBackend : INativeCpuBackend, I
 	}
 
 	private static unsafe bool WriteTlsInstruction(nint address, ReadOnlySpan<byte> 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<byte> replacement)
 	{
 		if (replacement.Length is < 1 or > 15 ||
 			VirtualQuery((void*)address, out var information, (nuint)sizeof(MEMORY_BASIC_INFORMATION64)) == 0 ||

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<nint>());
+        SetPrivateField(backend, "_importHandlerTrampolines", new List<nint>());
+        SetPrivateField(backend, "_importHandlerTrampolineByIndex", new Dictionary<int, nint>());
 
         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<byte> code, ReadOnlySpan<byte> expected)
     {
         Assert.True(