diff --git a/src/SharpEmu.Core/Memory/PhysicalVirtualMemory.cs b/src/SharpEmu.Core/Memory/PhysicalVirtualMemory.cs index d609541..0781356 100644 --- a/src/SharpEmu.Core/Memory/PhysicalVirtualMemory.cs +++ b/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; - } - - 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(); + goto Rollback; } + 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( diff --git a/tests/SharpEmu.Libs.Tests/Memory/GuestMemoryAllocatorTests.cs b/tests/SharpEmu.Libs.Tests/Memory/GuestMemoryAllocatorTests.cs index a0a8307..807c4c5 100644 --- a/tests/SharpEmu.Libs.Tests/Memory/GuestMemoryAllocatorTests.cs +++ b/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 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) =>