From 42f9af1744b39fd863d534cd6d7910d0ed96e9c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 25 Oct 2026 19:22:25 +0100 Subject: [PATCH] MemBlockInfo: Clamp marked ranges to the address space A size that makes addr + size wrap around 32 bits had Split() build a slab that ends before it starts. Lookups then skipped it, and a later Merge() ran FillHeads() over it with a huge count, writing far outside heads_. Sizes come from games through HLE calls, and the large ones are recorded even without detailed mem info. Seen in a Play crash report, on the FlushMemInfo thread. FillHeads() also took the address one past the end of heads_ for a slab in the last slice, which Debug builds assert on. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/Debugger/MemBlockInfo.cpp | 15 ++++++++++----- unittest/UnitTest.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/Core/Debugger/MemBlockInfo.cpp b/Core/Debugger/MemBlockInfo.cpp index c0cdcf9825..cd87aec1e6 100644 --- a/Core/Debugger/MemBlockInfo.cpp +++ b/Core/Debugger/MemBlockInfo.cpp @@ -127,6 +127,14 @@ MemSlabMap::~MemSlabMap() { } bool MemSlabMap::Mark(uint32_t addr, uint32_t size, uint64_t ticks, uint32_t pc, bool allocated, const char *tag, size_t tagLen) { + if (addr >= MAX_SIZE) { + return false; + } + // The size comes from the game. If addr + size wraps, Split() makes a slab that ends before it + // starts, and a later FillHeads() on it writes far outside heads_. + if (size > MAX_SIZE - addr) { + size = MAX_SIZE - addr; + } uint32_t end = addr + size; Slab *slab = FindSlab(addr); Slab *firstMatch = nullptr; @@ -430,11 +438,8 @@ void MemSlabMap::FillHeads(Slab *slab) { } // Now replace all the rest - we definitely cover the start of them. - Slab **next = &heads_[slice + 1]; - // We want to set slice + 1 through endSlice, inclusive. - size_t c = endSlice - slice; - for (size_t i = 0; i < c; ++i) { - next[i] = slab; + for (uint32_t i = slice + 1; i <= endSlice; ++i) { + heads_[i] = slab; } } diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index c560b3e9fd..f4763d6e73 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -924,6 +924,33 @@ bool TestMemBlockInfoSaveState() { return true; } +bool TestMemBlockInfoRangeWrap() { + MemBlockInfoInit(); + MemBlockOverrideDetailed(); + + // Sizes come from the game. One that wraps around 32 bits used to leave a slab that ends + // before it starts, hidden from lookups, and later marks crashed on it. + NotifyMemInfo(MemBlockFlags::WRITE, 0x08800800, 0xFFFFF000, "Wrap", 4); + NotifyMemInfo(MemBlockFlags::WRITE, 0x3FFFF000, 0x2000, "PastEnd", 7); + + auto findWrite = [](uint32_t addr, const char *tag) { + std::vector results = FindMemInfoByFlag(MemBlockFlags::WRITE, addr, 1); + return results.size() == 1 && results[0].tag == tag && results[0].start + results[0].size > results[0].start; + }; + EXPECT_TRUE(findWrite(0x08800800, "Wrap")); + EXPECT_TRUE(findWrite(0x20000000, "Wrap")); + EXPECT_TRUE(findWrite(0x3FFFFFFF, "PastEnd")); + + // Splitting the clamped slab again works too. + NotifyMemInfo(MemBlockFlags::WRITE, 0x08900000, 0x1000, "Inside", 6); + EXPECT_TRUE(findWrite(0x08900000, "Inside")); + EXPECT_TRUE(findWrite(0x08901000, "Wrap")); + + MemBlockReleaseDetailed(); + MemBlockInfoShutdown(); + return true; +} + // Covers BreakpointManager::ChangeBreakPointAddress(), which the ImDebugger uses to relocate a // breakpoint the user is editing. Only the pure bookkeeping is exercised here - there's no JIT in // this build, so the cache invalidation it also does is a no-op. @@ -4055,6 +4082,7 @@ TestItem availableTests[] = { TEST_ITEM(Parsers), TEST_ITEM(TruncateCpy), TEST_ITEM(MemBlockInfoSaveState), + TEST_ITEM(MemBlockInfoRangeWrap), TEST_ITEM(Serializer), TEST_ITEM(BlockAllocator), TEST_ITEM(SymbolMap),