Skip to content
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.

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

Certain instances of PeekByte() before controller initialization causes crashes (e.g. rebooting some cores with the hex editor open) #1168

Closed
NarryG opened this issue Mar 31, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@NarryG
Copy link
Contributor

commented Mar 31, 2018

With various cores, it appears that the controllers aren't initialized before the core load is complete. This means if you use PeekByte() on an address that results in a read to the IController object before the controller has been initialized, a crash occurs.

The easiest place to demonstrate this is with the TIA in 2600Hawk. Open the hex editor, swap to TIA, and reload the core. It should crash every time.

Another place where the same issue exists is in PCEHawk. Open the hex editor, go into the System Bus (21 bit), scroll down all the way to somewhere around 0x1FF332, reboot the core, same crash occurs.

This issue seems isolated to certain cores. I'm unsure if this affects other cores, but the root cause seems to be that the controller object doesn't exist at the time of the PeekByte()

Example crash log

   at BizHawk.Emulation.Cores.PCEngine.StandardController.Read(IController c, Boolean sel) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Emulation.Cores\Consoles\PC Engine\PceControllerDeck.cs:line 139
   at BizHawk.Emulation.Cores.PCEngine.PCEngine.ReadInput() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Emulation.Cores\Consoles\PC Engine\PCEngine.Input.cs:line 38
   at BizHawk.Emulation.Cores.PCEngine.PCEngine.ReadMemorySF2(Int32 addr) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Emulation.Cores\Consoles\PC Engine\MemoryMap.SF2.cs:line 32
   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.HexEditor.SetDataSize(Int32 size) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\HexEditor\HexEditor.cs:line 773
   at BizHawk.Client.EmuHawk.HexEditor.Restart() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\HexEditor\HexEditor.cs:line 194
   at BizHawk.Client.EmuHawk.ToolManager.Restart() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\tools\ToolManager.cs:line 493
   at BizHawk.Client.EmuHawk.MainForm.LoadRom(String path, LoadRomArgs args) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\MainForm.cs:line 3680
   at BizHawk.Client.EmuHawk.MainForm.CheckHotkey(String trigger) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\MainForm.Hotkey.cs:line 125
   at BizHawk.Client.EmuHawk.MainForm.<ProcessInput>b__94_1(Boolean current, String trigger) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\MainForm.cs:line 770
   at System.Linq.Enumerable.Aggregate[TSource,TAccumulate](IEnumerable`1 source, TAccumulate seed, Func`3 func)
   at BizHawk.Client.EmuHawk.MainForm.ProcessInput() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\MainForm.cs:line 770
   at BizHawk.Client.EmuHawk.MainForm.ProgramRunLoop() in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\MainForm.cs:line 454
   at BizHawk.Client.EmuHawk.Program.SubMain(String[] args) in C:\Users\Daniel\Documents\BizHawk\BizHawk.Client.EmuHawk\Program.cs:line 227

Occurs in the latest dev build

@NarryG NarryG changed the title PeekByte() before controller initialization causes crashes (e.g. rebooting some cores with the hex editor open) Certain instances of PeekByte() before controller initialization causes crashes (e.g. rebooting some cores with the hex editor open) Mar 31, 2018

@alyosha-tas

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2018

The controller is initialized in EmuHawk but not in the core. Currently the only access the core has to the controller is through FrameAdvance.

Some cores that don't crash have indirect variables that can return a default value before a FrameAdvance call updates them. A7800 is an example.

NESHawk avoids the problem by not reading the controller during peek. It makes sense here because reading controllers in NES updates the state.

Ported cores don't have an independent instance of the controller at all since they don't need it.

So yeah, this is really inconsistent. Cores aren't required to have an independent instance of IController around, but currently most in house cores do. Probably they shouldn't? I'll think about it.

EDIT: looks like the easiest thing to do is to just initialize all such instances with a null controller.

@alyosha-tas alyosha-tas self-assigned this Mar 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.