From b42a03e09533ebc6504bf8adfac5b6f7dc15c322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 17 Aug 2026 12:06:35 +0200 Subject: [PATCH] Add savestate serializer tests, fix three bounds-check bugs PointerWrap and the Do() overloads around it are how every savestate is written and read, and had no direct coverage. Everything read back came off disk, so the corrupt-input paths matter as much as the round trips. Three bugs, all in the bounds checking added in 58d4759ceb: 1. sizeof(T) is not a lower bound on how many bytes an element serializes to. It only holds for the types DoHelper_ writes out raw. A std::string is 32-40 bytes in memory and serializes to as few as five; a T* serializes to whatever T::DoState() writes. So DoVector/DoList/DoSet/DoMap could reject a perfectly valid savestate whenever count * sizeof(element) exceeded the bytes left in the buffer. That is not hypothetical: pspFileSystem is serialized dead last in SaveStart::DoState, and MetaFileSystem::DoState does Do(p, currentDir) on a std::map, so the check runs with only a few hundred bytes remaining and claims 44 bytes per entry against roughly 22 actual. Added SerializeMinElemSize(), mirroring DoHelper_'s own condition, and used it in all five containers. The bound is only loosened, so nothing that loaded before can stop loading. 2. Do(p, std::map &) deletes every value before reading the new ones, and DoMap then returned on a bad count without clearing - leaving the map full of freed pointers to be used or deleted again. Six live maps go through this (sceMpeg, sceMp3, sceAac, sceFont, sceHeap, sceKernelThread's pending calls), so a corrupt savestate meant a use-after-free. Clear before the guard can bail out, in DoMap, DoMultimap and DoSet. 3. The wstring and u16string overloads validated stringLen < 0 but not 0, and didn't require a whole number of characters. read() computes stringLen / sizeof(char) - 1, so a length of 0 resized to SIZE_MAX and memcpy'd with a wrapped-around size. PSPOskDialog::DoState serializes both (inputChars at v2, a legacy wstring below that), so this was reachable: the test aborts the process without the fix. The test covers round trips of PODs, strings (empty, embedded NUL), vector, map, set, list and map-of-pointers, section titles and version gating in both directions, marker mismatches, measure-vs-write checkpoint disagreement, the error latch dropping to MODE_NOOP, every truncation of a valid buffer, and hand-corrupted counts and lengths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Common/Serialize/SerializeFuncs.h | 16 +- Common/Serialize/SerializeList.h | 2 +- Common/Serialize/SerializeMap.h | 12 +- Common/Serialize/SerializeSet.h | 6 +- Common/Serialize/Serializer.cpp | 8 +- unittest/UnitTest.cpp | 405 ++++++++++++++++++++++++++++++ 6 files changed, 437 insertions(+), 12 deletions(-) diff --git a/Common/Serialize/SerializeFuncs.h b/Common/Serialize/SerializeFuncs.h index 6d2ff1d914..c8c8eb4b33 100644 --- a/Common/Serialize/SerializeFuncs.h +++ b/Common/Serialize/SerializeFuncs.h @@ -85,6 +85,17 @@ void DoArray(PointerWrap &p, T *x, int count) { DoHelper_::DoArray(p, x, count); } +// Lower bound on the bytes one element of a serialized container takes up, used to sanity check +// element counts read from a savestate against how much buffer is actually left. +// sizeof(T) is only valid for the types DoHelper_ writes out raw - the same condition as its +// specialization above. Anything with its own Do()/DoState() (a string, a pointer to a class, a +// nested container) routinely serializes far fewer bytes than it occupies in memory, and using +// sizeof(T) for those rejects perfectly good savestates. +template +constexpr size_t SerializeMinElemSize() { + return std::is_standard_layout::value && std::is_trivial::value && !std::is_pointer::value ? sizeof(T) : 1; +} + template void Do(PointerWrap &p, T &x) { DoHelper_::DoThing(p, x); @@ -95,10 +106,9 @@ void DoVector(PointerWrap &p, std::vector &x, T &default_val) { u32 vec_size = (u32)x.size(); Do(p, vec_size); // Guard against an attacker-controlled size that would both resize the - // vector hugely and read past the end of the buffer. sizeof(T) is a lower - // bound on the bytes consumed per element for most uses. + // vector hugely and read past the end of the buffer. if (p.mode == PointerWrap::MODE_READ || p.mode == PointerWrap::MODE_VERIFY) { - if (vec_size > p.Remaining() / sizeof(T)) { + if (vec_size > p.Remaining() / SerializeMinElemSize()) { p.SetError(PointerWrap::ERROR_FAILURE); return; } diff --git a/Common/Serialize/SerializeList.h b/Common/Serialize/SerializeList.h index d344152fe8..f768ddabb6 100644 --- a/Common/Serialize/SerializeList.h +++ b/Common/Serialize/SerializeList.h @@ -27,7 +27,7 @@ void DoList(PointerWrap &p, std::list &x, T &default_val) { Do(p, list_size); // Guard against an attacker-controlled size driving a huge resize, same as DoVector. if (p.mode == PointerWrap::MODE_READ || p.mode == PointerWrap::MODE_VERIFY) { - if (list_size > p.Remaining() / sizeof(T)) { + if (list_size > p.Remaining() / SerializeMinElemSize()) { p.SetError(PointerWrap::ERROR_FAILURE); return; } diff --git a/Common/Serialize/SerializeMap.h b/Common/Serialize/SerializeMap.h index 9c085b2cc2..05c16a3fe5 100644 --- a/Common/Serialize/SerializeMap.h +++ b/Common/Serialize/SerializeMap.h @@ -29,14 +29,16 @@ void DoMap(PointerWrap &p, M &x, typename M::mapped_type &default_val) { switch (p.mode) { case PointerWrap::MODE_READ: { + // Clear before the guard below can bail out: for a map of pointers, our caller has + // already deleted every value, so leaving them in place would be a use-after-free. + x.clear(); // Guard against an attacker-controlled count driving an enormous number of // loop iterations/allocations, same spirit as DoVector's guard. - constexpr size_t minElemSize = sizeof(typename M::key_type) + sizeof(typename M::mapped_type); + constexpr size_t minElemSize = SerializeMinElemSize() + SerializeMinElemSize(); if (number > p.Remaining() / minElemSize) { p.SetError(PointerWrap::ERROR_FAILURE); return; } - x.clear(); while (number > 0) { typename M::key_type first = typename M::key_type(); Do(p, first); @@ -107,14 +109,16 @@ void DoMultimap(PointerWrap &p, M &x, typename M::mapped_type &default_val) { switch (p.mode) { case PointerWrap::MODE_READ: { + // Clear before the guard below can bail out: for a map of pointers, our caller has + // already deleted every value, so leaving them in place would be a use-after-free. + x.clear(); // Guard against an attacker-controlled count driving an enormous number of // loop iterations/allocations, same spirit as DoVector's guard. - constexpr size_t minElemSize = sizeof(typename M::key_type) + sizeof(typename M::mapped_type); + constexpr size_t minElemSize = SerializeMinElemSize() + SerializeMinElemSize(); if (number > p.Remaining() / minElemSize) { p.SetError(PointerWrap::ERROR_FAILURE); return; } - x.clear(); while (number > 0) { typename M::key_type first = typename M::key_type(); Do(p, first); diff --git a/Common/Serialize/SerializeSet.h b/Common/Serialize/SerializeSet.h index e555bd53ec..018e674b29 100644 --- a/Common/Serialize/SerializeSet.h +++ b/Common/Serialize/SerializeSet.h @@ -29,13 +29,15 @@ void DoSet(PointerWrap &p, std::set &x) { switch (p.mode) { case PointerWrap::MODE_READ: { + // Clear before the guard below can bail out: for a set of pointers, our caller has + // already deleted every element, so leaving them in place would be a use-after-free. + x.clear(); // Guard against an attacker-controlled count driving an enormous number of // loop iterations/allocations, same spirit as DoVector's guard. - if (number > p.Remaining() / sizeof(T)) { + if (number > p.Remaining() / SerializeMinElemSize()) { p.SetError(PointerWrap::ERROR_FAILURE); return; } - x.clear(); while (number-- > 0) { T it = T(); Do(p, it); diff --git a/Common/Serialize/Serializer.cpp b/Common/Serialize/Serializer.cpp index 7a9b95a5e1..86d2334bef 100644 --- a/Common/Serialize/Serializer.cpp +++ b/Common/Serialize/Serializer.cpp @@ -217,7 +217,9 @@ void Do(PointerWrap &p, std::wstring &x) { int stringLen = sizeof(wchar_t) * ((int)x.length() + 1); Do(p, stringLen); - if (stringLen < 0 || stringLen > MAX_SANE_STRING_LENGTH) { + // The length is in bytes, so it has to be a whole number of characters, and at least the NUL + // terminator. Otherwise read() below computes a negative character count. + if (stringLen < (int)sizeof(wchar_t) || (stringLen % sizeof(wchar_t)) != 0 || stringLen > MAX_SANE_STRING_LENGTH) { WARN_LOG(Log::SaveState, "Savestate failure: bad stringLen %d", stringLen); p.SetError(PointerWrap::ERROR_FAILURE); return; @@ -250,7 +252,9 @@ void Do(PointerWrap &p, std::u16string &x) { int stringLen = sizeof(char16_t) * ((int)x.length() + 1); Do(p, stringLen); - if (stringLen < 0 || stringLen > MAX_SANE_STRING_LENGTH) { + // The length is in bytes, so it has to be a whole number of characters, and at least the NUL + // terminator. Otherwise read() below computes a negative character count. + if (stringLen < (int)sizeof(char16_t) || (stringLen % sizeof(char16_t)) != 0 || stringLen > MAX_SANE_STRING_LENGTH) { WARN_LOG(Log::SaveState, "Savestate failure: bad stringLen %d", stringLen); p.SetError(PointerWrap::ERROR_FAILURE); return; diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 3998e77bcc..7dca73788a 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -87,6 +87,13 @@ #include "Common/File/VFS/DirectoryReader.h" #include "Common/Math/fast/fast_matrix.h" #include "Common/Serialize/Serializer.h" +#include "Common/Serialize/SerializeFuncs.h" +#include "Common/Serialize/SerializeMap.h" +#include "Common/Serialize/SerializeSet.h" +#include "Common/Serialize/SerializeList.h" +#include +#include +#include #include "Core/CmdLine.h" #include "Common/Data/Collections/Hashmaps.h" #include "Core/Util/BlockAllocator.h" @@ -511,6 +518,403 @@ bool TestUtf8() { return true; } +// PointerWrap is the savestate serializer. The same DoState() code runs in MEASURE, WRITE and READ +// mode, so mistakes here don't show up as compile errors - they show up as savestates that don't +// load, or worse. Everything read back came off disk and is therefore attacker-controllable, so the +// corrupt-input cases below matter as much as the round trips. + +struct SerializerPOD { + u32 a; + s16 b; + u8 c; + float d; +}; + +// Held by pointer in a map below, the way a lot of HLE state is (sceMpeg's contexts, sceFont's +// fonts, sceKernelThread's pending calls, ...). +struct SerializerTestObj { + u32 value = 0; + void DoState(PointerWrap &p) { + Do(p, value); + } +}; + +// Shaped like real DoState() code: a versioned section, a few fields, and one field that only +// exists from version 2 on. Set version to 1 before serializing to produce an old-format buffer. +struct SerializerTestState { + int version = 2; + u32 a = 0; + std::string name; + std::vector values; + int addedInV2 = 0; + + void DoState(PointerWrap &p) { + PointerWrapSection s = p.Section("TestState", 1, version); + if (!s) + return; + Do(p, a); + Do(p, name); + Do(p, values); + if (s >= 2) + Do(p, addedInV2); + } +}; + +// Measures, then rewinds into a buffer of exactly the measured size - the same sequence +// CChunkFileReader::MeasureAndSavePtr() uses, so the measure-vs-write checkpoint machinery gets +// exercised as well. Returns false if either pass reported an error or the two disagreed. +template +static bool SerializerWrite(std::vector *out, Func f) { + u8 *ptr = nullptr; + PointerWrap p(&ptr, PointerWrap::MODE_MEASURE); + f(p); + if (p.Failed()) + return false; + // Fill with junk so a field the write pass forgets shows up as garbage rather than zero. + out->assign(p.Offset(), 0xCD); + p.RewindForWrite(out->empty() ? nullptr : &(*out)[0]); + f(p); + return p.CheckAfterWrite() && !p.Failed(); +} + +// Reads out of a copy of the buffer with the read end set, which is what LoadPtr() does and what +// all the bounds checks depend on. +template +static PointerWrap::Error SerializerRead(const std::vector &buf, Func f) { + std::vector copy = buf; + u8 *ptr = copy.empty() ? nullptr : ©[0]; + PointerWrap p(&ptr, PointerWrap::MODE_READ); + if (!copy.empty()) + p.SetReadEnd(©[0] + copy.size()); + f(p); + return p.error; +} + +// A buffer whose first four bytes are a length/count field, for feeding hand-corrupted values in. +static std::vector SerializerBufferWithCount(int count, size_t totalSize) { + std::vector buf(totalSize < sizeof(int) ? sizeof(int) : totalSize, 0); + memcpy(&buf[0], &count, sizeof(int)); + return buf; +} + +bool TestSerializer() { + // Plain values and PODs survive a measure/write/read round trip, and the measure pass agrees + // with the write pass about the size. + { + SerializerPOD pod{ 0x12345678, -1234, 0xAB, 1.5f }; + u32 plain = 0xDEADBEEF; + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { + Do(p, plain); + Do(p, pod); + })); + EXPECT_EQ_INT((int)buf.size(), (int)(sizeof(u32) + sizeof(SerializerPOD))); + + u32 outPlain = 0; + SerializerPOD outPod{}; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { + Do(p, outPlain); + Do(p, outPod); + }), (int)PointerWrap::ERROR_NONE); + EXPECT_EQ_HEX(outPlain, plain); + EXPECT_EQ_HEX(outPod.a, pod.a); + EXPECT_EQ_INT(outPod.b, pod.b); + EXPECT_EQ_INT(outPod.c, pod.c); + EXPECT_EQ_FLOAT(outPod.d, pod.d); + } + + // Strings, including the empty one and one with an embedded NUL - the length is serialized + // separately, so the NUL shouldn't truncate anything. + { + std::string empty; + std::string normal = "hello savestate"; + std::string embedded("a\0b", 3); + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { + Do(p, empty); + Do(p, normal); + Do(p, embedded); + })); + + std::string outEmpty = "junk", outNormal, outEmbedded; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { + Do(p, outEmpty); + Do(p, outNormal); + Do(p, outEmbedded); + }), (int)PointerWrap::ERROR_NONE); + EXPECT_TRUE(outEmpty.empty()); + EXPECT_EQ_STR(outNormal, normal); + EXPECT_EQ_INT((int)outEmbedded.size(), 3); + EXPECT_TRUE(outEmbedded == embedded); + } + + // The containers that DoState() code actually uses. + { + std::vector vec{ 1, 2, 3, 0xFFFFFFFF }; + std::vector strs{ "one", "", "three" }; + std::map map{ { 5, 50 }, { 1, 10 }, { 9, 90 } }; + std::set set{ 7, 3, 11 }; + std::list list{ 4, 5, 6 }; + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { + Do(p, vec); + Do(p, strs); + Do(p, map); + Do(p, set); + Do(p, list); + })); + + // Deliberately non-empty to start with, so a load that forgets to clear shows up. + std::vector outVec{ 99, 99 }; + std::vector outStrs{ "junk" }; + std::map outMap{ { 123, 456 } }; + std::set outSet{ 123 }; + std::list outList{ 99 }; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { + Do(p, outVec); + Do(p, outStrs); + Do(p, outMap); + Do(p, outSet); + Do(p, outList); + }), (int)PointerWrap::ERROR_NONE); + EXPECT_TRUE(outVec == vec); + EXPECT_TRUE(outStrs == strs); + EXPECT_TRUE(outMap == map); + EXPECT_TRUE(outSet == set); + EXPECT_TRUE(outList == list); + } + + // Sections: a matching title and an acceptable version give a usable section, and the marker + // the section destructor writes lines up on read. + { + SerializerTestState state; + state.a = 0x1234; + state.name = "statename"; + state.values = { 10, 20 }; + state.addedInV2 = 77; + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { state.DoState(p); })); + + SerializerTestState out; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { out.DoState(p); }), (int)PointerWrap::ERROR_NONE); + EXPECT_EQ_HEX(out.a, state.a); + EXPECT_EQ_STR(out.name, state.name); + EXPECT_TRUE(out.values == state.values); + EXPECT_EQ_INT(out.addedInV2, state.addedInV2); + + // A section written by a newer build than we understand must be refused, not + // misinterpreted - this is what stops a future savestate from being read as garbage. + bool sectionUsable = true; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { + PointerWrapSection s = p.Section("TestState", 1, 1); + sectionUsable = (bool)s; + }), (int)PointerWrap::ERROR_FAILURE); + EXPECT_FALSE(sectionUsable); + + // So must a different section title where we expected this one. + sectionUsable = true; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { + PointerWrapSection s = p.Section("SomethingElse", 1, 2); + sectionUsable = (bool)s; + }), (int)PointerWrap::ERROR_FAILURE); + EXPECT_FALSE(sectionUsable); + } + + // The backwards compatibility mechanism itself: a version 1 buffer read by version 2 code + // yields a version 1 section, and the field that didn't exist yet keeps its default. + { + SerializerTestState old; + old.version = 1; + old.a = 0xAAAA; + old.name = "old"; + old.values = { 1 }; + old.addedInV2 = 12345; // Not written at version 1. + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { old.DoState(p); })); + + SerializerTestState out; // version 2, addedInV2 defaults to 0 + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { out.DoState(p); }), (int)PointerWrap::ERROR_NONE); + EXPECT_EQ_HEX(out.a, old.a); + EXPECT_EQ_STR(out.name, old.name); + EXPECT_EQ_INT(out.addedInV2, 0); + } + + // A truncated savestate has to fail cleanly at every possible cut point rather than read past + // the end of the buffer. This is the case a corrupt file on disk actually produces. + { + SerializerTestState state; + state.a = 0x5555; + state.name = "truncate me"; + state.values = { 1, 2, 3, 4, 5 }; + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { state.DoState(p); })); + + for (size_t cut = 1; cut < buf.size(); ++cut) { + std::vector truncated(buf.begin(), buf.begin() + cut); + SerializerTestState out; + const PointerWrap::Error err = SerializerRead(truncated, [&](PointerWrap &p) { out.DoState(p); }); + if (err != PointerWrap::ERROR_FAILURE) { + printf("Truncating to %d of %d bytes was accepted\n", (int)cut, (int)buf.size()); + return false; + } + } + } + + // Hand-corrupted counts and lengths. In each case there is nowhere near enough buffer left for + // what the header claims, so the load must be refused before anything is allocated or copied. + { + // A vector claiming four billion elements. + { + std::vector buf = SerializerBufferWithCount((int)0xFFFFFFFF, 64); + std::vector out; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { Do(p, out); }), (int)PointerWrap::ERROR_FAILURE); + EXPECT_TRUE(out.empty()); + } + // A map, a set and a list claiming the same. + { + std::vector buf = SerializerBufferWithCount((int)0xFFFFFFFF, 64); + std::map outMap; + std::set outSet; + std::list outList; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { Do(p, outMap); }), (int)PointerWrap::ERROR_FAILURE); + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { Do(p, outSet); }), (int)PointerWrap::ERROR_FAILURE); + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { Do(p, outList); }), (int)PointerWrap::ERROR_FAILURE); + EXPECT_TRUE(outMap.empty()); + EXPECT_TRUE(outSet.empty()); + EXPECT_TRUE(outList.empty()); + } + // Strings: negative, absurd, zero (there is always at least a NUL byte), and merely longer + // than what's left in the buffer. + { + const int lengths[] = { -1, 0x7FFFFFFF, 0, 1000 }; + for (size_t i = 0; i < ARRAY_SIZE(lengths); ++i) { + std::vector buf = SerializerBufferWithCount(lengths[i], 64); + std::string out = "untouched"; + const PointerWrap::Error err = SerializerRead(buf, [&](PointerWrap &p) { Do(p, out); }); + if (err != PointerWrap::ERROR_FAILURE) { + printf("String length %d was accepted\n", lengths[i]); + return false; + } + } + } + // u16strings are measured in bytes, so on top of the above they can also claim a length + // that isn't a whole number of characters. + { + const int lengths[] = { -1, 0x7FFFFFFF, 0, 1, 3, 1000 }; + for (size_t i = 0; i < ARRAY_SIZE(lengths); ++i) { + std::vector buf = SerializerBufferWithCount(lengths[i], 64); + std::u16string out = u"untouched"; + const PointerWrap::Error err = SerializerRead(buf, [&](PointerWrap &p) { Do(p, out); }); + if (err != PointerWrap::ERROR_FAILURE) { + printf("u16string byte length %d was accepted\n", lengths[i]); + return false; + } + } + } + } + + // Maps of pointers, which is how most HLE contexts are savestated. Loading deletes whatever + // was in the map before reading the new contents, so bailing out on a corrupt count must not + // leave the freed pointers behind - the next access to them, or the destructor, would be a + // use-after-free. + { + std::map ptrMap; + ptrMap[1] = new SerializerTestObj(); + ptrMap[1]->value = 0x1111; + ptrMap[7] = new SerializerTestObj(); + ptrMap[7]->value = 0x7777; + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { Do(p, ptrMap); })); + + std::map outMap; + outMap[99] = new SerializerTestObj(); + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { Do(p, outMap); }), (int)PointerWrap::ERROR_NONE); + EXPECT_EQ_INT((int)outMap.size(), 2); + EXPECT_EQ_HEX(outMap[1]->value, (u32)0x1111); + EXPECT_EQ_HEX(outMap[7]->value, (u32)0x7777); + + std::vector badBuf = SerializerBufferWithCount((int)0xFFFFFFFF, 64); + EXPECT_EQ_INT((int)SerializerRead(badBuf, [&](PointerWrap &p) { Do(p, outMap); }), (int)PointerWrap::ERROR_FAILURE); + const bool leftDangling = !outMap.empty(); + outMap.clear(); // Must not delete these - the loader already did. + EXPECT_FALSE(leftDangling); + + for (const std::pair &entry : ptrMap) + delete entry.second; + } + + // Valid u16strings still round trip, including the empty one. + { + std::u16string empty; + std::u16string text = u"unicode"; + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { + Do(p, empty); + Do(p, text); + })); + std::u16string outEmpty = u"junk", outText; + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { + Do(p, outEmpty); + Do(p, outText); + }), (int)PointerWrap::ERROR_NONE); + EXPECT_TRUE(outEmpty.empty()); + EXPECT_TRUE(outText == text); + } + + // Once a failure is latched the serializer drops to MODE_NOOP and stops touching the caller's + // data, so the rest of a broken savestate can be walked without doing damage. A warning, on the + // other hand, must not stop anything. + { + std::vector buf = SerializerBufferWithCount(-1, 64); + u32 shouldBeUntouched = 0x11111111; + std::string alsoUntouched = "keepme"; + bool wentNoop = false; + const PointerWrap::Error err = SerializerRead(buf, [&](PointerWrap &p) { + std::string bad; + Do(p, bad); // fails: negative length + wentNoop = p.mode == PointerWrap::MODE_NOOP; + Do(p, shouldBeUntouched); + Do(p, alsoUntouched); + }); + EXPECT_EQ_INT((int)err, (int)PointerWrap::ERROR_FAILURE); + EXPECT_TRUE(wentNoop); + EXPECT_EQ_HEX(shouldBeUntouched, (u32)0x11111111); + EXPECT_EQ_STR(alsoUntouched, std::string("keepme")); + + u8 *ptr = &buf[0]; + PointerWrap p(&ptr, PointerWrap::MODE_READ); + p.SetError(PointerWrap::ERROR_WARNING); + EXPECT_FALSE(p.Failed()); + EXPECT_EQ_INT((int)p.mode, (int)PointerWrap::MODE_READ); + } + + // A marker that doesn't match means the writer and reader disagree about the layout, which has + // to be a hard failure - carrying on would read every following field from the wrong offset. + { + std::vector buf; + EXPECT_TRUE(SerializerWrite(&buf, [&](PointerWrap &p) { p.DoMarker("Thing", 0x1234); })); + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { p.DoMarker("Thing", 0x1234); }), (int)PointerWrap::ERROR_NONE); + EXPECT_EQ_INT((int)SerializerRead(buf, [&](PointerWrap &p) { p.DoMarker("Thing", 0x4321); }), (int)PointerWrap::ERROR_FAILURE); + } + + // The measure pass and the write pass have to visit the same sections at the same offsets; + // CheckAfterWrite() exists to catch DoState() code whose behaviour depends on something that + // changed in between. Fake exactly that and make sure it's noticed rather than silently + // producing a savestate that can't be loaded. + { + int pass = 0; + std::vector buf; + EXPECT_FALSE(SerializerWrite(&buf, [&](PointerWrap &p) { + u32 v = 0; + PointerWrapSection s = p.Section(pass++ == 0 ? "SectionA" : "SectionB", 1); + if (s) + Do(p, v); + })); + } + + return true; +} + bool TestMemBlockInfoSaveState() { MemBlockInfoInit(); MemBlockOverrideDetailed(); @@ -2464,6 +2868,7 @@ TestItem availableTests[] = { TEST_ITEM(Parsers), TEST_ITEM(TruncateCpy), TEST_ITEM(MemBlockInfoSaveState), + TEST_ITEM(Serializer), TEST_ITEM(BlockAllocator), TEST_ITEM(SymbolMap), TEST_ITEM(Hashmaps),