From c68c8c388cc29cfd4e97be8fc7f5ea7534508672 Mon Sep 17 00:00:00 2001 From: nitsuja Date: Sat, 17 Dec 2011 16:49:24 -0800 Subject: [PATCH] made savestate loads less fragile by adding some markers and rolling back on a mismatch. This should make it so if you try to load an incompatible save, it simply doesn't load, instead of crashing dolphin. (I can't guarantee no crash but it's much less likely now) --- Source/Core/Common/Src/ChunkFile.h | 11 ++++++++++ Source/Core/Core/Src/CoreTiming.cpp | 2 ++ Source/Core/Core/Src/HW/HW.cpp | 12 +++++++++++ Source/Core/Core/Src/HW/Memmap.cpp | 3 +++ Source/Core/Core/Src/State.cpp | 33 +++++++++++++++++++++++------ 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/Source/Core/Common/Src/ChunkFile.h b/Source/Core/Common/Src/ChunkFile.h index 03216deaf1..b0662d2517 100644 --- a/Source/Core/Common/Src/ChunkFile.h +++ b/Source/Core/Common/Src/ChunkFile.h @@ -170,6 +170,17 @@ public: // TODO PanicAlert("Do(linked list<>) does not yet work."); } + + void DoMarker(const char* prevName, u32 arbitraryNumber=0x42) + { + u32 cookie = arbitraryNumber; + Do(cookie); + if(mode == PointerWrap::MODE_READ && cookie != arbitraryNumber) + { + PanicAlertT("Error: After \"%s\", found %d (0x%X) instead of save marker %d (0x%X). Aborting savestate load...", prevName, cookie, cookie, arbitraryNumber, arbitraryNumber); + mode = PointerWrap::MODE_MEASURE; + } + } }; diff --git a/Source/Core/Core/Src/CoreTiming.cpp b/Source/Core/Core/Src/CoreTiming.cpp index 9aacea5847..979add358b 100644 --- a/Source/Core/Core/Src/CoreTiming.cpp +++ b/Source/Core/Core/Src/CoreTiming.cpp @@ -163,6 +163,7 @@ void DoState(PointerWrap &p) p.Do(fakeDecStartTicks); p.Do(fakeTBStartValue); p.Do(fakeTBStartTicks); + p.DoMarker("CoreTimingData"); // OK, here we're gonna need to specialize depending on the mode. // Should do something generic to serialize linked lists. switch (p.GetMode()) { @@ -209,6 +210,7 @@ void DoState(PointerWrap &p) break; } } + p.DoMarker("CoreTimingEvents"); } u64 GetTicks() diff --git a/Source/Core/Core/Src/HW/HW.cpp b/Source/Core/Core/Src/HW/HW.cpp index ec8aa7063f..a078955815 100644 --- a/Source/Core/Core/Src/HW/HW.cpp +++ b/Source/Core/Core/Src/HW/HW.cpp @@ -89,18 +89,30 @@ namespace HW void DoState(PointerWrap &p) { Memory::DoState(p); + p.DoMarker("Memory"); VideoInterface::DoState(p); + p.DoMarker("VideoInterface"); SerialInterface::DoState(p); + p.DoMarker("SerialInterface"); ProcessorInterface::DoState(p); + p.DoMarker("ProcessorInterface"); DSP::DoState(p); + p.DoMarker("DSP"); DVDInterface::DoState(p); + p.DoMarker("DVDInterface"); GPFifo::DoState(p); + p.DoMarker("GPFifo"); ExpansionInterface::DoState(p); + p.DoMarker("ExpansionInterface"); AudioInterface::DoState(p); + p.DoMarker("AudioInterface"); if (SConfig::GetInstance().m_LocalCoreStartupParameter.bWii) { WII_IPCInterface::DoState(p); + p.DoMarker("WII_IPCInterface"); WII_IPC_HLE_Interface::DoState(p); + p.DoMarker("WII_IPC_HLE_Interface"); } + p.DoMarker("WIIHW"); } } diff --git a/Source/Core/Core/Src/HW/Memmap.cpp b/Source/Core/Core/Src/HW/Memmap.cpp index 980b080761..f26ac76a14 100644 --- a/Source/Core/Core/Src/HW/Memmap.cpp +++ b/Source/Core/Core/Src/HW/Memmap.cpp @@ -373,10 +373,13 @@ void DoState(PointerWrap &p) p.DoArray(m_pPhysicalRAM, RAM_SIZE); // p.DoArray(m_pVirtualEFB, EFB_SIZE); p.DoArray(m_pVirtualL1Cache, L1_CACHE_SIZE); + p.DoMarker("Memory RAM"); if (bFakeVMEM) p.DoArray(m_pVirtualFakeVMEM, FAKEVMEM_SIZE); + p.DoMarker("Memory FakeVMEM"); if (wii) p.DoArray(m_pEXRAM, EXRAM_SIZE); + p.DoMarker("Memory EXRAM"); } void Shutdown() diff --git a/Source/Core/Core/Src/State.cpp b/Source/Core/Core/Src/State.cpp index 7cfcba66d5..e8dcddcb62 100644 --- a/Source/Core/Core/Src/State.cpp +++ b/Source/Core/Core/Src/State.cpp @@ -64,6 +64,7 @@ static std::string g_current_filename, g_last_filename; // Temporary undo state buffer static std::vector g_undo_load_buffer; static std::vector g_current_buffer; +static int g_loadDepth = 0; static std::thread g_save_thread; @@ -74,7 +75,7 @@ static u64 lastCheckedStates[NUM_HOOKS]; static u8 hook; // Don't forget to increase this after doing changes on the savestate system -static const int STATE_VERSION = 6; +static const int STATE_VERSION = 7; struct StateHeader { @@ -121,18 +122,26 @@ void DoState(PointerWrap &p) } } + p.DoMarker("Version"); + // Begin with video backend, so that it gets a chance to clear it's caches and writeback modified things to RAM // Pause the video thread in multi-threaded mode g_video_backend->RunLoop(false); g_video_backend->DoState(p); + p.DoMarker("video_backend"); if (Core::g_CoreStartupParameter.bWii) Wiimote::DoState(p.GetPPtr(), p.GetMode()); + p.DoMarker("Wiimote"); PowerPC::DoState(p); + p.DoMarker("PowerPC"); HW::DoState(p); + p.DoMarker("HW"); CoreTiming::DoState(p); + p.DoMarker("CoreTiming"); Movie::DoState(p, version<6); + p.DoMarker("Movie"); // Resume the video thread g_video_backend->RunLoop(true); @@ -353,6 +362,8 @@ void LoadFileStateCallback(u64 userdata, int cyclesLate) // Stop the core while we load the state CCPU::EnableStepping(true); + g_loadDepth++; + Flush(); // Save temp buffer for undo load state @@ -371,20 +382,30 @@ void LoadFileStateCallback(u64 userdata, int cyclesLate) DoState(p); if (p.GetMode() == PointerWrap::MODE_READ) + { Core::DisplayMessage(StringFromFormat("Loaded state from %s", g_current_filename.c_str()).c_str(), 2000); + if (File::Exists(g_current_filename + ".dtm")) + Movie::LoadInput((g_current_filename + ".dtm").c_str()); + else if (!Movie::IsJustStartingRecordingInputFromSaveState()) + Movie::EndPlayInput(false); + } else + { + // failed to load Core::DisplayMessage("Unable to Load : Can't load state from other revisions !", 4000); - - if (File::Exists(g_current_filename + ".dtm")) - Movie::LoadInput((g_current_filename + ".dtm").c_str()); - else if (!Movie::IsJustStartingRecordingInputFromSaveState()) - Movie::EndPlayInput(false); + + // since we're probably in an inconsistent state now (and might crash or whatever), undo. + if(g_loadDepth < 2) + UndoLoadState(); + } } ResetCounters(); g_op_in_progress = false; + g_loadDepth--; + // resume dat core CCPU::EnableStepping(false); }