fix: roll back earlier host allocations on later gap failure in TryBackFixedRange (#472) (#474)

When a fixed mapping spans multiple free runs and a later gap cannot be
backed, any earlier host allocations were leaked. Stage all allocations
during the walk and insert MemoryRegions only after every gap has been
backed successfully. On any failure, free all staged allocations.

Fixes #472

🤖 Generated with Hermes Agent

src/SharpEmu.Core/Memory/PhysicalVirtualMemory.cs

@@ -446,13 +446,20 @@ public sealed unsafe class PhysicalVirtualMemory : IVirtualMemory, IGuestMemoryA
         // us over whole free or occupied stretches. Only free stretches get backed;
         // stretches already reserved or committed by another allocation are left as
         // they are, which is exactly what a fixed mapping does on hardware.
+        //
+        // Because backing may span several disjoint free runs, allocations are
+        // staged: host pages are reserved/committed first, and the corresponding
+        // MemoryRegions are inserted only once every gap in the range has been
+        // backed. If any gap fails to back, every earlier host allocation is freed
+        // and no region is inserted, so the address space is left untouched.
+        var stagedAllocations = new List<(ulong Address, ulong Size)>();
+
         var cursor = start;
-        var backedAny = false;
         while (cursor < end)
         {
             if (!_hostMemory.Query(cursor, out var info))
             {
-                return false;
+                goto Rollback;
             }
 
             var queriedEnd = info.RegionSize > ulong.MaxValue - info.BaseAddress
@@ -461,7 +468,7 @@ public sealed unsafe class PhysicalVirtualMemory : IVirtualMemory, IGuestMemoryA
             var runEnd = Math.Min(end, queriedEnd);
             if (runEnd <= cursor)
             {
-                return false;
+                goto Rollback;
             }
 
             if (info.State == HostRegionState.Free)
@@ -475,35 +482,52 @@ public sealed unsafe class PhysicalVirtualMemory : IVirtualMemory, IGuestMemoryA
                         _hostMemory.Free(allocated);
                     }
 
-                    return false;
+                    goto Rollback;
-                }
-
-                var protection = executable ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE;
-                _gate.EnterWriteLock();
-                try
-                {
-                    InsertRegionSorted(new MemoryRegion
-                    {
-                        VirtualAddress = cursor,
-                        Size = runSize,
-                        IsExecutable = executable,
-                        IsReservedOnly = false,
-                        Protection = protection
-                    });
-                }
-                finally
-                {
-                    _gate.ExitWriteLock();
                 }
 
+                stagedAllocations.Add((cursor, runSize));
                 TraceVmem($"Backed fixed range gap: 0x{cursor:X16} - 0x{runEnd:X16} ({runSize} bytes)");
-                backedAny = true;
             }
 
             cursor = runEnd;
         }
 
-        return backedAny;
+        if (stagedAllocations.Count == 0)
+        {
+            return false;
+        }
+
+        // All gaps backed successfully — insert regions in one batch.
+        var protection = executable ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE;
+        _gate.EnterWriteLock();
+        try
+        {
+            foreach (var (gapAddress, gapSize) in stagedAllocations)
+            {
+                InsertRegionSorted(new MemoryRegion
+                {
+                    VirtualAddress = gapAddress,
+                    Size = gapSize,
+                    IsExecutable = executable,
+                    IsReservedOnly = false,
+                    Protection = protection
+                });
+            }
+        }
+        finally
+        {
+            _gate.ExitWriteLock();
+        }
+
+        return true;
+
+    Rollback:
+        foreach (var (gapAddress, _) in stagedAllocations)
+        {
+            _hostMemory.Free(gapAddress);
+        }
+
+        return false;
     }
 
     public bool TryAllocateAtOrAbove(

tests/SharpEmu.Libs.Tests/Memory/GuestMemoryAllocatorTests.cs

@@ -121,6 +121,27 @@ public sealed class GuestMemoryAllocatorTests
         Assert.Equal([alignedAddress + 0x1000], host.FreedAddresses);
     }
 
+    [Fact]
+    public void TryBackFixedRangeRollsBackEarlierGapsWhenLaterGapCannotBeBacked()
+    {
+        // Layout: committed | free | committed | free
+        // First free gap allocates successfully, second fails.
+        // The first allocation must be freed — nothing should leak.
+        const ulong rangeBase = 0x0000_0020_2F00_0000;
+        const ulong rangeSize = 0x4000;
+        using var host = new GappedHostMemory(rangeBase);
+        using var memory = new PhysicalVirtualMemory(host);
+
+        Assert.False(memory.TryBackFixedRange(rangeBase, rangeSize, executable: false));
+
+        // First gap allocates successfully; second gap is attempted and fails.
+        Assert.Equal(
+            [(rangeBase + 0x1000, 0x1000), (rangeBase + 0x3000, 0x1000)],
+            host.AllocationCalls);
+        // First gap must have been freed during rollback — nothing leaks.
+        Assert.Equal([rangeBase + 0x1000], host.FreedAddresses);
+    }
+
     [Fact]
     public void TryBackFixedRangeFillsOnlyTheFreePagesOfAPartiallyOccupiedRange()
     {
@@ -228,6 +249,108 @@ public sealed class GuestMemoryAllocatorTests
         }
     }
 
+    private sealed class GappedHostMemory : IHostMemory, IDisposable
+    {
+        // Layout (4 × 0x1000 pages):
+        //   [committed] [free] [committed] [free]
+        // Allocate succeeds for the first free gap, fails for the second.
+        private readonly ulong _base;
+        private readonly ulong _firstGapStart;
+        private readonly ulong _firstGapEnd;
+        private readonly ulong _secondGapStart;
+        private readonly ulong _secondGapEnd;
+        private readonly ulong _end;
+
+        public GappedHostMemory(ulong @base)
+        {
+            _base = @base;
+            _firstGapStart = @base + 0x1000;
+            _firstGapEnd = @base + 0x2000;
+            _secondGapStart = @base + 0x3000;
+            _secondGapEnd = @base + 0x4000;
+            _end = @base + 0x4000;
+        }
+
+        public List<(ulong Address, ulong Size)> AllocationCalls { get; } = [];
+        public List<ulong> FreedAddresses { get; } = [];
+
+        public ulong Allocate(ulong desiredAddress, ulong size, HostPageProtection protection)
+        {
+            AllocationCalls.Add((desiredAddress, size));
+
+            // First free gap — succeed.
+            if (desiredAddress == _firstGapStart && size == 0x1000)
+            {
+                return desiredAddress;
+            }
+
+            // Second free gap — fail to trigger rollback.
+            return 0;
+        }
+
+        public ulong Reserve(ulong desiredAddress, ulong size, HostPageProtection protection) => 0;
+
+        public bool Commit(ulong address, ulong size, HostPageProtection protection) => true;
+
+        public bool Free(ulong address)
+        {
+            FreedAddresses.Add(address);
+            return true;
+        }
+
+        public bool Protect(ulong address, ulong size, HostPageProtection protection, out uint rawOldProtection)
+        {
+            rawOldProtection = 0;
+            return true;
+        }
+
+        public bool ProtectRaw(ulong address, ulong size, uint rawProtection, out uint rawOldProtection)
+        {
+            rawOldProtection = 0;
+            return true;
+        }
+
+        public bool Query(ulong address, out HostRegionInfo info)
+        {
+            if (address < _firstGapStart)
+            {
+                // First block: committed.
+                info = new HostRegionInfo(address, _base, _firstGapStart - address,
+                    HostRegionState.Committed, RawState: 0x1000,
+                    HostPageProtection.ReadWrite, RawProtection: 0x04, RawAllocationProtection: 0x04);
+                return true;
+            }
+
+            if (address < _firstGapEnd)
+            {
+                // Second block: free (first gap).
+                info = new HostRegionInfo(address, AllocationBase: 0, _firstGapEnd - address,
+                    HostRegionState.Free, RawState: 0x10000,
+                    HostPageProtection.NoAccess, RawProtection: 0x01, RawAllocationProtection: 0);
+                return true;
+            }
+
+            if (address < _secondGapStart)
+            {
+                // Third block: committed.
+                info = new HostRegionInfo(address, _base + 0x2000, _secondGapStart - address,
+                    HostRegionState.Committed, RawState: 0x1000,
+                    HostPageProtection.ReadWrite, RawProtection: 0x04, RawAllocationProtection: 0x04);
+                return true;
+            }
+
+            // Fourth block: free (second gap — this one will fail to allocate).
+            info = new HostRegionInfo(address, AllocationBase: 0, _end - address,
+                HostRegionState.Free, RawState: 0x10000,
+                HostPageProtection.NoAccess, RawProtection: 0x01, RawAllocationProtection: 0);
+            return true;
+        }
+
+        public void FlushInstructionCache(ulong address, ulong size) { }
+
+        public void Dispose() { }
+    }
+
     private sealed class FakeHostMemory : IHostMemory
     {
         public ulong Allocate(ulong desiredAddress, ulong size, HostPageProtection protection) =>