Skip to content

Improve consistency and reliability across MemoryDomains, with unit tests #4495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Improve consistency and reliability across MemoryDomains, with unit tests #4495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a1aaabd
Reduce nesting in `MemoryDomain` subclasses and clean up
YoshiRulz Oct 7, 2025
1ff6288
Make `MemoryDomain.Poke*` throw in Debug if the domain isn't pokable
YoshiRulz Oct 7, 2025
d7fef09
WIP Add unit tests for `MemoryDomain` subclasses
YoshiRulz Oct 6, 2025
9c81faa
WIP Move range checks into `MemoryDomainDelegate` from instances
YoshiRulz Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/BizHawk.Emulation.Common/Base Implementations/MemoryDomain.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#nullable disable

using System.Buffers.Binary;
using System.Diagnostics;

using BizHawk.Common;

Expand All @@ -20,6 +21,16 @@ 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}";

[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; }
Expand All @@ -30,12 +41,18 @@ public enum Endian

public bool Writable { get; protected set; }

/// <exception cref="ArgumentOutOfRangeException"><paramref name="addr"/> not in <c>0..&lt;Size</c></exception>
/// <exception cref="IndexOutOfRangeException">some implementations throw this instead of <see cref="ArgumentOutOfRangeException"/></exception>
public abstract byte PeekByte(long addr);

/// <inheritdoc cref="PeekByte"/>
/// <exception cref="NotSupportedException">poking is unavailable (<see cref="Writable"/> is <see langword="false"/>)</exception>
public abstract void PokeByte(long addr, byte val);

public override string ToString() => Name;

/// <exception cref="ArgumentOutOfRangeException"><paramref name="addr"/> (or addr implied by width of peek) not in <c>0..&lt;Size</c></exception>
/// <exception cref="IndexOutOfRangeException">some implementations throw this instead of <see cref="ArgumentOutOfRangeException"/></exception>
public virtual ushort PeekUshort(long addr, bool bigEndian)
{
if (bigEndian)
Expand All @@ -46,6 +63,7 @@ public virtual ushort PeekUshort(long addr, bool bigEndian)
return (ushort)(PeekByte(addr) | (PeekByte(addr + 1) << 8));
}

/// <inheritdoc cref="PeekUshort"/>
public virtual uint PeekUint(long addr, bool bigEndian)
{
ReadOnlySpan<byte> scratch = stackalloc byte[]
Expand All @@ -60,6 +78,9 @@ public virtual uint PeekUint(long addr, bool bigEndian)
: BinaryPrimitives.ReadUInt32LittleEndian(scratch);
}

/// <exception cref="ArgumentOutOfRangeException"><paramref name="addr"/> (or addr implied by width of poke) not in <c>0..&lt;Size</c></exception>
/// <exception cref="IndexOutOfRangeException">some implementations throw this instead of <see cref="ArgumentOutOfRangeException"/></exception>
/// <exception cref="NotSupportedException">poking is unavailable (<see cref="Writable"/> is <see langword="false"/>)</exception>
public virtual void PokeUshort(long addr, ushort val, bool bigEndian)
{
if (bigEndian)
Expand All @@ -74,6 +95,7 @@ public virtual void PokeUshort(long addr, ushort val, bool bigEndian)
}
}

/// <inheritdoc cref="PokeUshort"/>
public virtual void PokeUint(long addr, uint val, bool bigEndian)
{
Span<byte> scratch = stackalloc byte[4];
Expand All @@ -85,6 +107,8 @@ public virtual void PokeUint(long addr, uint val, bool bigEndian)
PokeByte(addr + 3, scratch[3]);
}

/// <exception cref="ArgumentOutOfRangeException"><paramref name="addresses"/> not contained in <c>0..&lt;Size</c></exception>
/// <exception cref="IndexOutOfRangeException">some implementations throw this instead of <see cref="ArgumentOutOfRangeException"/></exception>
public virtual void BulkPeekByte(Range<long> addresses, byte[] values)
{
if (addresses is null) throw new ArgumentNullException(paramName: nameof(addresses));
Expand All @@ -104,6 +128,7 @@ public virtual void BulkPeekByte(Range<long> addresses, byte[] values)
}
}

/// <inheritdoc cref="BulkPeekByte"/>
public virtual void BulkPeekUshort(Range<long> addresses, bool bigEndian, ushort[] values)
{
if (addresses is null) throw new ArgumentNullException(paramName: nameof(addresses));
Expand All @@ -128,6 +153,7 @@ public virtual void BulkPeekUshort(Range<long> addresses, bool bigEndian, ushort
}
}

/// <inheritdoc cref="BulkPeekByte"/>
public virtual void BulkPeekUint(Range<long> addresses, bool bigEndian, uint[] values)
{
if (addresses is null) throw new ArgumentNullException(paramName: nameof(addresses));
Expand Down
153 changes: 66 additions & 87 deletions src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainImpls.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,19 @@ public Action<long, byte> 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);
}

public override void PokeByte(long addr, byte val)
{
_poke?.Invoke(addr, val);
if (_poke is null)
{
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);
}

public override void BulkPeekByte(Range<long> addresses, byte[] values)
Expand All @@ -53,6 +60,10 @@ public override void BulkPeekUshort(Range<long> 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
Expand All @@ -65,6 +76,10 @@ public override void BulkPeekUint(Range<long> 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
Expand Down Expand Up @@ -118,10 +133,12 @@ public override byte PeekByte(long addr)

public override void PokeByte(long addr, byte val)
{
if (Writable)
if (!Writable)
{
Data[addr] = val;
FailPokingNotAllowed();
return;
}
Data[addr] = val;
}

public MemoryDomainByteArray(string name, Endian endian, byte[] data, bool writable, int wordSize)
Expand Down Expand Up @@ -162,7 +179,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)
Expand All @@ -181,48 +201,37 @@ 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));
}
FailPokingNotAllowed();
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<long> 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)
Expand All @@ -248,33 +257,21 @@ 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));
}
FailPokingNotAllowed();
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)
Expand Down Expand Up @@ -307,27 +304,21 @@ 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));
}
FailPokingNotAllowed();
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)
Expand All @@ -348,33 +339,21 @@ 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));
}
FailPokingNotAllowed();
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,
Expand Down
16 changes: 10 additions & 6 deletions src/BizHawk.Emulation.Cores/Arcades/MAME/MAME.MemoryDomains.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,23 @@ 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})");
FailPokingNotAllowed();
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()
Expand Down
3 changes: 1 addition & 2 deletions src/BizHawk.Emulation.Cores/Computers/Doom/DSDA.IMemoryDomains.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,7 @@ public override void BulkPeekUint(Range<long> addresses, bool bigEndian, uint[]
}

public override void PokeByte(long addr, byte val)
{
}
=> FailPokingNotAllowed();
}
}
}
Loading