diff --git a/Common/Serialize/Serializer.cpp b/Common/Serialize/Serializer.cpp index 6b36fb2167..135bef6bce 100644 --- a/Common/Serialize/Serializer.cpp +++ b/Common/Serialize/Serializer.cpp @@ -33,8 +33,25 @@ enum class SerializeCompressType { static constexpr SerializeCompressType SAVE_TYPE = SerializeCompressType::ZSTD; -PointerWrapSection PointerWrap::Section(const char *title, int ver) { - return Section(title, ver, ver); +void PointerWrap::RewindForWrite(u8 *writePtr) { + _assert_(mode == MODE_MEASURE); + // Switch to writing mode and + mode = MODE_WRITE; + *ptr = writePtr; + ptrStart_ = writePtr; +} + +bool PointerWrap::CheckAfterWrite() { + _assert_(mode == MODE_WRITE); + if (measuredSize_ != 0 && Offset() != measuredSize_) { + WARN_LOG(SAVESTATE, "CheckAfterWrite: Size mismatch! %d vs %d", (int)Offset(), (int)measuredSize_); + return false; + } + if (!checkpoints_.empty() && curCheckpoint_ != checkpoints_.size()) { + WARN_LOG(SAVESTATE, "Checkpoint count mismatch!"); + return false; + } + return true; } PointerWrapSection PointerWrap::Section(const char *title, int minVer, int ver) { @@ -44,6 +61,30 @@ PointerWrapSection PointerWrap::Section(const char *title, int minVer, int ver) // 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)); + + // Compare the measure and write passes. Sanity check to catch bugs, doesn't do anything for output. + if (mode == MODE_MEASURE) { + checkpoints_.emplace_back(marker, Offset()); + } else if (mode == MODE_WRITE) { + if (!checkpoints_.empty()) { + if (checkpoints_.size() <= curCheckpoint_) { + WARN_LOG(SAVESTATE, "Write: Not enough checkpoints from measure pass (%d). cur section: %s", (int)checkpoints_.size(), title); + SetError(ERROR_FAILURE); + return PointerWrapSection(*this, -1, title); + } + if (!checkpoints_[curCheckpoint_].Matches(marker, Offset())) { + WARN_LOG(SAVESTATE, "Checkpoint mismatch during write! Section %s vs %s, offset %d vs %d", title, marker, (int)Offset(), (int)checkpoints_[curCheckpoint_].offset); + if (curCheckpoint_ > 1) { + WARN_LOG(SAVESTATE, "Previous checkpoint: %s (%d)", checkpoints_[curCheckpoint_ - 1].title, (int)checkpoints_[curCheckpoint_ - 1].offset); + } + SetError(ERROR_FAILURE); + return PointerWrapSection(*this, -1, title); + } + } else { + WARN_LOG(SAVESTATE, "Writing savestate without checkpoints. This is OK but should be fixed."); + } + } + if (!ExpectVoid(marker, sizeof(marker))) { // Might be before we added name markers for safety. if (foundVersion == 1 && ExpectVoid(&foundVersion, sizeof(foundVersion))) { diff --git a/Common/Serialize/Serializer.h b/Common/Serialize/Serializer.h index cdf8bd4f06..5db52f4347 100644 --- a/Common/Serialize/Serializer.h +++ b/Common/Serialize/Serializer.h @@ -30,6 +30,7 @@ // - Serialization code for anything complex has to be manually written. #include +#include #include #include @@ -52,8 +53,7 @@ class PointerWrap; class PointerWrapSection { public: - PointerWrapSection(PointerWrap &p, int ver, const char *title) : p_(p), ver_(ver), title_(title) { - } + PointerWrapSection(PointerWrap &p, int ver, const char *title) : p_(p), ver_(ver), title_(title) {} ~PointerWrapSection(); bool operator == (const int &v) const { return ver_ == v; } @@ -73,6 +73,22 @@ private: const char *title_; }; +// For measure vs write detailed verification +struct SerializeCheckpoint { + char title[17]; // 16-byte section header, plus a zero terminator for debug printing. + size_t offset; + + SerializeCheckpoint(char _title[16], size_t off) { + memcpy(title, _title, 16); + title[16] = 0; + offset = off; + } + + bool Matches(const char *_title, size_t off) const { + return memcmp(title, _title, 16) == 0 && off == offset; + } +}; + // Wrapper class class PointerWrap { @@ -94,19 +110,23 @@ public: Mode mode; Error error = ERROR_NONE; - PointerWrap(u8 **ptr_, Mode mode_) : ptr(ptr_), mode(mode_) {} - PointerWrap(unsigned char **ptr_, int mode_) : ptr((u8**)ptr_), mode((Mode)mode_) {} + PointerWrap(u8 **ptr_, Mode mode_) : ptr(ptr_), ptrStart_(*ptr), mode(mode_) {} + PointerWrap(unsigned char **ptr_, int mode_) : ptr((u8**)ptr_), ptrStart_(*ptr), mode((Mode)mode_) {} - PointerWrapSection Section(const char *title, int ver); + void RewindForWrite(u8 *writePtr); + bool CheckAfterWrite(); // The returned object can be compared against the version that was loaded. // This can be used to support versions as old as minVer. // Version = 0 means the section was not found. PointerWrapSection Section(const char *title, int minVer, int ver); + PointerWrapSection Section(const char *title, int ver) { + return Section(title, ver, ver); + } - void SetMode(Mode mode_) {mode = mode_;} - Mode GetMode() const {return mode;} - u8 **GetPPtr() {return ptr;} + void SetMode(Mode mode_) { mode = mode_; } + Mode GetMode() const { return mode; } + u8 **GetPPtr() { return ptr; } void SetError(Error error_); const char *GetBadSectionTitle() const { @@ -120,7 +140,13 @@ public: void DoMarker(const char *prevName, u32 arbitraryNumber = 0x42); private: + size_t Offset() const { return *ptr - ptrStart_; } + const char *firstBadSectionTitle_ = nullptr; + u8 *ptrStart_; + std::vector checkpoints_; + size_t curCheckpoint_ = 0; + size_t measuredSize_ = 0; }; class CChunkFileReader @@ -204,7 +230,7 @@ public: return ERROR_BAD_ALLOC; Error error = SavePtr(buffer, _class, sz); - // SaveFile takes ownership of buffer + // SaveFile takes ownership of buffer (malloc/free) if (error == ERROR_NONE) error = SaveFile(filename, title, gitVersion, buffer, sz); return error;