From 4abe3f7932fc532ce20e7aac2bd1bf09c5a42699 Mon Sep 17 00:00:00 2001 From: nattthebear Date: Fri, 15 May 2020 07:40:28 -0400 Subject: [PATCH] Revert 4a5ece2076d3570c13350db4b4 (#2007) Because of intervening commits, there may be some other incidental changes. While well intentioned, the refactoring was just a mess when it came to actually groking this low level memory block shuffling code. --- .../CallingConventionAdapter.cs | 8 ++-- src/BizHawk.BizInvoke/MemoryBlock.cs | 14 +++--- src/BizHawk.BizInvoke/MemoryBlockBase.cs | 46 +++++++++++-------- src/BizHawk.BizInvoke/MemoryBlockUnix.cs | 14 +++--- src/BizHawk.Emulation.Cores/Waterbox/Heap.cs | 16 +++---- .../Waterbox/MapHeap.cs | 22 +++++---- .../Waterbox/PeRunner.cs | 6 +-- .../Waterbox/PeWrapper.cs | 4 +- 8 files changed, 73 insertions(+), 57 deletions(-) diff --git a/src/BizHawk.BizInvoke/CallingConventionAdapter.cs b/src/BizHawk.BizInvoke/CallingConventionAdapter.cs index 83dbe3a527..5770ca6d91 100644 --- a/src/BizHawk.BizInvoke/CallingConventionAdapter.cs +++ b/src/BizHawk.BizInvoke/CallingConventionAdapter.cs @@ -185,20 +185,20 @@ namespace BizHawk.BizInvoke private void WriteThunk(byte[] data, int placeholderIndex, IntPtr p, int index) { - _memory.Protect(_memory.AddressRange.Start, _memory.Size, MemoryBlockBase.Protection.RW); - var ss = _memory.GetStream(_memory.AddressRange.Start + BlockSize * (ulong) index, BlockSize, true); + _memory.Protect(_memory.Start, _memory.Size, MemoryBlockBase.Protection.RW); + var ss = _memory.GetStream(_memory.Start + (ulong)index * BlockSize, BlockSize, true); ss.Write(data, 0, data.Length); for (int i = data.Length; i < BlockSize; i++) ss.WriteByte(Padding); ss.Position = placeholderIndex; var bw = new BinaryWriter(ss); bw.Write((long)p); - _memory.Protect(_memory.AddressRange.Start, _memory.Size, MemoryBlockBase.Protection.RX); + _memory.Protect(_memory.Start, _memory.Size, MemoryBlockBase.Protection.RX); } private IntPtr GetThunkAddress(int index) { - return Z.US(_memory.AddressRange.Start + BlockSize * (ulong) index); + return Z.US(_memory.Start + (ulong)index * BlockSize); } private void SetLifetime(int index, object lifetime) diff --git a/src/BizHawk.BizInvoke/MemoryBlock.cs b/src/BizHawk.BizInvoke/MemoryBlock.cs index 9f4008d41c..e146127c06 100644 --- a/src/BizHawk.BizInvoke/MemoryBlock.cs +++ b/src/BizHawk.BizInvoke/MemoryBlock.cs @@ -37,8 +37,8 @@ namespace BizHawk.BizInvoke 0, 0, Z.UU(Size), - Z.US(AddressRange.Start) - ) != Z.US(AddressRange.Start)) + Z.US(Start) + ) != Z.US(Start)) { throw new InvalidOperationException($"{nameof(Kernel32.MapViewOfFileEx)}() returned NULL"); } @@ -51,7 +51,7 @@ namespace BizHawk.BizInvoke { if (!Active) throw new InvalidOperationException("Not active"); - if (!Kernel32.UnmapViewOfFile(Z.US(AddressRange.Start))) + if (!Kernel32.UnmapViewOfFile(Z.US(Start))) throw new InvalidOperationException($"{nameof(Kernel32.UnmapViewOfFile)}() returned NULL"); Active = false; } @@ -67,12 +67,12 @@ namespace BizHawk.BizInvoke // temporarily switch the entire block to `R`: in case some areas are unreadable, we don't want // that to complicate things Kernel32.MemoryProtection old; - if (!Kernel32.VirtualProtect(Z.UU(AddressRange.Start), Z.UU(Size), Kernel32.MemoryProtection.READONLY, out old)) + if (!Kernel32.VirtualProtect(Z.UU(Start), Z.UU(Size), Kernel32.MemoryProtection.READONLY, out old)) throw new InvalidOperationException($"{nameof(Kernel32.VirtualProtect)}() returned FALSE!"); _snapshot = new byte[Size]; var ds = new MemoryStream(_snapshot, true); - var ss = GetStream(AddressRange.Start, Size, false); + var ss = GetStream(Start, Size, false); ss.CopyTo(ds); XorHash = WaterboxUtils.Hash(_snapshot); @@ -86,9 +86,9 @@ namespace BizHawk.BizInvoke throw new InvalidOperationException("Not active"); // temporarily switch the entire block to `R` Kernel32.MemoryProtection old; - if (!Kernel32.VirtualProtect(Z.UU(AddressRange.Start), Z.UU(Size), Kernel32.MemoryProtection.READONLY, out old)) + if (!Kernel32.VirtualProtect(Z.UU(Start), Z.UU(Size), Kernel32.MemoryProtection.READONLY, out old)) throw new InvalidOperationException($"{nameof(Kernel32.VirtualProtect)}() returned FALSE!"); - var ret = WaterboxUtils.Hash(GetStream(AddressRange.Start, Size, false)); + var ret = WaterboxUtils.Hash(GetStream(Start, Size, false)); ProtectAll(); return ret; } diff --git a/src/BizHawk.BizInvoke/MemoryBlockBase.cs b/src/BizHawk.BizInvoke/MemoryBlockBase.cs index 10dcda123c..359db4c6d4 100644 --- a/src/BizHawk.BizInvoke/MemoryBlockBase.cs +++ b/src/BizHawk.BizInvoke/MemoryBlockBase.cs @@ -1,7 +1,6 @@ using System; using System.IO; using System.Runtime.InteropServices; - using BizHawk.Common; namespace BizHawk.BizInvoke @@ -12,22 +11,28 @@ namespace BizHawk.BizInvoke /// is not aligned or is 0 protected MemoryBlockBase(ulong start, ulong size) { - if (!WaterboxUtils.Aligned(start)) throw new ArgumentOutOfRangeException(nameof(start), start, "start address must be aligned"); - if (size == 0) throw new ArgumentOutOfRangeException(nameof(size), size, "cannot create 0-length block"); + if (!WaterboxUtils.Aligned(start)) + throw new ArgumentOutOfRangeException(nameof(start), start, "start address must be aligned"); + if (size == 0) + throw new ArgumentOutOfRangeException(nameof(size), size, "cannot create 0-length block"); + Start = start; Size = WaterboxUtils.AlignUp(size); - AddressRange = start.RangeToExclusive(start + Size); - _pageData = new Protection[1 + GetPage(AddressRange.EndInclusive)]; + End = Start + Size; + _pageData = new Protection[GetPage(End - 1) + 1]; } /// stores last set memory protection value for each page protected readonly Protection[] _pageData; - /// valid address range of the memory block - public readonly Range AddressRange; + /// ending address of the memory block; equal to + + public readonly ulong End; /// total size of the memory block public readonly ulong Size; + /// starting address of the memory block + public readonly ulong Start; + /// snapshot for XOR buffer protected byte[] _snapshot; @@ -37,31 +42,35 @@ namespace BizHawk.BizInvoke public byte[] XorHash { get; protected set; } /// get a page index within the block - protected int GetPage(ulong addr) => AddressRange.Contains(addr) - ? (int) ((addr - AddressRange.Start) >> WaterboxUtils.PageShift) - : throw new ArgumentOutOfRangeException(nameof(addr), addr, "invalid address"); + protected int GetPage(ulong addr) + { + if (addr < Start || End <= addr) throw new ArgumentOutOfRangeException(); + return (int) ((addr - Start) >> WaterboxUtils.PageShift); + } /// get a start address for a page index within the block - protected ulong GetStartAddr(int page) => AddressRange.Start + ((ulong) page << WaterboxUtils.PageShift); + protected ulong GetStartAddr(int page) => ((ulong) page << WaterboxUtils.PageShift) + Start; /// Get a stream that can be used to read or write from part of the block. Does not check for or change ! /// or end (= + ) are outside public Stream GetStream(ulong start, ulong length, bool writer) { - if (start < AddressRange.Start) throw new ArgumentOutOfRangeException(nameof(start), start, "invalid address"); - if (AddressRange.EndInclusive < start + length - 1) throw new ArgumentOutOfRangeException(nameof(length), length, "requested length implies invalid end address"); + if (start < Start) + throw new ArgumentOutOfRangeException(nameof(start)); + if (End < start + length) + throw new ArgumentOutOfRangeException(nameof(length)); return new MemoryViewStream(!writer, writer, (long) start, (long) length, this); } /// get a stream that can be used to read or write from part of the block. both reads and writes will be XORed against an earlier recorded snapshot - /// or end (= + ) are outside + /// or end (= + ) are outside bounds of memory block ( and ) /// no snapshot taken (haven't called ) public Stream GetXorStream(ulong start, ulong length, bool writer) { - if (start < AddressRange.Start) throw new ArgumentOutOfRangeException(nameof(start), start, "invalid address"); - if (AddressRange.EndInclusive < start + length - 1) throw new ArgumentOutOfRangeException(nameof(length), length, "requested length implies invalid end address"); + if (start < Start) throw new ArgumentOutOfRangeException(nameof(start)); + if (End < start + length) throw new ArgumentOutOfRangeException(nameof(length)); if (_snapshot == null) throw new InvalidOperationException("No snapshot taken!"); - return new MemoryViewXorStream(!writer, writer, (long) start, (long) length, this, _snapshot, (long) (start - AddressRange.Start)); + return new MemoryViewXorStream(!writer, writer, (long) start, (long) length, this, _snapshot, (long) (start - Start)); } /// activate the memory block, swapping it in at the pre-specified address @@ -142,7 +151,8 @@ namespace BizHawk.BizInvoke private void EnsureNotDisposed() { - if (_owner.AddressRange.Start == 0) throw new ObjectDisposedException(nameof(MemoryBlockBase)); //TODO bug? + if (_owner.Start == 0) + throw new ObjectDisposedException(nameof(MemoryBlockBase)); } public override void Flush() {} diff --git a/src/BizHawk.BizInvoke/MemoryBlockUnix.cs b/src/BizHawk.BizInvoke/MemoryBlockUnix.cs index 3200222e67..b3914cd7de 100644 --- a/src/BizHawk.BizInvoke/MemoryBlockUnix.cs +++ b/src/BizHawk.BizInvoke/MemoryBlockUnix.cs @@ -27,8 +27,8 @@ namespace BizHawk.BizInvoke { if (Active) throw new InvalidOperationException("Already active"); - var ptr = mmap(Z.US(AddressRange.Start), Z.UU(Size), MemoryProtection.Read | MemoryProtection.Write | MemoryProtection.Execute, 16, _fd, IntPtr.Zero); - if (ptr != Z.US(AddressRange.Start)) throw new InvalidOperationException($"{nameof(mmap)}() returned NULL or the wrong pointer"); + var ptr = mmap(Z.US(Start), Z.UU(Size), MemoryProtection.Read | MemoryProtection.Write | MemoryProtection.Execute, 16, _fd, IntPtr.Zero); + if (ptr != Z.US(Start)) throw new InvalidOperationException($"{nameof(mmap)}() returned NULL or the wrong pointer"); ProtectAll(); Active = true; @@ -39,7 +39,7 @@ namespace BizHawk.BizInvoke { if (!Active) throw new InvalidOperationException("Not active"); - var exitCode = munmap(Z.US(AddressRange.Start), Z.UU(Size)); + var exitCode = munmap(Z.US(Start), Z.UU(Size)); if (exitCode != 0) throw new InvalidOperationException($"{nameof(munmap)}() returned {exitCode}"); Active = false; @@ -51,10 +51,10 @@ namespace BizHawk.BizInvoke if (!Active) throw new InvalidOperationException("Not active"); // temporarily switch the entire block to `R` - var exitCode = mprotect(Z.US(AddressRange.Start), Z.UU(Size), MemoryProtection.Read); + var exitCode = mprotect(Z.US(Start), Z.UU(Size), MemoryProtection.Read); if (exitCode != 0) throw new InvalidOperationException($"{nameof(mprotect)}() returned {exitCode}!"); - var ret = WaterboxUtils.Hash(GetStream(AddressRange.Start, Size, false)); + var ret = WaterboxUtils.Hash(GetStream(Start, Size, false)); ProtectAll(); return ret; } @@ -107,11 +107,11 @@ namespace BizHawk.BizInvoke if (!Active) throw new InvalidOperationException("Not active"); // temporarily switch the entire block to `R`: in case some areas are unreadable, we don't want that to complicate things - var exitCode = mprotect(Z.US(AddressRange.Start), Z.UU(Size), MemoryProtection.Read); + var exitCode = mprotect(Z.US(Start), Z.UU(Size), MemoryProtection.Read); if (exitCode != 0) throw new InvalidOperationException($"{nameof(mprotect)}() returned {exitCode}!"); _snapshot = new byte[Size]; - GetStream(AddressRange.Start, Size, false).CopyTo(new MemoryStream(_snapshot, true)); + GetStream(Start, Size, false).CopyTo(new MemoryStream(_snapshot, true)); XorHash = WaterboxUtils.Hash(_snapshot); ProtectAll(); } diff --git a/src/BizHawk.Emulation.Cores/Waterbox/Heap.cs b/src/BizHawk.Emulation.Cores/Waterbox/Heap.cs index ca5fbb56d3..24d0629dec 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/Heap.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/Heap.cs @@ -61,8 +61,8 @@ namespace BizHawk.Emulation.Cores.Waterbox { throw new InvalidOperationException($"Failed to allocate {size} bytes from heap {Name}"); } - ulong ret = Memory.AddressRange.Start + allocstart; - Memory.Protect(Memory.AddressRange.Start + Used, newused - Used, MemoryBlockBase.Protection.RW); + ulong ret = Memory.Start + allocstart; + Memory.Protect(Memory.Start + Used, newused - Used, MemoryBlockBase.Protection.RW); Used = newused; Console.WriteLine($"Allocated {size} bytes on {Name}, utilization {Used}/{Memory.Size} ({100.0 * Used / Memory.Size:0.#}%)"); return ret; @@ -72,8 +72,8 @@ namespace BizHawk.Emulation.Cores.Waterbox { if (!Sealed) { - Memory.Protect(Memory.AddressRange.Start, Used, MemoryBlockBase.Protection.R); - _hash = WaterboxUtils.Hash(Memory.GetStream(Memory.AddressRange.Start, Used, false)); + Memory.Protect(Memory.Start, Used, MemoryBlockBase.Protection.R); + _hash = WaterboxUtils.Hash(Memory.GetStream(Memory.Start, Used, false)); Sealed = true; } else @@ -89,7 +89,7 @@ namespace BizHawk.Emulation.Cores.Waterbox if (!Sealed) { bw.Write(Memory.XorHash); - var ms = Memory.GetXorStream(Memory.AddressRange.Start, WaterboxUtils.AlignUp(Used), false); + var ms = Memory.GetXorStream(Memory.Start, WaterboxUtils.AlignUp(Used), false); ms.CopyTo(bw.BaseStream); } else @@ -116,9 +116,9 @@ namespace BizHawk.Emulation.Cores.Waterbox } var usedAligned = WaterboxUtils.AlignUp(used); - Memory.Protect(Memory.AddressRange.Start, Memory.Size, MemoryBlockBase.Protection.None); - Memory.Protect(Memory.AddressRange.Start, used, MemoryBlockBase.Protection.RW); - var ms = Memory.GetXorStream(Memory.AddressRange.Start, usedAligned, true); + Memory.Protect(Memory.Start, Memory.Size, MemoryBlockBase.Protection.None); + Memory.Protect(Memory.Start, used, MemoryBlockBase.Protection.RW); + var ms = Memory.GetXorStream(Memory.Start, usedAligned, true); WaterboxUtils.CopySome(br.BaseStream, ms, (long)usedAligned); Used = used; } diff --git a/src/BizHawk.Emulation.Cores/Waterbox/MapHeap.cs b/src/BizHawk.Emulation.Cores/Waterbox/MapHeap.cs index 4413954c67..7785161181 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/MapHeap.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/MapHeap.cs @@ -27,12 +27,18 @@ namespace BizHawk.Emulation.Cores.Waterbox /// /// get a page index within the block /// - private int GetPage(ulong addr) => (int) ((addr - Memory.AddressRange.Start) >> WaterboxUtils.PageShift); + private int GetPage(ulong addr) + { + return (int)((addr - Memory.Start) >> WaterboxUtils.PageShift); + } /// /// get a start address for a page index within the block /// - private ulong GetStartAddr(int page) => Memory.AddressRange.Start + ((ulong) page << WaterboxUtils.PageShift); + private ulong GetStartAddr(int page) + { + return ((ulong)page << WaterboxUtils.PageShift) + Memory.Start; + } private const MemoryBlockBase.Protection FREE = (MemoryBlockBase.Protection)255; @@ -172,7 +178,7 @@ namespace BizHawk.Emulation.Cores.Waterbox { // TODO: what is the expected behavior when everything requested for remap is allocated, // but with different protections? - if (oldSize == 0 || newSize == 0 || start < Memory.AddressRange.Start || Memory.AddressRange.EndInclusive < start + oldSize - 1) + if (start < Memory.Start || start + oldSize > Memory.End || oldSize == 0 || newSize == 0) return 0; var oldStartPage = GetPage(start); @@ -237,7 +243,7 @@ namespace BizHawk.Emulation.Cores.Waterbox public bool Protect(ulong start, ulong size, MemoryBlockBase.Protection prot) { - if (size == 0 || start < Memory.AddressRange.Start || Memory.AddressRange.EndInclusive < start + size - 1) + if (start < Memory.Start || start + size > Memory.End || size == 0) return false; var startPage = GetPage(start); @@ -268,8 +274,8 @@ namespace BizHawk.Emulation.Cores.Waterbox bw.Write(Memory.XorHash); bw.Write(_pagesAsBytes); - Memory.Protect(Memory.AddressRange.Start, Memory.Size, MemoryBlockBase.Protection.R); - var srcs = Memory.GetXorStream(Memory.AddressRange.Start, Memory.Size, false); + Memory.Protect(Memory.Start, Memory.Size, MemoryBlockBase.Protection.R); + var srcs = Memory.GetXorStream(Memory.Start, Memory.Size, false); for (int i = 0, addr = 0; i < _pages.Length; i++, addr += WaterboxUtils.PageSize) { if (_pages[i] != FREE) @@ -299,8 +305,8 @@ namespace BizHawk.Emulation.Cores.Waterbox throw new InvalidOperationException("Unexpected error reading!"); Used = 0; - Memory.Protect(Memory.AddressRange.Start, Memory.Size, MemoryBlockBase.Protection.RW); - var dsts = Memory.GetXorStream(Memory.AddressRange.Start, Memory.Size, true); + Memory.Protect(Memory.Start, Memory.Size, MemoryBlockBase.Protection.RW); + var dsts = Memory.GetXorStream(Memory.Start, Memory.Size, true); for (int i = 0, addr = 0; i < _pages.Length; i++, addr += WaterboxUtils.PageSize) { if (_pages[i] != FREE) diff --git a/src/BizHawk.Emulation.Cores/Waterbox/PeRunner.cs b/src/BizHawk.Emulation.Cores/Waterbox/PeRunner.cs index 4a7b053895..542d92bf66 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/PeRunner.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/PeRunner.cs @@ -430,13 +430,13 @@ namespace BizHawk.Emulation.Cores.Waterbox { var heap = _parent._heap; - var start = heap.Memory.AddressRange.Start; + var start = heap.Memory.Start; var end = start + heap.Used; - var max = heap.Memory.AddressRange.EndInclusive; + var max = heap.Memory.End; var p = (ulong)_p; - if (p < start || max + 1 < p) //TODO bug? + if (p < start || p > max) { // failure: return current break return Z.UU(end); diff --git a/src/BizHawk.Emulation.Cores/Waterbox/PeWrapper.cs b/src/BizHawk.Emulation.Cores/Waterbox/PeWrapper.cs index ecae3feffa..362efb7ce6 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/PeWrapper.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/PeWrapper.cs @@ -296,7 +296,7 @@ namespace BizHawk.Emulation.Cores.Waterbox /// private void ProtectMemory() { - Memory.Protect(Memory.AddressRange.Start, Memory.Size, MemoryBlockBase.Protection.R); + Memory.Protect(Memory.Start, Memory.Size, MemoryBlockBase.Protection.R); foreach (var s in _sections) { @@ -441,7 +441,7 @@ namespace BizHawk.Emulation.Cores.Waterbox // unlikely to get this far if the previous checks pssed throw new InvalidOperationException("Trickys elves moved on you!"); - Memory.Protect(Memory.AddressRange.Start, Memory.Size, MemoryBlockBase.Protection.RW); + Memory.Protect(Memory.Start, Memory.Size, MemoryBlockBase.Protection.RW); foreach (var s in _sections) {