don't capture memory when read fails

src/devkit/context/impl/EmulatorMemoryContext.cpp

@@ -176,22 +176,23 @@ uint8_t EmulatorMemoryContext::ReadMemoryByte(ra::data::ByteAddress nAddress) co
 }
 
 uint32_t EmulatorMemoryContext::ReadMemory(ra::data::ByteAddress nAddress, uint8_t pBuffer[], size_t nCount,
-                                   const EmulatorMemoryContext::MemoryBlock& pBlock)
+                                   const EmulatorMemoryContext::MemoryBlock& pBlock, bool bFill)
 {
     Expects(pBuffer != nullptr);
 
     if (pBlock.readBlock)
     {
         const size_t nRead = pBlock.readBlock(nAddress, pBuffer, gsl::narrow_cast<uint32_t>(nCount));
-        if (nRead < nCount)
+        if (nRead < nCount && bFill)
             memset(pBuffer + nRead, 0, nCount - nRead);
 
         return gsl::narrow_cast<uint32_t>(nRead);
     }
 
     if (!pBlock.read)
     {
-        memset(pBuffer, 0, nCount);
+        if (bFill)
+            memset(pBuffer, 0, nCount);
         return 0;
     }
 
@@ -588,8 +589,11 @@ void EmulatorMemoryContext::CaptureMemory(std::vector<ra::data::CapturedMemoryBl
             }
 
             Expects(pBlock != nullptr);
-            ReadMemory(nAdjustedAddress, pBlock->GetBytes(), nBlockSize, pMemoryBlock);
-            pBlock->OptimizeMemory(vBlocks);
+            const auto nRead = ReadMemory(nAdjustedAddress, pBlock->GetBytes(), nBlockSize, pMemoryBlock, false);
+            if (nRead == 0)
+                vBlocks.pop_back();
+            else
+                pBlock->OptimizeMemory(vBlocks);
 
             nAddress += nBlockSize;
             nAdjustedAddress += nBlockSize;

src/devkit/context/impl/EmulatorMemoryContext.hh

@@ -123,7 +123,7 @@ protected:
         MemoryWriteFunction* write;
         MemoryReadBlockFunction* readBlock;
     };
-    static uint32_t ReadMemory(ra::data::ByteAddress nAddress, uint8_t pBuffer[], size_t nCount, const MemoryBlock& pBlock);
+    static uint32_t ReadMemory(ra::data::ByteAddress nAddress, uint8_t pBuffer[], size_t nCount, const MemoryBlock& pBlock, bool bFill = true);
 
     std::vector<MemoryBlock> m_vMemoryBlocks;
     size_t m_nTotalMemorySize = 0U;

src/ui/viewmodels/MemoryViewerViewModel.cpp

@@ -471,11 +471,19 @@ void MemoryViewerViewModel::ReadMemory(ra::data::ByteAddress nFirstAddress, int
         }
 
         const auto& pMemoryContext = ra::services::ServiceLocator::Get<ra::context::IEmulatorMemoryContext>();
-        pMemoryContext.ReadMemory(nFirstAddress, m_pMemory, gsl::narrow_cast<size_t>(nNumVisibleLines) * 16);
+        const auto nToRead = gsl::narrow_cast<size_t>(nNumVisibleLines) * 16;
+        const auto nRead = pMemoryContext.ReadMemory(nFirstAddress, m_pMemory, nToRead);
 
-        UpdateInvalidRegions();
         UpdateColors();
 
+        // if a portion of memory couldn't be captured, read lines (16-bytes) and mark each failure.
+        if (nRead < nToRead) {
+            for (size_t nOffset = 0; nOffset < nToRead; nOffset += 16) {
+                if (pMemoryContext.ReadMemory(gsl::narrow_cast<ra::data::ByteAddress>(nFirstAddress + nOffset), &m_pMemory[nOffset], 16) == 0)
+                    memset(&m_pInvalid[nOffset], 1, 16);
+            }
+        }
+
         Redraw();
     });
 }
@@ -1151,7 +1159,17 @@ void MemoryViewerViewModel::DoFrame()
     Expects(nVisibleLines < MaxLines);
 
     const auto& pMemoryContext = ra::services::ServiceLocator::Get<ra::context::IEmulatorMemoryContext>();
-    pMemoryContext.ReadMemory(nAddress, pMemory, gsl::narrow_cast<size_t>(nVisibleLines) * 16);
+    const auto nToRead = gsl::narrow_cast<size_t>(nVisibleLines) * 16;
+    if (m_pInvalid[0] || pMemoryContext.ReadMemory(nAddress, pMemory, nToRead) != nToRead)
+    {
+        memset(&pMemory, 0, sizeof(pMemory));
+
+        for (size_t nOffset = 0; nOffset < nToRead; nOffset += 16)
+        {
+            if (!m_pInvalid[nOffset])
+                pMemoryContext.ReadMemory(gsl::narrow_cast<ra::data::ByteAddress>(nAddress + nOffset), &pMemory[nOffset], 16);
+        }
+    }
 
     constexpr int nStride = 8;
     for (int nIndex = 0; nIndex < nVisibleLines * 16; nIndex += nStride)

tests/devkit/context/impl/EmulatorMemoryContext_Tests.cpp

@@ -60,6 +60,11 @@ TEST_CLASS(EmulatorMemoryContext_Tests)
         return nBytes;
     }
 
+    static uint32_t ReadMemoryBlockNull(uint32_t, uint8_t*, uint32_t) noexcept
+    {
+        return 0;
+    }
+
     static void WriteMemory0(uint32_t nAddress, uint8_t nValue) noexcept { memory.at(nAddress) = nValue; }
     static void WriteMemory1(uint32_t nAddress, uint8_t nValue) noexcept { memory.at(gsl::narrow_cast<size_t>(nAddress) + 10) = nValue; }
     static void WriteMemory2(uint32_t nAddress, uint8_t nValue) noexcept { memory.at(gsl::narrow_cast<size_t>(nAddress) + 20) = nValue; }
@@ -761,6 +766,33 @@ TEST_CLASS(EmulatorMemoryContext_Tests)
         for (size_t i = 0; i < 10; i++)
             Assert::AreEqual(memory.at(i + 20), pBytes[i]);
     }
+
+    TEST_METHOD(TestCaptureMemoryGapReadFailure)
+    {
+        EmulatorMemoryContextHarness emulator;
+
+        InitializeMemory();
+        emulator.AddMemoryBlock(0, 10, &ReadMemory0, &WriteMemory0);
+        emulator.AddMemoryBlock(1, 10, nullptr, nullptr);
+        emulator.AddMemoryBlockReader(1, &ReadMemoryBlockNull);
+        emulator.AddMemoryBlock(2, 10, &ReadMemory2, &WriteMemory2);
+
+        std::vector<ra::data::CapturedMemoryBlock> vBlocks;
+        emulator.CaptureMemory(vBlocks, 0, 30, 0);
+        Assert::AreEqual({ 2 }, vBlocks.size());
+
+        Assert::AreEqual(10U, vBlocks.at(0).GetBytesSize());
+        const auto* pBytes = vBlocks.at(0).GetBytes();
+        Expects(pBytes != nullptr);
+        for (size_t i = 0; i < 10; i++)
+            Assert::AreEqual(memory.at(i), pBytes[i]);
+
+        Assert::AreEqual(10U, vBlocks.at(1).GetBytesSize());
+        pBytes = vBlocks.at(1).GetBytes();
+        Expects(pBytes != nullptr);
+        for (size_t i = 0; i < 10; i++)
+            Assert::AreEqual(memory.at(i + 20), pBytes[i]);
+    }
 };
 
 std::array<uint8_t, 64> EmulatorMemoryContext_Tests::memory;

tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp

@@ -67,6 +67,11 @@ TEST_CLASS(MemoryViewerViewModel_Tests)
             return m_pMemory[nAddress - GetFirstAddress()];
         }
 
+        bool GetInvalid(ra::data::ByteAddress nAddress) const
+        {
+            return m_pInvalid[nAddress - GetFirstAddress()];
+        }
+
         unsigned char GetColor(ra::data::ByteAddress nAddress) const
         {
             return gsl::narrow_cast<unsigned char>(ra::itoe<TextColor>(m_pColor[nAddress - GetFirstAddress()]));
@@ -2115,6 +2120,63 @@ TEST_CLASS(MemoryViewerViewModel_Tests)
         viewer.DecreaseCurrentValue(16);
         Assert::AreEqual({ 0xFFEFU }, viewer.mockEmulatorContext.ReadMemory(0U, ra::data::Memory::Size::SixteenBit));
     }
+
+    TEST_METHOD(TestReadMemoryInvalidBlock)
+    {
+        MemoryViewerViewModelHarness viewer;
+        viewer.mockEmulatorContext.AddMemoryBlock(0, 256, [](uint32_t nAddress) -> unsigned char { return nAddress & 0xFF; }, nullptr);
+        viewer.mockEmulatorContext.AddMemoryBlock(1, 256, nullptr, nullptr);
+        viewer.mockEmulatorContext.AddMemoryBlock(2, 256, [](uint32_t nAddress) -> unsigned char { return nAddress & 0xFF; }, nullptr);
+        viewer.SetNumVisibleLines(16);
+
+        // 0x00E0 -> 0x00FF valid, 0x0100-0x01DF invalid
+        viewer.SetFirstAddress(0x00E0);
+        for (uint32_t i = 0x00E0; i < 0x0100; ++i)
+            Assert::IsFalse(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+        for (uint32_t i = 0x0100; i < 0x01E0; ++i)
+            Assert::IsTrue(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+
+        // 0x0100 -> 0x01FF invalid
+        viewer.SetFirstAddress(0x0100);
+        for (uint32_t i = 0x0100; i < 0x0200; ++i)
+            Assert::IsTrue(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+
+        // 0x01C0 -> 0x01FF invalid, 0x0200-0x02BF valid
+        viewer.SetFirstAddress(0x01C0);
+        for (uint32_t i = 0x01C0; i < 0x0200; ++i)
+            Assert::IsTrue(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+        for (uint32_t i = 0x0200; i < 0x02C0; ++i)
+            Assert::IsFalse(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+    }
+
+    TEST_METHOD(TestReadMemoryFail)
+    {
+        MemoryViewerViewModelHarness viewer;
+        viewer.mockEmulatorContext.AddMemoryBlock(0, 256, [](uint32_t nAddress) -> unsigned char { return nAddress & 0xFF; }, nullptr);
+        viewer.mockEmulatorContext.AddMemoryBlock(1, 256, [](uint32_t) -> unsigned char { return 0; }, nullptr);
+        viewer.mockEmulatorContext.AddMemoryBlockReader(1, [](uint32_t, uint8_t*, uint32_t) -> unsigned { return 0; });
+        viewer.mockEmulatorContext.AddMemoryBlock(2, 256, [](uint32_t nAddress) -> unsigned char { return nAddress & 0xFF; }, nullptr);
+        viewer.SetNumVisibleLines(16);
+
+        // 0x00E0 -> 0x00FF valid, 0x0100-0x01DF invalid
+        viewer.SetFirstAddress(0x00E0);
+        for (uint32_t i = 0x00E0; i < 0x0100; ++i)
+            Assert::IsFalse(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+        for (uint32_t i = 0x0100; i < 0x01E0; ++i)
+            Assert::IsTrue(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+
+        // 0x0100 -> 0x01FF invalid
+        viewer.SetFirstAddress(0x0100);
+        for (uint32_t i = 0x0100; i < 0x0200; ++i)
+            Assert::IsTrue(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+
+        // 0x01C0 -> 0x01FF invalid, 0x0200-0x02BF valid
+        viewer.SetFirstAddress(0x01C0);
+        for (uint32_t i = 0x01C0; i < 0x0200; ++i)
+            Assert::IsTrue(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+        for (uint32_t i = 0x0200; i < 0x02C0; ++i)
+            Assert::IsFalse(viewer.GetInvalid(i), viewer.mockEmulatorContext.FormatAddress(i).c_str());
+    }
 };
 
 } // namespace tests