SaveState: On section version failure, log out the name of the bad section.

Also some other minor improvements to logging and comments.
This commit is contained in:
Henrik Rydgård 2020-08-02 17:11:09 +02:00
parent dee0f3f9ec
commit bf4db22165
5 changed files with 55 additions and 57 deletions

View File

@ -30,23 +30,26 @@ PointerWrapSection PointerWrap::Section(const char *title, int minVer, int ver)
char marker[16] = {0};
int foundVersion = ver;
// This is strncpy because we rely on its weird non-null-terminating truncation behaviour.
// This is strncpy because we rely on its weird non-null-terminating zero-filling truncation behaviour.
// Can't replace it with the more sensible truncate_cpy because that would break savestates.
strncpy(marker, title, sizeof(marker));
if (!ExpectVoid(marker, sizeof(marker)))
{
if (!ExpectVoid(marker, sizeof(marker))) {
// Might be before we added name markers for safety.
if (foundVersion == 1 && ExpectVoid(&foundVersion, sizeof(foundVersion)))
if (foundVersion == 1 && ExpectVoid(&foundVersion, sizeof(foundVersion))) {
DoMarker(title);
// Wasn't found, but maybe we can still load the state.
else
} else {
// Wasn't found, but maybe we can still load the state.
foundVersion = 0;
}
else
}
} else {
Do(foundVersion);
}
if (error == ERROR_FAILURE || foundVersion < minVer || foundVersion > ver) {
WARN_LOG(SAVESTATE, "Savestate failure: wrong version %d found for %s", foundVersion, title);
if (!firstBadSectionTitle_) {
firstBadSectionTitle_ = title;
}
WARN_LOG(SAVESTATE, "Savestate failure: wrong version %d found for section '%s'", foundVersion, title);
SetError(ERROR_FAILURE);
return PointerWrapSection(*this, -1, title);
}
@ -58,6 +61,7 @@ void PointerWrap::SetError(Error error_) {
error = error_;
}
if (error > ERROR_WARNING) {
// For the rest of this run, just measure.
mode = PointerWrap::MODE_MEASURE;
}
}

View File

@ -76,6 +76,7 @@ private:
// Wrapper class
class PointerWrap
{
private:
// This makes it a compile error if you forget to define DoState() on non-POD.
// Which also can be a problem, for example struct tm is non-POD on linux, for whatever reason...
#ifdef _MSC_VER
@ -111,6 +112,8 @@ class PointerWrap
}
};
const char *firstBadSectionTitle_ = nullptr;
public:
enum Mode {
MODE_READ = 1, // load
@ -127,11 +130,11 @@ public:
u8 **ptr;
Mode mode;
Error error;
Error error = ERROR_NONE;
public:
PointerWrap(u8 **ptr_, Mode mode_) : ptr(ptr_), mode(mode_), error(ERROR_NONE) {}
PointerWrap(unsigned char **ptr_, int mode_) : ptr((u8**)ptr_), mode((Mode)mode_), error(ERROR_NONE) {}
PointerWrap(u8 **ptr_, Mode mode_) : ptr(ptr_), mode(mode_) {}
PointerWrap(unsigned char **ptr_, int mode_) : ptr((u8**)ptr_), mode((Mode)mode_) {}
PointerWrapSection Section(const char *title, int ver);
@ -145,6 +148,10 @@ public:
u8 **GetPPtr() {return ptr;}
void SetError(Error error_);
const char *GetBadSectionTitle() const {
return firstBadSectionTitle_;
}
// Same as DoVoid, except doesn't advance pointer if it doesn't match on read.
bool ExpectVoid(void *data, int size);
void DoVoid(void *data, int size);
@ -586,7 +593,7 @@ public:
// May fail badly if ptr doesn't point to valid data.
template<class T>
static Error LoadPtr(u8 *ptr, T &_class)
static Error LoadPtr(u8 *ptr, T &_class, std::string *errorString)
{
PointerWrap p(&ptr, PointerWrap::MODE_READ);
_class.DoState(p);
@ -594,6 +601,7 @@ public:
if (p.error != p.ERROR_FAILURE) {
return ERROR_NONE;
} else {
*errorString = std::string("Failure at ") + p.GetBadSectionTitle();
return ERROR_BROKEN_STATE;
}
}
@ -630,14 +638,13 @@ public:
u8 *ptr = nullptr;
size_t sz;
Error error = LoadFile(filename, gitVersion, ptr, sz, failureReason);
if (error == ERROR_NONE) {
error = LoadPtr(ptr, _class);
delete [] ptr;
}
INFO_LOG(SAVESTATE, "ChunkReader: Done loading %s", filename.c_str());
if (error == ERROR_NONE) {
failureReason->clear();
error = LoadPtr(ptr, _class, failureReason);
delete [] ptr;
INFO_LOG(SAVESTATE, "ChunkReader: Done loading '%s'", filename.c_str());
} else {
WARN_LOG(SAVESTATE, "ChunkReader: Error found during load of '%s'", filename.c_str());
}
return error;
}

View File

@ -91,9 +91,9 @@ namespace SaveState
return CChunkFileReader::SavePtr(&data[0], state);
}
CChunkFileReader::Error LoadFromRam(std::vector<u8> &data) {
CChunkFileReader::Error LoadFromRam(std::vector<u8> &data, std::string *errorString) {
SaveStart state;
return CChunkFileReader::LoadPtr(&data[0], state);
return CChunkFileReader::LoadPtr(&data[0], state, errorString);
}
struct StateRingbuffer
@ -135,7 +135,7 @@ namespace SaveState
return err;
}
CChunkFileReader::Error Restore()
CChunkFileReader::Error Restore(std::string *errorString)
{
std::lock_guard<std::mutex> guard(lock_);
@ -149,7 +149,7 @@ namespace SaveState
static std::vector<u8> buffer;
LockedDecompress(buffer, states_[n], bases_[baseMapping_[n]]);
return LoadFromRam(buffer);
return LoadFromRam(buffer, errorString);
}
void ScheduleCompress(std::vector<u8> *result, const std::vector<u8> *state, const std::vector<u8> *base)
@ -632,13 +632,14 @@ namespace SaveState
return copy;
}
bool HandleFailure()
bool HandleLoadFailure()
{
// Okay, first, let's give the rewind state a shot - maybe we can at least not reset entirely.
// Even if this was a rewind, maybe we can still load a previous one.
CChunkFileReader::Error result;
do {
result = rewindStates.Restore();
std::string errorString;
result = rewindStates.Restore(&errorString);
} while (result == CChunkFileReader::ERROR_BROKEN_STATE);
if (result == CChunkFileReader::ERROR_NONE) {
@ -728,7 +729,6 @@ namespace SaveState
Status callbackResult;
bool tempResult;
std::string callbackMessage;
std::string reason;
std::string title;
auto sc = GetI18NCategory("Screen");
@ -740,13 +740,14 @@ namespace SaveState
i18nSaveFailure = sc->T("Failed to save state");
std::string slot_prefix = op.slot >= 0 ? StringFromFormat("(%d) ", op.slot + 1) : "";
std::string errorString;
switch (op.type)
{
case SAVESTATE_LOAD:
INFO_LOG(SAVESTATE, "Loading state from '%s'", op.filename.c_str());
// Use the state's latest version as a guess for saveStateInitialGitVersion.
result = CChunkFileReader::Load(op.filename, &saveStateInitialGitVersion, state, &reason);
result = CChunkFileReader::Load(op.filename, &saveStateInitialGitVersion, state, &errorString);
if (result == CChunkFileReader::ERROR_NONE) {
callbackMessage = slot_prefix + sc->T("Loaded State");
callbackResult = Status::SUCCESS;
@ -777,12 +778,13 @@ namespace SaveState
}
#endif
} else if (result == CChunkFileReader::ERROR_BROKEN_STATE) {
HandleFailure();
callbackMessage = i18nLoadFailure;
ERROR_LOG(SAVESTATE, "Load state failure: %s", reason.c_str());
HandleLoadFailure();
callbackMessage = std::string(i18nLoadFailure) + ": " + errorString;
ERROR_LOG(SAVESTATE, "Load state failure: %s", errorString.c_str());
callbackResult = Status::FAILURE;
} else {
callbackMessage = sc->T(reason.c_str(), i18nLoadFailure);
// ?
callbackMessage = sc->T(errorString.c_str(), i18nLoadFailure);
callbackResult = Status::FAILURE;
}
break;
@ -812,9 +814,9 @@ namespace SaveState
}
#endif
} else if (result == CChunkFileReader::ERROR_BROKEN_STATE) {
HandleFailure();
// TODO: What else might we want to do here? This should be very unusual.
callbackMessage = i18nSaveFailure;
ERROR_LOG(SAVESTATE, "Save state failure: %s", reason.c_str());
ERROR_LOG(SAVESTATE, "Save state failure");
callbackResult = Status::FAILURE;
} else {
callbackMessage = i18nSaveFailure;
@ -834,24 +836,24 @@ namespace SaveState
case SAVESTATE_REWIND:
INFO_LOG(SAVESTATE, "Rewinding to recent savestate snapshot");
result = rewindStates.Restore();
result = rewindStates.Restore(&errorString);
if (result == CChunkFileReader::ERROR_NONE) {
callbackMessage = sc->T("Loaded State");
callbackResult = Status::SUCCESS;
hasLoadedState = true;
} else if (result == CChunkFileReader::ERROR_BROKEN_STATE) {
// Cripes. Good news is, we might have more. Let's try those too, better than a reset.
if (HandleFailure()) {
if (HandleLoadFailure()) {
// Well, we did rewind, even if too much...
callbackMessage = sc->T("Loaded State");
callbackResult = Status::SUCCESS;
hasLoadedState = true;
} else {
callbackMessage = i18nLoadFailure;
callbackMessage = std::string(i18nLoadFailure) + ": " + errorString;
callbackResult = Status::FAILURE;
}
} else {
callbackMessage = i18nLoadFailure;
callbackMessage = std::string(i18nLoadFailure) + ": " + errorString;
callbackResult = Status::FAILURE;
}
break;

View File

@ -69,7 +69,7 @@ namespace SaveState
void Save(const std::string &filename, int slot, Callback callback = Callback(), void *cbUserData = 0);
CChunkFileReader::Error SaveToRam(std::vector<u8> &state);
CChunkFileReader::Error LoadFromRam(std::vector<u8> &state);
CChunkFileReader::Error LoadFromRam(std::vector<u8> &state, std::string *errorString);
// For testing / automated tests. Runs a save state verification pass (async.)
// Warning: callback will be called on a different thread.

View File

@ -1439,13 +1439,15 @@ void EmuScreen::render() {
return;
}
// Freeze-frame functionality (loads a savestate on every frame).
if (PSP_CoreParameter().freezeNext) {
PSP_CoreParameter().frozen = true;
PSP_CoreParameter().freezeNext = false;
SaveState::SaveToRam(freezeState_);
} else if (PSP_CoreParameter().frozen) {
if (CChunkFileReader::ERROR_NONE != SaveState::LoadFromRam(freezeState_)) {
ERROR_LOG(SAVESTATE, "Failed to load freeze state. Unfreezing.");
std::string errorString;
if (CChunkFileReader::ERROR_NONE != SaveState::LoadFromRam(freezeState_, &errorString)) {
ERROR_LOG(SAVESTATE, "Failed to load freeze state (%s). Unfreezing.", errorString.c_str());
PSP_CoreParameter().frozen = false;
}
}
@ -1503,23 +1505,6 @@ void EmuScreen::render() {
screenManager()->getUIContext()->BeginFrame();
renderUI();
}
// We have no use for backbuffer depth or stencil, so let tiled renderers discard them after tiling.
/*
if (gl_extensions.GLES3 && glInvalidateFramebuffer != nullptr) {
GLenum attachments[2] = { GL_DEPTH, GL_STENCIL };
glInvalidateFramebuffer(GL_FRAMEBUFFER, 2, attachments);
} else if (!gl_extensions.GLES3) {
#ifdef USING_GLES2
// Tiled renderers like PowerVR should benefit greatly from this. However - seems I can't call it?
bool hasDiscard = gl_extensions.EXT_discard_framebuffer; // TODO
if (hasDiscard) {
//const GLenum targets[3] = { GL_COLOR_EXT, GL_DEPTH_EXT, GL_STENCIL_EXT };
//glDiscardFramebufferEXT(GL_FRAMEBUFFER, 3, targets);
}
#endif
}
*/
}
bool EmuScreen::hasVisibleUI() {