spu: save a stopped ADPCM cursor as its own sentinel

kStopped was stored as an int32 pointer difference, which truncates on a
64-bit host, so a state saved with a finished voice loaded a wild cursor
and crashed in decodeBlock. Store it as -2, and load any offset outside
sound RAM as stopped, which also repairs states written before this.
Fixes #2175.

src/spu/adpcm.cc

@@ -24,7 +24,15 @@ void PCSX::SPU::AdpcmDecoder::saveTo(Protobuf::Int32 &history1, Protobuf::Int32
                                      Protobuf::Int32 *sb, Protobuf::Int32 &sbPos) const {
     history1.value = m_s1;
     history2.value = m_s2;
-    auto storeOffset = [ramBase](uint8_t *ptr, Protobuf::Int32 &offset) { offset.value = ptr ? ptr - ramBase : -1; };
+    auto storeOffset = [ramBase](uint8_t *ptr, Protobuf::Int32 &offset) {
+        if (!ptr) {
+            offset.value = kNullOffset;
+        } else if (ptr == kStopped) {
+            offset.value = kStoppedOffset;
+        } else {
+            offset.value = ptr - ramBase;
+        }
+    };
     storeOffset(m_start, startOffset);
     storeOffset(m_curr, currOffset);
     storeOffset(m_loop, loopOffset);
@@ -38,8 +46,13 @@ void PCSX::SPU::AdpcmDecoder::loadFrom(const Protobuf::Int32 &history1, const Pr
                                        const Protobuf::Int32 &sbPos) {
     m_s1 = history1.value;
     m_s2 = history2.value;
+    // Anything outside sound RAM other than the null marker restores as stopped. That covers
+    // kStoppedOffset, and also states written before it existed, which stored kStopped as a
+    // truncated pointer difference.
     auto restore = [ramBase](const Protobuf::Int32 &offset) -> uint8_t * {
-        return offset.value == -1 ? nullptr : offset.value + ramBase;
+        if (offset.value == kNullOffset) return nullptr;
+        if (offset.value < 0 || offset.value >= kRamSize) return kStopped;
+        return offset.value + ramBase;
     };
     m_start = restore(startOffset);
     m_curr = restore(currOffset);

src/spu/adpcm.h

@@ -65,11 +65,12 @@ class AdpcmDecoder {
     // Cursor sentinel for a voice that decoded a stop/loop-end block with no loop
     // target: the next synthesis pass sees it in the cursor and turns the voice
     // off. It is a deliberately invalid, non-null pointer, distinct from the
-    // nullptr "wiped" state. Its exact all-ones bit pattern is what the savestate
-    // pointer-offset bridge stores and restores by plain arithmetic, so it must
-    // stay all-ones. (Was an inline (uint8_t *)-1 cast smuggled into the cursor.)
+    // nullptr "wiped" state. The savestate bridge stores it as kStoppedOffset.
     inline static uint8_t *const kStopped = reinterpret_cast<uint8_t *>(~uintptr_t(0));
     bool stopped() const { return m_curr == kStopped; }
+    static constexpr int32_t kNullOffset = -1;
+    static constexpr int32_t kStoppedOffset = -2;
+    static constexpr int32_t kRamSize = 512 * 1024;
 
     // Decode cursor accessors (raw pointers into sound RAM).
     uint8_t *start() const { return m_start; }
@@ -84,8 +85,7 @@ class AdpcmDecoder {
     // the per-channel savestate fields. The fields are passed in rather than the
     // whole channel message because adpcm.h cannot include types.h (it is
     // included by it). `ramBase` is spuRamBase; each cursor is stored as a byte
-    // offset, with -1 standing in for a null pointer. The kStopped sentinel is
-    // an all-ones (non-null) pointer, so it round-trips as raw offset arithmetic.
+    // offset, with kNullOffset for a null pointer and kStoppedOffset for kStopped.
     void saveTo(Protobuf::Int32 &history1, Protobuf::Int32 &history2, Protobuf::Int32 &startOffset,
                 Protobuf::Int32 &currOffset, Protobuf::Int32 &loopOffset, uint8_t *ramBase, Protobuf::Int32 *sb,
                 Protobuf::Int32 &sbPos) const;

tests/sstate-adpcm.cc

@@ -0,0 +1,74 @@
+/*
+ * A voice parked at AdpcmDecoder::kStopped must come back stopped from a save
+ * state, whatever address sound RAM happens to live at.
+ */
+
+#include <stdint.h>
+
+#include "gtest/gtest.h"
+#include "spu/adpcm.h"
+
+namespace {
+
+using PCSX::SPU::AdpcmDecoder;
+
+// saveTo and loadFrom only do pointer arithmetic against the base, so it never
+// needs to point at real memory. Pick one above 4 GiB, like a 64-bit heap. Its
+// low 32 bits must not be zero: an old state then stored kStopped as -1, which
+// can't be told apart from a null cursor.
+uint8_t *const kHighBase = reinterpret_cast<uint8_t *>(uintptr_t(0x78351234a000ull));
+
+struct Fields {
+    PCSX::Protobuf::Int32 h1, h2, start, curr, loop, pos;
+    PCSX::Protobuf::Int32 sb[AdpcmDecoder::kSamplesPerBlock];
+};
+
+void save(const AdpcmDecoder &d, Fields &f, uint8_t *base) {
+    d.saveTo(f.h1, f.h2, f.start, f.curr, f.loop, base, f.sb, f.pos);
+}
+
+void load(AdpcmDecoder &d, const Fields &f, uint8_t *base) {
+    d.loadFrom(f.h1, f.h2, f.start, f.curr, f.loop, base, f.sb, f.pos);
+}
+
+}  // namespace
+
+TEST(SaveStateAdpcm, StoppedCursorRoundTrips) {
+    AdpcmDecoder saved;
+    saved.setStart(kHighBase + 0x1000);
+    saved.setCurr(AdpcmDecoder::kStopped);
+    saved.setLoop(nullptr);
+    Fields f;
+    save(saved, f, kHighBase);
+
+    AdpcmDecoder loaded;
+    load(loaded, f, kHighBase);
+    EXPECT_TRUE(loaded.stopped());
+    EXPECT_EQ(loaded.start(), kHighBase + 0x1000);
+    EXPECT_EQ(loaded.loop(), nullptr);
+}
+
+TEST(SaveStateAdpcm, ValidCursorRoundTrips) {
+    AdpcmDecoder saved;
+    saved.setCurr(kHighBase + AdpcmDecoder::kRamSize - 16);
+    Fields f;
+    save(saved, f, kHighBase);
+
+    AdpcmDecoder loaded;
+    load(loaded, f, kHighBase);
+    EXPECT_FALSE(loaded.stopped());
+    EXPECT_EQ(loaded.curr(), kHighBase + AdpcmDecoder::kRamSize - 16);
+}
+
+// States written before kStoppedOffset stored kStopped as a truncated pointer
+// difference. Those must load as stopped, not as a wild pointer.
+TEST(SaveStateAdpcm, LegacyStoppedCursorLoadsStopped) {
+    Fields f;
+    AdpcmDecoder blank;
+    save(blank, f, kHighBase);
+    f.curr.value = int32_t(uint32_t(uintptr_t(AdpcmDecoder::kStopped) - uintptr_t(kHighBase)));
+
+    AdpcmDecoder loaded;
+    load(loaded, f, kHighBase);
+    EXPECT_TRUE(loaded.stopped());
+}