TASEmulators / BizHawk

BizHawk is a multi-system emulator written in C#. BizHawk provides nice features for casual gamers such as full screen, and joypad support in addition to full rerecording and debugging tools for all system cores.
http://tasvideos.org/BizHawk.html
Other
2.1k stars 378 forks source link

PeekByte() on parts of the Apple II system bus results in crash (scrolling in the hex editor) #1169

Open NarryG opened 6 years ago

NarryG commented 6 years ago

If you load up the Apple II core, open the hex editor, swap the domain to the System Bus, and try scrolling to the bottom, Bizhawk will crash with the following exception:

   at Jellyfish.Virtu.DiskIIDrive.get_IsWriteProtected()
   at Jellyfish.Virtu.DiskIIController.ReadIoRegionC0C0(Int32 address)
   at Jellyfish.Virtu.Memory.ReadIoRegionC0C0(Int32 address)
   at Jellyfish.Virtu.Memory.ReadIoRegionC0CF(Int32 address)
   at Jellyfish.Virtu.Memory.Peek(Int32 address)
   at BizHawk.Emulation.Cores.Computers.AppleII.AppleII.<SetupMemoryDomains>b__85_2(Int64 addr) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Emulation.Cores\Computers\AppleII\AppleII.IMemoryDomains.cs:line 44
   at BizHawk.Client.EmuHawk.HexEditor.MakeValue(Int32 dataSize, Int64 address, Boolean& is_cheat) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\HexEditor\HexEditor.cs:line 603
   at BizHawk.Client.EmuHawk.HexEditor.GenerateMemoryViewString(Boolean forWindow) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\HexEditor\HexEditor.cs:line 539
   at BizHawk.Client.EmuHawk.HexEditor.UpdateValues() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\HexEditor\HexEditor.cs:line 156
   at BizHawk.Client.EmuHawk.ToolManager.UpdateAfter() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\ToolManager.cs:line 445
   at BizHawk.Client.EmuHawk.ToolManager.UpdateToolsAfter(Boolean fromLua) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\ToolManager.cs:line 673
   at BizHawk.Client.EmuHawk.MainForm.StepRunLoop_Core(Boolean force) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\MainForm.cs:line 2988
   at BizHawk.Client.EmuHawk.MainForm.ProgramRunLoop() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\MainForm.cs:line 483
   at BizHawk.Client.EmuHawk.Program.SubMain(String[] args) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\Program.cs:line 227

Latest dev build

adelikat commented 6 years ago

I did some investigating. It happens on the first time the address 0xC0ED is read (but only the first one).

The virtu code eventually ends up calling this property in DiskIIDrive.cs public bool IsWriteProtected { get { return this._disk.IsWriteProtected; } }

and _disk is currently null.

I think some deeper digging needs to happen because i don't think it is intended that disk is null at this point, and some kind of initialization process was missed

Asnivor commented 5 years ago

Hmm, so it looks like the issue here isnt just that _disk is null. The disk controller supports 2 drives (although it looks like only 1 is ever used). Drive0 has the loaded disk in, and Drive1 (which is not used) has a null disk.

According to here:

http://yesterbits.com/media/pubs/AppleOrchard/articles/disk-ii-part-1-1983-apr.pdf

..the Apple IIe has soft switches that are triggered by reads (and sometimes writes) to the peripheral card IO page at $CO. However, we only currently have a single 'Peek()' method (which HexEditor and presumably other things like RAMSeach and RAMWatch are using).

So when you open HexEditor and scroll down to the bottom of the list, every soft switch is triggered once. So:

At this point the second drive is selected, so when you scroll back up in the HexEditor all the address are read again, but an exception is thrown when the floating bus read is attempted (presumably because it is interfacing with the null drive 2).

Obviously this is a terrible quantum mess at the moment.

So we need to rename the current 'Peek()' method to something that doesn't imply a non-destructive process (perhaps 'Read()') and create a new Peek() method that doesn't trigger the soft switches (and maybe still returns a floating bus value?).

Secondly, we need to eliminate exceptions thrown when any call is made to a null disk (because this is almost certainly possible on real hardware).

I'll try and get something added to the currently open PR #1394 when I have the chance.