diff --git a/src/ui/viewmodels/MemoryWatchViewModel.cpp b/src/ui/viewmodels/MemoryWatchViewModel.cpp index 34d8c38c..53b07dd9 100644 --- a/src/ui/viewmodels/MemoryWatchViewModel.cpp +++ b/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) diff --git a/src/ui/viewmodels/MemoryWatchViewModel.hh b/src/ui/viewmodels/MemoryWatchViewModel.hh index 671dfbe1..0df9430c 100644 --- a/src/ui/viewmodels/MemoryWatchViewModel.hh +++ b/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; diff --git a/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp b/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp index 95f95b41..2a835cde 100644 --- a/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp +++ b/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 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) { diff --git a/tests/ui/viewmodels/MemoryWatchListViewModel_Tests.cpp b/tests/ui/viewmodels/MemoryWatchListViewModel_Tests.cpp index 76190e08..7d536b04 100644 --- a/tests/ui/viewmodels/MemoryWatchListViewModel_Tests.cpp +++ b/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"