05704: ddcrew: Crash on certain games when creating a save state. Related to bitmaps saved in state.

ID	Category [?]	Severity [?]	Reproducibility	Date Submitted	Last Update
05704	Crash/Freeze	Critical (emulator)	Always	Sep 23, 2014, 20:09	Nov 13, 2016, 12:30

Tester	Testweazle	View Status	Public	Platform	MAME (Official Binary)
Assigned To		Resolution	Fixed	OS	Windows XP
Status [?]	Resolved			Driver
Version	0.154	Fixed in Version		Build	Normal
		Fixed in Git Commit		Github Pull Request #

Summary	05704: ddcrew: Crash on certain games when creating a save state. Related to bitmaps saved in state.
Description	When creating a save state on certain games, such as ddcrew.zip (D.D. Crew) and mt_srage.zip (Megatech Streets of Rage), it will crash the emulator. The issue is described as follows by the developer of MAMEHub: ------------------------------------------------------------------------------------------------------------------------------------------------------------------ When mame starts ddcrew, it creates some bitmaps that are the size of the screen: https://github.com/MisterTea/MAMEHub/blob/master/Sources/Emulator/src/emu/video/315_5124.c#L1791 Those bitmaps are marked to remember that they should be the size of the screen here: https://github.com/MisterTea/MAMEHub/blob/master/Sources/Emulator/src/emu/screen.c#L800 At the same time, it adds a bitmap of that size to the save state: https://github.com/MisterTea/MAMEHub/blob/master/Sources/Emulator/src/emu/video/315_5124.c#L1825 The problem comes when you resize the screen (which happens automatically after a few frames if you have maximize set to 1), it resizes the bitmap, destroying the original: https://github.com/MisterTea/MAMEHub/blob/master/Sources/Emulator/src/emu/screen.c#L538 But the save state is still pointing to the original, hence the crash. ------------------------------------------------------------------------------------------------------------------------------------------------------------------ For the discussion thread on GitHub go here: https://github.com/MisterTea/MAMEHub/issues/252 It is very likely that the same issue is present in MESS as well (if so do I need to create a duplicate report for MESS?). I also tested the games on MAME 0.149 and I was able to create a save-state without any problems. The following access violation is produced after the crash: ----------------------------------------------------- Exception at EIP=77C46FA3 (register_frame_ctor+0x743b2993): ACCESS VIOLATION While attempting to read memory at 0920C000 ----------------------------------------------------- EAX=0920EED2 EBX=06288C08 ECX=00000BB5 EDX=00000000 ESI=0920BFFE EDI=06297404 EBP=0022B348 ESP=0022B340 ----------------------------------------------------- Stack crawl: 0022B348: 77C46FA3 (memcpy+0x0033) 0022B398: 021F3A23 (fill_window+0x0093) 0022B3D8: 021F3FA8 (deflate_slow+0x01c8) 0022B428: 021F5E3F (deflate+0x00ff) 0022B488: 02040C7F (core_fwrite(core_file, void const, unsigned int)+0x00ef) 0022B4A8: 01EF3EFB (emu_file::write(void const, unsigned int)+0x002b) 0022B508: 01EC1BB4 (save_manager::write_file(emu_file&)+0x0164) 0022B6F8: 01E875A2 (running_machine::handle_saveload()+0x01c2) 0022B7A8: 01E8A36D (running_machine::run(bool)+0x03cd) 0022F708: 01E9F16A (machine_manager::execute()+0x05aa) 0022F978: 01F3101B (cli_frontend::execute(int, char)+0x0f0b) 0022FEB8: 01332635 (utf8_main(int, char*)+0x04a5) 0022FEF8: 021FA8DE (wmain+0x006e) 0022FFC0: 0040140B (__tmainCRTStartup+0x028b) 0022FFF0: 7C81776F (RegisterWaitForInputIdle+0x0049)
Steps To Reproduce
Additional Information	Removing the bitmaps from the save states fixes the crash. This has been tested in the custom build used by MAMEHub. Save states now function normally.
Github Commit

Flags
Regression Version
Affected Sets / Systems	ddcrew

Attached Files

No.10994 Osso Moderator Sep 24, 2014, 11:04	AFAIK both of the drivers you mentioned don't support save states. MAME even tells this when the user tries to save or load a state.
No.10995 Testweazle Tester Sep 24, 2014, 14:50 edited on: Sep 24, 2014, 14:58	Thanks for your reply! Perhaps the bitmaps are exactly the reason why these games are marked as not having complete save state support? The build used by MAMEHub removes these bitmaps from the state and as a result save states function normally. No crash. My apologies if this is a known issue & solution; in which case feel free to close it.
No.10997 NekoEd Senior Tester Sep 24, 2014, 17:06	I'm not sure what to do with this one. Technically it should be closed, because save states are not supported for these games. However, a potential solution is discussed, which I'm thinking some devs may want to take a look at. I'll leave this open for now to get some developer opinions.
No.10998 Haze Senior Tester Sep 24, 2014, 17:48	It sounds to me like it's a bug in the Megadrive / Genesis VDP save states? I think etabeta considers those 'working' (although I've had various issues with it) so it's probably a legitimate report.
No.11000 etabeta Developer Sep 24, 2014, 18:40	I marked them as working because I was able to use extensively save states with no issues in 3 games (one being sonic 1, I don't recall the other two), and I have randomly tested another dozen with no issues either but it might well be that I did not test any game which was changing resolution across my save/restore processes, which means that the reported problem would not have been visible Haze, if you see no easy fix (you're the real VDP expert having written the code in first place), please demote it from SUPPORTS_SAVE_STATES
No.11002 Osso Moderator Sep 25, 2014, 08:46	In debug builds they crash trying to save the state.
No.11005 rcoltrane Tester Sep 25, 2014, 19:27 edited on: Sep 25, 2014, 19:28	Here it crashes too but it's not a 'new' bug. It's happening since this driver was added to MAME. It saves the state but crashes when you try to load the state. I believe it happens to all System18 games because I think astorm3 had the same issue, but I don't have that game anymore so I cannot test it again.
No.11010 NekoEd Senior Tester Sep 26, 2014, 01:42	I'm going to promote this to Confirmed. Now whether the fix is to demote the games from GAME_SUPPORTS_SAVE or to implement the proposed fix is up to devs.
No.11013 Osso Moderator Sep 26, 2014, 05:51 edited on: Sep 26, 2014, 05:51	The games are not flagged GAME_SUPPORTS_SAVE, but anyway..
No.11261 Haze Senior Tester Nov 21, 2014, 14:40 edited on: Nov 21, 2014, 14:55	fwiw looking at this further those bitmaps are from the SMS vdp implementation m_screen->register_screen_bitmap(m_tmpbitmap); m_screen->register_screen_bitmap(m_y1_bitmap) the Genesis VDP currently inherits from the SMS vdp in order to be able to run in SMS mode (a feature of the chip) but the SMS code has bitmaps and buffers allocated based on the screen sizes, with the genesis code then changing the screen size this fails. registering bitmaps of the screen size, then saving them when the screen size can change simply doesn't seem to be safe, strikes me as more of a core problem as the same would happen in any driver that attempted to do this.
No.13358 Fujix Administrator Nov 13, 2016, 12:30 edited on: Nov 15, 2016, 11:18	Confirmed no more crash on saving states both in ddcrew and mt_srange in 0.179. I closed the report.