From a1aaabd1b40cc9e9cd13dd2f0072ff550e4d2875 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 7 Oct 2025 11:28:56 +1000 Subject: [PATCH 1/4] Reduce nesting in `MemoryDomain` subclasses and clean up --- .../Base Implementations/MemoryDomain.cs | 19 +++ .../Base Implementations/MemoryDomainImpls.cs | 128 ++++++------------ .../Arcades/MAME/MAME.MemoryDomains.cs | 15 +- .../Waterbox/WaterboxMemoryDomain.cs | 88 ++++-------- 4 files changed, 96 insertions(+), 154 deletions(-) diff --git a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs index 66c1a56b4f9..cb716f5564b 100644 --- a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs +++ b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs @@ -20,6 +20,12 @@ public enum Endian Unknown, } + protected const string ERR_FMT_STR_ADDR_OOR = "address must be in 0..<{0}"; + + protected const string ERR_FMT_STR_END_OOR = "last address of range must be in 0..<{0}"; + + protected const string ERR_FMT_STR_START_OOR = "first address of range must be in 0..<{0}"; + public string Name { get; protected set; } public long Size { get; protected set; } @@ -30,12 +36,17 @@ public enum Endian public bool Writable { get; protected set; } + /// not in 0..<Size + /// some implementations throw this instead of public abstract byte PeekByte(long addr); + /// public abstract void PokeByte(long addr, byte val); public override string ToString() => Name; + /// (or addr implied by width of peek) not in 0..<Size + /// some implementations throw this instead of public virtual ushort PeekUshort(long addr, bool bigEndian) { if (bigEndian) @@ -46,6 +57,7 @@ public virtual ushort PeekUshort(long addr, bool bigEndian) return (ushort)(PeekByte(addr) | (PeekByte(addr + 1) << 8)); } + /// public virtual uint PeekUint(long addr, bool bigEndian) { ReadOnlySpan scratch = stackalloc byte[] @@ -60,6 +72,8 @@ public virtual uint PeekUint(long addr, bool bigEndian) : BinaryPrimitives.ReadUInt32LittleEndian(scratch); } + /// (or addr implied by width of poke) not in 0..<Size + /// some implementations throw this instead of public virtual void PokeUshort(long addr, ushort val, bool bigEndian) { if (bigEndian) @@ -74,6 +88,7 @@ public virtual void PokeUshort(long addr, ushort val, bool bigEndian) } } + /// public virtual void PokeUint(long addr, uint val, bool bigEndian) { Span scratch = stackalloc byte[4]; @@ -85,6 +100,8 @@ public virtual void PokeUint(long addr, uint val, bool bigEndian) PokeByte(addr + 3, scratch[3]); } + /// not contained in 0..<Size + /// some implementations throw this instead of public virtual void BulkPeekByte(Range addresses, byte[] values) { if (addresses is null) throw new ArgumentNullException(paramName: nameof(addresses)); @@ -104,6 +121,7 @@ public virtual void BulkPeekByte(Range addresses, byte[] values) } } + /// public virtual void BulkPeekUshort(Range addresses, bool bigEndian, ushort[] values) { if (addresses is null) throw new ArgumentNullException(paramName: nameof(addresses)); @@ -128,6 +146,7 @@ public virtual void BulkPeekUshort(Range addresses, bool bigEndian, ushort } } + /// public virtual void BulkPeekUint(Range addresses, bool bigEndian, uint[] values) { if (addresses is null) throw new ArgumentNullException(paramName: nameof(addresses)); diff --git a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs index a51fac18280..00719dadc17 100644 --- a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs +++ b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs @@ -118,10 +118,11 @@ public override byte PeekByte(long addr) public override void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - Data[addr] = val; + return; } + Data[addr] = val; } public MemoryDomainByteArray(string name, Endian endian, byte[] data, bool writable, int wordSize) @@ -181,48 +182,36 @@ public MemoryDomainUshortArray(string name, Endian endian, ushort[] data, bool w } } - public unsafe class MemoryDomainIntPtr : MemoryDomain + public class MemoryDomainIntPtr : MemoryDomain { public IntPtr Data { get; set; } - public override byte PeekByte(long addr) + public unsafe override byte PeekByte(long addr) { - if ((ulong)addr < (ulong)Size) - { - return ((byte*)Data)[addr]; - } - - throw new ArgumentOutOfRangeException(nameof(addr)); + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + return ((byte*) Data)[addr]; } - public override void PokeByte(long addr, byte val) + public override unsafe void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - if ((ulong)addr < (ulong)Size) - { - ((byte*)Data)[addr] = val; - } - else - { - throw new ArgumentOutOfRangeException(nameof(addr)); - } + return; } + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + ((byte*) Data)[addr] = val; } public override void BulkPeekByte(Range addresses, byte[] values) { + //TODO why are we casting `long`s to `ulong` here? var start = (ulong)addresses.Start; + if ((ulong) Size <= start) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), start, message: string.Format(ERR_FMT_STR_START_OOR, Size)); var count = addresses.Count(); - - if (start < (ulong)Size && (start + count) <= (ulong)Size) - { - Marshal.Copy((IntPtr)((ulong)Data + start), values, 0, (int)count); - } - else - { - throw new ArgumentOutOfRangeException(nameof(addresses)); - } + var endExcl = start + count; + if ((ulong) Size < endExcl) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), endExcl, message: string.Format(ERR_FMT_STR_END_OOR, Size)); + Marshal.Copy(source: (IntPtr) ((ulong) Data + start), destination: values, startIndex: 0, length: (int) count); } public void SetSize(long size) @@ -248,33 +237,20 @@ public unsafe class MemoryDomainIntPtrMonitor : MemoryDomain public override byte PeekByte(long addr) { - if ((ulong)addr < (ulong)Size) - { - using (_monitor.EnterExit()) - { - return ((byte*)Data)[addr]; - } - } - - throw new ArgumentOutOfRangeException(nameof(addr)); + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + using (_monitor.EnterExit()) return ((byte*) Data)[addr]; } public override void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - if ((ulong)addr < (ulong)Size) - { - using (_monitor.EnterExit()) - { - ((byte*)Data)[addr] = val; - } - } - else - { - throw new ArgumentOutOfRangeException(nameof(addr)); - } + return; } + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + using (_monitor.EnterExit()) ((byte*) Data)[addr] = val; } public void SetSize(long size) @@ -307,27 +283,20 @@ public unsafe class MemoryDomainIntPtrSwap16 : MemoryDomain public override byte PeekByte(long addr) { - if ((ulong)addr < (ulong)Size) - { - return ((byte*)Data)[addr ^ 1]; - } - - throw new ArgumentOutOfRangeException(nameof(addr)); + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + return ((byte*) Data)[addr ^ 1L]; } public override void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - if ((ulong)addr < (ulong)Size) - { - ((byte*)Data)[addr ^ 1] = val; - } - else - { - throw new ArgumentOutOfRangeException(nameof(addr)); - } + return; } + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + ((byte*) Data)[addr ^ 1L] = val; } public MemoryDomainIntPtrSwap16(string name, Endian endian, IntPtr data, long size, bool writable) @@ -348,33 +317,20 @@ public unsafe class MemoryDomainIntPtrSwap16Monitor : MemoryDomain public override byte PeekByte(long addr) { - if ((ulong)addr < (ulong)Size) - { - using (_monitor.EnterExit()) - { - return ((byte*)Data)[addr ^ 1]; - } - } - - throw new ArgumentOutOfRangeException(nameof(addr)); + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + using (_monitor.EnterExit()) return ((byte*) Data)[addr ^ 1L]; } public override void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - if ((ulong)addr < (ulong)Size) - { - using (_monitor.EnterExit()) - { - ((byte*)Data)[addr ^ 1] = val; - } - } - else - { - throw new ArgumentOutOfRangeException(nameof(addr)); - } + return; } + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + using (_monitor.EnterExit()) ((byte*) Data)[addr ^ 1L] = val; } public MemoryDomainIntPtrSwap16Monitor(string name, Endian endian, IntPtr data, long size, bool writable, diff --git a/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs b/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs index 5ab82b5fb20..6a3ca4349aa 100644 --- a/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs +++ b/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs @@ -34,19 +34,22 @@ public MAMEMemoryDomain(string name, long size, Endian endian, int dataWidth, bo public override byte PeekByte(long addr) { - if ((ulong)addr >= (ulong)_systemBusSize) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: "address out of range"); + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) _systemBusSize <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); addr += _firstOffset; - return _core.mame_read_byte((uint)addr << _systemBusAddressShift); + return _core.mame_read_byte((uint) addr << _systemBusAddressShift); } public override void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - if ((ulong)addr >= (ulong)_systemBusSize) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: "address out of range"); - addr += _firstOffset; - _core.mame_lua_execute($"{MAMELuaCommand.GetSpace}:write_u8({addr << _systemBusAddressShift}, {val})"); + return; } + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) _systemBusSize <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + addr += _firstOffset; + _core.mame_lua_execute($"{MAMELuaCommand.GetSpace}:write_u8({addr << _systemBusAddressShift}, {val})"); } public override void Enter() diff --git a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs b/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs index 04fabb50ac8..d62bfdc51fb 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs @@ -70,33 +70,18 @@ internal WaterboxMemoryDomainPointer(MemoryArea m, IMonitor monitor) public override byte PeekByte(long addr) { - if ((ulong)addr < (ulong)Size) - { - using (_monitor.EnterExit()) - { - return ((byte*)_data)[addr ^ _addressMangler]; - } - } - - throw new ArgumentOutOfRangeException(nameof(addr)); + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + using (_monitor.EnterExit()) return ((byte*) _data)[addr ^ _addressMangler]; } public override void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - if ((ulong)addr < (ulong)Size) - { - using (_monitor.EnterExit()) - { - ((byte*)_data)[addr ^ _addressMangler] = val; - } - } - else - { - throw new ArgumentOutOfRangeException(nameof(addr)); - } + return; } + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + using (_monitor.EnterExit()) ((byte*) _data)[addr ^ _addressMangler] = val; } public override void BulkPeekByte(Range addresses, byte[] values) @@ -106,21 +91,13 @@ public override void BulkPeekByte(Range addresses, byte[] values) base.BulkPeekByte(addresses, values); return; } - + //TODO why are we casting `long`s to `ulong` here? var start = (ulong)addresses.Start; var count = addresses.Count(); - - if (start < (ulong)Size && (start + count) <= (ulong)Size) - { - using (_monitor.EnterExit()) - { - Marshal.Copy(Z.US((ulong)_data + start), values, 0, (int)count); - } - } - else - { - throw new ArgumentOutOfRangeException(nameof(addresses)); - } + if ((ulong) Size <= start) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), start, message: string.Format(ERR_FMT_STR_START_OOR, Size)); + var endExcl = start + count; + if ((ulong) Size < endExcl) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), endExcl, message: string.Format(ERR_FMT_STR_END_OOR, Size)); + using (_monitor.EnterExit()) Marshal.Copy(source: Z.US((ulong) _data + start), destination: values, startIndex: 0, length: (int) count); } } @@ -164,29 +141,22 @@ internal WaterboxMemoryDomainFunc(MemoryArea m, WaterboxHost monitor) public override byte PeekByte(long addr) { - if ((ulong)addr < (ulong)Size) - { - byte ret = 0; - _access.Access((IntPtr)(&ret), addr, 1, false); - return ret; - } - - throw new ArgumentOutOfRangeException(nameof(addr)); + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + byte ret = 0; + _access.Access((IntPtr) (&ret), address: addr, count: 1, write: false); + return ret; } public override void PokeByte(long addr, byte val) { - if (Writable) + if (!Writable) { - if ((ulong)addr < (ulong)Size) - { - _access.Access((IntPtr)(&val), addr, 1, true); - } - else - { - throw new ArgumentOutOfRangeException(nameof(addr)); - } + return; } + //TODO why are we casting `long`s to `ulong` here? + if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); + _access.Access((IntPtr) (&val), address: addr, count: 1, write: true); } public override void BulkPeekByte(Range addresses, byte[] values) @@ -196,19 +166,13 @@ public override void BulkPeekByte(Range addresses, byte[] values) base.BulkPeekByte(addresses, values); return; } - + //TODO why are we casting `long`s to `ulong` here? var start = (ulong)addresses.Start; var count = addresses.Count(); - - if (start < (ulong)Size && (start + count) <= (ulong)Size) - { - fixed(byte* p = values) - _access.Access((IntPtr)p, (long)start, (long)count, false); - } - else - { - throw new ArgumentOutOfRangeException(nameof(addresses)); - } + if ((ulong) Size <= start) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), start, message: string.Format(ERR_FMT_STR_START_OOR, Size)); + var endExcl = start + count; + if ((ulong) Size < endExcl) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), endExcl, message: string.Format(ERR_FMT_STR_END_OOR, Size)); + fixed (byte* p = values) _access.Access((IntPtr) p, address: addresses.Start, count: (long) count, write: false); } } } From 1ff62884e4f050bc9629e7ea7c2cb06b75613b38 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 7 Oct 2025 13:55:34 +1000 Subject: [PATCH 2/4] Make `MemoryDomain.Poke*` throw in Debug if the domain isn't pokable --- .../Base Implementations/MemoryDomain.cs | 7 +++++++ .../Base Implementations/MemoryDomainImpls.cs | 15 ++++++++++++++- .../Arcades/MAME/MAME.MemoryDomains.cs | 1 + .../Computers/Doom/DSDA.IMemoryDomains.cs | 3 +-- .../Waterbox/WaterboxHost.cs | 4 +--- .../Waterbox/WaterboxMemoryDomain.cs | 2 ++ 6 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs index cb716f5564b..446fc7441c2 100644 --- a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs +++ b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs @@ -1,6 +1,7 @@ #nullable disable using System.Buffers.Binary; +using System.Diagnostics; using BizHawk.Common; @@ -26,6 +27,10 @@ public enum Endian protected const string ERR_FMT_STR_START_OOR = "first address of range must be in 0..<{0}"; + [Conditional("DEBUG")] + protected static void FailPokingNotAllowed() + => throw new NotImplementedException($"{nameof(MemoryDomain)} does not support poking"); + public string Name { get; protected set; } public long Size { get; protected set; } @@ -41,6 +46,7 @@ public enum Endian public abstract byte PeekByte(long addr); /// + /// poking is unavailable ( is ) public abstract void PokeByte(long addr, byte val); public override string ToString() => Name; @@ -74,6 +80,7 @@ public virtual uint PeekUint(long addr, bool bigEndian) /// (or addr implied by width of poke) not in 0..<Size /// some implementations throw this instead of + /// poking is unavailable ( is ) public virtual void PokeUshort(long addr, ushort val, bool bigEndian) { if (bigEndian) diff --git a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs index 00719dadc17..d79abff54d5 100644 --- a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs +++ b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs @@ -34,7 +34,12 @@ public override byte PeekByte(long addr) public override void PokeByte(long addr, byte val) { - _poke?.Invoke(addr, val); + if (_poke is null) + { + FailPokingNotAllowed(); + return; + } + _poke.Invoke(addr, val); } public override void BulkPeekByte(Range addresses, byte[] values) @@ -120,6 +125,7 @@ public override void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } Data[addr] = val; @@ -163,7 +169,10 @@ public override byte PeekByte(long addr) public override void PokeByte(long addr, byte val) { if (!Writable) + { + FailPokingNotAllowed(); return; + } long bit0 = addr & 1; addr >>= 1; if (bit0 == 0) @@ -196,6 +205,7 @@ public override unsafe void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } //TODO why are we casting `long`s to `ulong` here? @@ -246,6 +256,7 @@ public override void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } //TODO why are we casting `long`s to `ulong` here? @@ -292,6 +303,7 @@ public override void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } //TODO why are we casting `long`s to `ulong` here? @@ -326,6 +338,7 @@ public override void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } //TODO why are we casting `long`s to `ulong` here? diff --git a/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs b/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs index 6a3ca4349aa..46e4c1d8755 100644 --- a/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs +++ b/src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs @@ -44,6 +44,7 @@ public override void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } //TODO why are we casting `long`s to `ulong` here? diff --git a/src/BizHawk.Emulation.Cores/Computers/Doom/DSDA.IMemoryDomains.cs b/src/BizHawk.Emulation.Cores/Computers/Doom/DSDA.IMemoryDomains.cs index 1043527c57b..0172fadd4d7 100644 --- a/src/BizHawk.Emulation.Cores/Computers/Doom/DSDA.IMemoryDomains.cs +++ b/src/BizHawk.Emulation.Cores/Computers/Doom/DSDA.IMemoryDomains.cs @@ -283,8 +283,7 @@ public override void BulkPeekUint(Range addresses, bool bigEndian, uint[] } public override void PokeByte(long addr, byte val) - { - } + => FailPokingNotAllowed(); } } } diff --git a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxHost.cs b/src/BizHawk.Emulation.Cores/Waterbox/WaterboxHost.cs index 4779521bf33..084c5e738ce 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxHost.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/WaterboxHost.cs @@ -337,9 +337,7 @@ public override byte PeekByte(long addr) } public override void PokeByte(long addr, byte val) - { - throw new InvalidOperationException(); - } + => FailPokingNotAllowed(); } public bool AvoidRewind => false; diff --git a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs b/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs index d62bfdc51fb..b8feaf2de99 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/WaterboxMemoryDomain.cs @@ -78,6 +78,7 @@ public override void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } if ((ulong) Size <= (ulong) addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); @@ -152,6 +153,7 @@ public override void PokeByte(long addr, byte val) { if (!Writable) { + FailPokingNotAllowed(); return; } //TODO why are we casting `long`s to `ulong` here? From d7fef09cdb088b9816d6064c00f9fa0554c453a6 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 7 Oct 2025 05:51:16 +1000 Subject: [PATCH 3/4] WIP Add unit tests for `MemoryDomain` subclasses --- .../memdomain/MemoryDomainTests.cs | 244 ++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 src/BizHawk.Tests.Emulation.Common/memdomain/MemoryDomainTests.cs diff --git a/src/BizHawk.Tests.Emulation.Common/memdomain/MemoryDomainTests.cs b/src/BizHawk.Tests.Emulation.Common/memdomain/MemoryDomainTests.cs new file mode 100644 index 00000000000..0eb94311cf9 --- /dev/null +++ b/src/BizHawk.Tests.Emulation.Common/memdomain/MemoryDomainTests.cs @@ -0,0 +1,244 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Runtime.InteropServices; + +using BizHawk.Emulation.Common; + +namespace BizHawk.Tests.Emulation.Common +{ + public abstract class MemoryDomainTestsBase + where TOORE : Exception + { + protected const MemoryDomain.Endian BIG_ENDIAN = MemoryDomain.Endian.Big; + + private const byte SENTINEL = 0xE3; + + protected const int SIZE = 6; + + protected static readonly ImmutableArray InitPattern = [ 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10 ]; + + protected MemoryDomain _domain = null!; + + private string SubclassName +#pragma warning disable BHI1101 // this is a simple way to disambiguate test cases in the log + => GetType().Name; +#pragma warning restore BHI1101 + + protected abstract void AssertUnchanged(); + + [TestMethod] + public void TestPeek1oAfterEnd() + { + Assert.Throws(() => _domain.PeekByte(6L), $"peeking byte at [^0] in {SubclassName}"); + AssertUnchanged(); + } + + [TestMethod] + public void TestPeek1oBeforeStart() + { + Assert.Throws(() => _domain.PeekByte(-1L), $"peeking byte at [-1] in {SubclassName}"); + AssertUnchanged(); + } + + [TestMethod] + public void TestPeek1oFirst() + { + Assert.AreEqual(0xDC, _domain.PeekByte(0L), $"peeking byte at [0] in {SubclassName}"); + AssertUnchanged(); + } + + [TestMethod] + public void TestPeek1oFourth() + { + Assert.AreEqual(0x76, _domain.PeekByte(3L), $"peeking byte at [3] in {SubclassName}"); + AssertUnchanged(); + } + + [TestMethod] + public void TestPeek1oLast() + { + Assert.AreEqual(0x32, _domain.PeekByte(5L), $"peeking byte at [5] in {SubclassName}"); + AssertUnchanged(); + } + + //TODO peek 2o + + //TODO peek 4o + + [TestMethod] + public void TestPoke1oAfterEnd() + { + Assert.Throws(() => _domain.PokeByte(6L, SENTINEL), $"poking byte at [^0] in {SubclassName}"); + AssertUnchanged(); + } + + [TestMethod] + public void TestPoke1oBeforeStart() + { + Assert.Throws(() => _domain.PokeByte(-1L, SENTINEL), $"poking byte at [-1] in {SubclassName}"); + AssertUnchanged(); + } + + [TestMethod] + public void TestPoke1oFirst() + { + Assert.Inconclusive("TODO"); + } + + [TestMethod] + public void TestPoke1oFourth() + { + Assert.Inconclusive("TODO"); + } + + [TestMethod] + public void TestPoke1oLast() + { + Assert.Inconclusive("TODO"); + } + + //TODO poke 2o + + //TODO poke 4o + } + + [TestClass] + public sealed class MemoryDomainByteArrayTests : MemoryDomainTestsBase + { +#if true + private byte[] _buf = null!; + + protected override void AssertUnchanged() + => CollectionAssert.AreEqual(InitPattern.Slice(start: 1, length: SIZE), _buf); + + [TestInitialize] + public void TestInitialize() + { + _buf = InitPattern.Slice(start: 1, length: SIZE).ToArray(); + _domain = new MemoryDomainByteArray("RAM", BIG_ENDIAN, _buf, writable: true, wordSize: 1); + } +#else + [StructLayout(LayoutKind.Sequential)] + private struct Buffer + { + public byte Before; + + public unsafe fixed byte Buf[6]; + + public byte After; + } + + private Buffer _buf; + + protected override void AssertUnchanged() + => CollectionAssert.AreEqual(InitPattern, MemoryMarshal.AsBytes([ _buf ]).ToArray()); + + [TestInitialize] + public void TestInitialize() + { + _buf = MemoryMarshal.Read(InitPattern.ToArray()); + byte[] usable = _buf.Buf; //TODO doesn't work--this expression's type is a pointer, not an array + // we may not be able to catch buffer under/overreads for this impl., though of course there's no need to since it's completely managed and the runtime ensures a bounds check + // maybe this struct idea could be used for `MemoryDomainIntPtr*` instead? + _domain = new MemoryDomainByteArray("RAM", BIG_ENDIAN, usable, writable: true, wordSize: 1); + } +#endif + } + + [TestClass] + public sealed class MemoryDomainDelegateTests : MemoryDomainTestsBase + { + private List _buf = null!; + + protected override void AssertUnchanged() + => CollectionAssert.AreEqual(InitPattern, _buf); + + private byte PeekByte(long addr) + => _buf[1 + unchecked((int) addr)]; + + private void PokeByte(long addr, byte value) + => _buf[1 + unchecked((int) addr)] = value; + + [TestInitialize] + public void TestInitialize() + { + _buf = InitPattern.ToList(); + _domain = new MemoryDomainDelegate("RAM", size: SIZE, BIG_ENDIAN, PeekByte, PokeByte, wordSize: 1); + } + } + +#if false + [TestClass] + public sealed class MemoryDomainIntPtrTests : MemoryDomainTestsBase + { + protected override void AssertUnchanged() + => throw new NotImplementedException("TODO"); + + [TestInitialize] + public void TestInitialize() + { + throw new NotImplementedException("TODO"); + } + } + + [TestClass] + public sealed class MemoryDomainIntPtrMonitorTests : MemoryDomainTestsBase + { + protected override void AssertUnchanged() + => throw new NotImplementedException("TODO"); + + [TestInitialize] + public void TestInitialize() + { + throw new NotImplementedException("TODO"); + } + } + + [TestClass] + public sealed class MemoryDomainIntPtrSwap16Tests : MemoryDomainTestsBase + { + protected override void AssertUnchanged() + => throw new NotImplementedException("TODO"); + + [TestInitialize] + public void TestInitialize() + { + throw new NotImplementedException("TODO"); + } + } + + [TestClass] + public sealed class MemoryDomainIntPtrSwap16MonitorTests : MemoryDomainTestsBase + { + protected override void AssertUnchanged() + => throw new NotImplementedException("TODO"); + + [TestInitialize] + public void TestInitialize() + { + throw new NotImplementedException("TODO"); + } + } +#endif + + [TestClass] + public sealed class MemoryDomainUshortArrayTests : MemoryDomainTestsBase + { + private ushort[] _buf = null!; + + protected override void AssertUnchanged() + => CollectionAssert.AreEqual(CloneInitPattern(), _buf); + + private ushort[] CloneInitPattern() + => MemoryMarshal.Cast(InitPattern.Slice(start: 1, length: SIZE).AsSpan()).ToArray(); + + [TestInitialize] + public void TestInitialize() + { + _buf = CloneInitPattern(); + // see note in `MemoryDomainByteArrayTests` re: buffer under/overread + _domain = new MemoryDomainUshortArray("RAM", BIG_ENDIAN, _buf, writable: true); + } + } +} From 9c81faa7eb0d492ddf57c111fa5d68dde1637fb5 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Tue, 7 Oct 2025 14:08:18 +1000 Subject: [PATCH 4/4] WIP Move range checks into `MemoryDomainDelegate` from instances --- .../Base Implementations/MemoryDomainImpls.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs index d79abff54d5..f5840bd38aa 100644 --- a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs +++ b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs @@ -29,6 +29,7 @@ public Action Poke public override byte PeekByte(long addr) { + if (addr < 0 || Size <= addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); return Peek(addr); } @@ -39,6 +40,7 @@ public override void PokeByte(long addr, byte val) FailPokingNotAllowed(); return; } + if (addr < 0 || Size <= addr) throw new ArgumentOutOfRangeException(paramName: nameof(addr), addr, message: string.Format(ERR_FMT_STR_ADDR_OOR, Size)); _poke.Invoke(addr, val); } @@ -58,6 +60,10 @@ public override void BulkPeekUshort(Range addresses, bool bigEndian, ushor { if (_bulkPeekUshort != null) { + var start = addresses.Start; + if (start < 0 || Size <= start) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), start, message: string.Format(ERR_FMT_STR_START_OOR, Size)); + var endExcl = start + checked((long) addresses.Count()); + if (Size < endExcl) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), endExcl, message: string.Format(ERR_FMT_STR_END_OOR, Size)); _bulkPeekUshort.Invoke(addresses, bigEndian, values); } else @@ -70,6 +76,10 @@ public override void BulkPeekUint(Range addresses, bool bigEndian, uint[] { if (_bulkPeekUint != null) { + var start = addresses.Start; + if (start < 0 || Size <= start) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), start, message: string.Format(ERR_FMT_STR_START_OOR, Size)); + var endExcl = start + checked((long) addresses.Count()); + if (Size < endExcl) throw new ArgumentOutOfRangeException(paramName: nameof(addresses), endExcl, message: string.Format(ERR_FMT_STR_END_OOR, Size)); _bulkPeekUint.Invoke(addresses, bigEndian, values); } else