fix freezing n64 pointed-at values using truncated pointers

src/ui/viewmodels/MemoryWatchViewModel.cpp

@@ -404,18 +404,46 @@ bool MemoryWatchViewModel::UpdateCurrentAddressFromIndirectAddress()
     {
         if (pCondition->type == RC_CONDITION_ADD_ADDRESS)
         {
-            if (m_bIndirectAddressValid) // don't need to check validity if we've already found a problem
+            // If the chain is already invalid, we don't need to validate it any further. Just follow it to
+            // the final address so we can capture the current value.
+            if (!m_bIndirectAddressValid)
+                continue;
+
+            rc_typed_value_t address;
+            rc_evaluate_operand(&address, &pCondition->operand1, nullptr);
+            rc_typed_value_convert(&address, RC_VALUE_TYPE_UNSIGNED);
+
+            if (address.value.u32 == 0) // pointer is null
             {
-                rc_typed_value_t address;
-                rc_evaluate_operand(&address, &pCondition->operand1, nullptr);
-                rc_typed_value_convert(&address, RC_VALUE_TYPE_UNSIGNED);
+                m_bIndirectAddressValid = false;
+                continue;
+            }
 
-                if (address.value.u32 == 0) // pointer is null
+            const auto nAdjustedAddress = pConsoleContext.ByteAddressFromRealAddress(address.value.u32);
+            if (nAdjustedAddress == 0xFFFFFFFF) // pointer is invalid
+            {
+                // if the operand is using a smaller size to mask the address, try to convert it back to
+                // a real address.
+                if (m_nPointerSize == ra::data::Memory::Size::Unknown)
+                {
+                    uint32_t nMask, nOffset;
+                    if (!pConsoleContext.GetRealAddressConversion(&m_nPointerSize, &nMask, &nOffset))
+                        m_nPointerSize = ra::data::Memory::Size::ThirtyTwoBit;
+                }
+
+                if (m_nPointerSize != ra::data::Memory::SizeFromRcheevosSize(pCondition->operand1.size))
+                {
+                    // size is not the expected masking size
                     m_bIndirectAddressValid = false;
+                    continue;
+                }
 
-                const auto nAdjustedAddress = pConsoleContext.ByteAddressFromRealAddress(address.value.u32);
-                if (nAdjustedAddress == 0xFFFFFFFF) // pointer is invalid
+                const auto nRealAddress = pConsoleContext.RealAddressFromByteAddress(address.value.u32);
+                if (nRealAddress == 0xFFFFFFFF) // pointer is invalid
+                {
                     m_bIndirectAddressValid = false;
+                    continue;
+                }
             }
         }
         else if (pCondition->type == RC_CONDITION_MEASURED)

src/ui/viewmodels/MemoryWatchViewModel.hh

@@ -297,6 +297,7 @@ private:
     ra::data::ByteAddress m_nAddress = 0;
     uint32_t m_nValue = 0;
     ra::data::Memory::Size m_nSize = ra::data::Memory::Size::EightBit;
+    ra::data::Memory::Size m_nPointerSize = ra::data::Memory::Size::Unknown;
     bool m_bModified = false;
     bool m_bInitialized = false;
     bool m_bSyncingDescriptionHeader = false;

tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp

@@ -2003,6 +2003,73 @@ TEST_CLASS(MemoryBookmarksViewModel_Tests)
         Assert::AreEqual({7}, memory.at(14));             // but not memory
     }
 
+
+    TEST_METHOD(TestDoFrameFrozenBookmarkInvalidTruncatedIndirectAddress)
+    {
+        MemoryBookmarksViewModelHarness bookmarks;
+        std::array<unsigned char, 32> memory{};
+        bookmarks.mockEmulatorContext.MockMemory(memory);
+        bookmarks.mockConsoleContext.SetId(N64); // N64 pointers are 24-bit addresses at $80000000
+        bookmarks.mockConsoleContext.ResetMemoryRegions();
+        bookmarks.mockConsoleContext.AddMemoryRegion(0x00000000, 0x001FFFFF, ra::data::MemoryRegion::Type::SystemRAM, 0x80000000);
+
+        bookmarks.AddBookmark("I:0xW0004_M:0xH0008"); // 24-bit read to mask pointer
+        auto* pBookmark = bookmarks.GetBookmark(0);
+        Expects(pBookmark != nullptr);
+
+        memory.at(0) = 6;
+        memory.at(1) = 1;
+        memory.at(4) = 4;
+        memory.at(7) = 0x80;
+        memory.at(12) = 7;
+        pBookmark->DoFrame();
+
+        Assert::AreEqual(12U, pBookmark->GetAddress());
+        Assert::IsTrue(pBookmark->IsIndirectAddress());
+        Assert::AreEqual(std::wstring(L"07"), pBookmark->GetCurrentValue());
+
+        pBookmark->SetBehavior(ra::ui::viewmodels::MemoryBookmarksViewModel::BookmarkBehavior::Frozen);
+
+        memory.at(0) = 3;
+        memory.at(12) = 4;
+
+        pBookmark->DoFrame();
+
+        // frozen values should be written back to memory
+        Assert::AreEqual(12U, pBookmark->GetAddress());
+        Assert::AreEqual(std::wstring(L"07"), pBookmark->GetCurrentValue());
+        Assert::AreEqual({ 7 }, memory.at(12));
+        Assert::IsTrue(pBookmark->IsIndirectAddressChainValid());
+
+        // console says only 0x1FFFFF bytes are valid. if pointer points beyond that, it shouldn't write
+        memory.at(6) = 0x20; // 80200004
+
+        pBookmark->DoFrame();
+        Assert::AreEqual({ 0x20000C }, pBookmark->GetAddress());  // address updated
+        Assert::IsFalse(pBookmark->IsIndirectAddressChainValid());
+
+        // null is implicitly invalid
+        memory.at(4) = 0;
+        memory.at(6) = 0;
+        memory.at(7) = 0;
+        Assert::AreEqual({ 0 }, memory.at(8));
+
+        pBookmark->DoFrame();
+        Assert::AreEqual({ 8 }, pBookmark->GetAddress());  // address updated
+        Assert::AreEqual({ 0 }, memory.at(8));             // but not memory
+        Assert::IsFalse(pBookmark->IsIndirectAddressChainValid());
+
+        // pointing at valid data again
+        memory.at(4) = 6;
+        memory.at(7) = 0x80;
+        Assert::AreEqual({ 0 }, memory.at(14));
+
+        pBookmark->DoFrame();
+        Assert::AreEqual({ 14 }, pBookmark->GetAddress());  // address updated
+        Assert::AreEqual({ 7 }, memory.at(14));             // but not memory
+        Assert::IsTrue(pBookmark->IsIndirectAddressChainValid());
+    }
+
 private:
     void FrozenTest(const std::string& sDefinition, uint8_t* pMemory, const std::wstring& sDisplay, uint8_t* pModifiedMemory)
     {

tests/ui/viewmodels/MemoryWatchListViewModel_Tests.cpp

@@ -10,12 +10,9 @@
 #include "tests\devkit\context\mocks\MockRcClient.hh"
 #include "tests\devkit\context\mocks\MockUserContext.hh"
 #include "tests\devkit\services\mocks\MockConfiguration.hh"
-#include "tests\devkit\services\mocks\MockFileSystem.hh"
-#include "tests\devkit\services\mocks\MockLocalStorage.hh"
 #include "tests\devkit\testutil\MemoryAsserts.hh"
 #include "tests\mocks\MockAchievementRuntime.hh"
 #include "tests\mocks\MockDesktop.hh"
-#include "tests\mocks\MockFrameEventQueue.hh"
 #include "tests\mocks\MockGameContext.hh"
 #include "tests\mocks\MockOverlayManager.hh"