From 6bb45edb3fbaea1eadfe157a5c88eecece29f425 Mon Sep 17 00:00:00 2001 From: Xusheng Date: Tue, 14 Jul 2026 12:27:56 -0400 Subject: [PATCH 1/2] [WIP] Mirror backend memory map into bounded regions so search works while debugging When a debug adapter reports a memory map (DebugAdapter::GetMemoryMap), mirror it into the debugger BinaryView as one bounded remote memory region per entry instead of the single blanket overlay that spans the whole address space. This makes GetBackedAddressRanges() / GetNextBackedAddress() (which drive Find) cover only mapped memory, so search works during a debug session and skips unmapped gaps. - SyncMemoryRegions() runs on every stop but no-ops when the map is unchanged, so single-stepping stays cheap. - Adapters that report no map fall back to the blanket "debugger" region (search stays disabled gracefully instead of hanging on a 2^64 scan). - Per-region accessors are retired (not freed) on change and drained at teardown to avoid use-after-free from in-flight reads. Prototype; not ready for review. Known follow-ups: accessor-backed AddRemoteMemoryRegion is O(N^2), and the fallback should likely move into the adapter (base default returns a full range + an "exact" bit). Co-Authored-By: Claude Opus 4.8 (1M context) --- core/debugadapter.h | 10 +++ core/debuggercontroller.cpp | 130 +++++++++++++++++++++++++++++++--- core/debuggercontroller.h | 24 +++++++ core/debuggerfileaccessor.cpp | 13 +++- core/debuggerfileaccessor.h | 11 +++ 5 files changed, 177 insertions(+), 11 deletions(-) diff --git a/core/debugadapter.h b/core/debugadapter.h index 5909a602..46cb0275 100644 --- a/core/debugadapter.h +++ b/core/debugadapter.h @@ -237,6 +237,16 @@ namespace BinaryNinjaDebugger { m_start(start), m_size(size), m_name(std::move(name)), m_read(read), m_write(write), m_execute(execute), m_shared(shared) {} + + // Value equality, used to detect whether the target's memory map changed between stops so the + // debugger can avoid rebuilding the BinaryView memory regions when nothing has moved. + bool operator==(const DebugMemoryRegion& other) const + { + return m_start == other.m_start && m_size == other.m_size && m_name == other.m_name + && m_read == other.m_read && m_write == other.m_write && m_execute == other.m_execute + && m_shared == other.m_shared; + } + bool operator!=(const DebugMemoryRegion& other) const { return !(*this == other); } }; // A symbol the debugger backend (e.g. LLDB, dbgeng) knows about for a loaded module, but which the diff --git a/core/debuggercontroller.cpp b/core/debuggercontroller.cpp index 173bd3fb..8369e57e 100644 --- a/core/debuggercontroller.cpp +++ b/core/debuggercontroller.cpp @@ -1446,13 +1446,108 @@ bool DebuggerController::CreateDebuggerBinaryView() m_state->GetMemory()->PrefillValueCache(); + // The primary accessor spans the whole address space and owns the stop-event view-refresh + // subscription. It backs the blanket "debugger" region in the fallback case, and stays alive for its + // refresh role even when we mirror a real memory map into bounded regions. m_accessor = new DebuggerFileAccessor(data); + + // Start with the blanket overlay so reads/navigation work immediately, before the first stop. The + // first stop's SyncMemoryRegions() refines this into bounded regions when the backend reports a + // memory map (which enables search); otherwise the blanket remains and search stays disabled. + m_appliedMemoryRegions.clear(); + m_debuggerRegionNames.clear(); + AddDebuggerMemoryRegions(); + + return true; +} + + +void DebuggerController::AddDebuggerMemoryRegions() +{ + BinaryViewRef data = GetData(); data->SetFunctionAnalysisUpdateDisabled(true); - data->GetMemoryMap()->AddRemoteMemoryRegion("debugger", 0, m_accessor); + + if (m_appliedMemoryRegions.empty()) + { + // No memory map available from the backend: fall back to one blanket region covering the entire + // address space so any address remains readable. It is named "debugger", which is the sentinel + // BinaryView::FindAll* checks to keep search disabled (search over a 2^64 blanket would hang). + data->GetMemoryMap()->AddRemoteMemoryRegion("debugger", 0, m_accessor); + m_debuggerRegionNames.push_back("debugger"); + } + else + { + // Mirror the backend's memory map as one bounded remote region per entry. Because the ranges are + // bounded, GetBackedAddressRanges()/GetNextBackedAddress() (which drive Find) only cover mapped + // memory, so search scans real regions and skips the gaps instead of walking the whole space. + // TODO: adding N accessor-backed regions is O(N^2) (each AddRemoteMemoryRegion clones the whole + // memory map). A bulk remote-region API in the core would make this O(N); acceptable here because + // SyncMemoryRegions only rebuilds when the map actually changes. + for (size_t i = 0; i < m_appliedMemoryRegions.size(); i++) + { + const DebugMemoryRegion& region = m_appliedMemoryRegions[i]; + if (region.m_size == 0) + continue; + + uint32_t flags = 0; + if (region.m_read) + flags |= SegmentReadable; + if (region.m_write) + flags |= SegmentWritable; + if (region.m_execute) + flags |= SegmentExecutable; + + std::string name = fmt::format("debugger:{}", i); + auto* accessor = new DebuggerFileAccessor(data, region.m_start, region.m_size); + if (data->GetMemoryMap()->AddRemoteMemoryRegion(name, region.m_start, accessor, flags)) + { + m_regionAccessors.push_back(accessor); + m_debuggerRegionNames.push_back(name); + } + else + { + // The region was rejected (e.g. a duplicate/overlap the core would not accept); the + // accessor was not adopted, so free it here. + delete accessor; + } + } + } + data->SetFunctionAnalysisUpdateDisabled(false); +} - return true; +void DebuggerController::RemoveDebuggerMemoryRegions() +{ + BinaryViewRef data = GetData(); + data->SetFunctionAnalysisUpdateDisabled(true); + for (const std::string& name : m_debuggerRegionNames) + data->GetMemoryMap()->RemoveMemoryRegion(name); + data->SetFunctionAnalysisUpdateDisabled(false); + m_debuggerRegionNames.clear(); + + // The regions are gone from the live map, but an in-flight BinaryView::Read may still hold a copy of + // the previous MemoryMap snapshot (which references these accessors' callbacks). Retire rather than + // free; m_retiredAccessors is drained at teardown. + m_retiredAccessors.insert(m_retiredAccessors.end(), m_regionAccessors.begin(), m_regionAccessors.end()); + m_regionAccessors.clear(); +} + + +void DebuggerController::SyncMemoryRegions() +{ + // GetMemoryMap() returns the backend's cached map, refreshed lazily (it was marked dirty by the stop + // that led here). Empty means the adapter does not support memory maps. + std::vector regions = GetMemoryMap(); + + // The user's requirement: do nothing when the map has not changed between stops. This keeps the + // common single-step case free of any memory-map churn. + if (regions == m_appliedMemoryRegions) + return; + + RemoveDebuggerMemoryRegions(); + m_appliedMemoryRegions = std::move(regions); + AddDebuggerMemoryRegions(); } @@ -2061,6 +2156,9 @@ void DebuggerController::ApplyOwnStateForEvent(const DebuggerEvent& event) m_ranges.clear(); DetectLoadedModule(); + // Refresh the BinaryView memory regions from the backend's (now up-to-date) memory map. No-ops + // when the map has not changed, so single-stepping stays cheap. + SyncMemoryRegions(); UpdateStackVariables(); AddRegisterValuesToExpressionParser(); AddModuleValuesToExpressionParser(); @@ -2112,6 +2210,19 @@ void DebuggerController::FinalizeTargetGoneCleanup() // so freeing the accessor first would leave the map with a dangling pointer that any concurrent // BinaryView::Read (e.g. a linear-view refresh triggered by TargetExited) would dereference. RemoveDebuggerMemoryRegion(); + // After the regions are gone from the map, the per-entry accessors accumulated over the session (all + // now in m_retiredAccessors, since RemoveDebuggerMemoryRegions retires them) can be freed. Same + // detached-thread rationale as m_accessor below: they hold a DbgRef. + if (!m_retiredAccessors.empty()) + { + auto retired = std::move(m_retiredAccessors); + m_retiredAccessors.clear(); + std::thread([retired]() { + for (auto* accessor : retired) + delete accessor; + }).detach(); + } + m_appliedMemoryRegions.clear(); if (m_accessor) { // Defer deletion to a detached thread. The accessor holds a DbgRef; @@ -4272,19 +4383,18 @@ void DebuggerController::OnRebased(BinaryView* oldView, BinaryView* newView) bool DebuggerController::RemoveDebuggerMemoryRegion() { - GetData()->SetFunctionAnalysisUpdateDisabled(true); - auto ret = GetData()->GetMemoryMap()->RemoveMemoryRegion("debugger"); - GetData()->SetFunctionAnalysisUpdateDisabled(false); - return ret; + // Tear down whatever mirror is currently registered (blanket or per-entry). Kept as a thin wrapper + // because the rebase path and FinalizeTargetGoneCleanup call it by this name. + RemoveDebuggerMemoryRegions(); + return true; } bool DebuggerController::ReAddDebuggerMemoryRegion() { - GetData()->SetFunctionAnalysisUpdateDisabled(true); - auto ret = GetData()->GetMemoryMap()->AddRemoteMemoryRegion("debugger", 0, GetMemoryAccessor()); - GetData()->SetFunctionAnalysisUpdateDisabled(false); - return ret; + // Rebuild from the last-applied memory map (the mirror survives a rebase; only the view changed). + AddDebuggerMemoryRegions(); + return true; } diff --git a/core/debuggercontroller.h b/core/debuggercontroller.h index ee474462..c0edf1d6 100644 --- a/core/debuggercontroller.h +++ b/core/debuggercontroller.h @@ -102,6 +102,19 @@ namespace BinaryNinjaDebugger { FileMetadataRef m_file; BinaryViewRef m_data; DebuggerFileAccessor* m_accessor {}; + // When the backend reports a memory map, we mirror it into the BinaryView as one bounded remote + // region per entry (instead of the single blanket overlay). These track the live mirror so we can + // tear it down and diff it against the next stop's map. + std::vector m_regionAccessors; + // Accessors from a previous memory-map generation. The MemoryMap may still hold copies of their + // callbacks in an in-flight snapshot, so we retire rather than free them on change and only delete + // them at teardown. Map changes are rare (module load, new mmap), so this stays small. + std::vector m_retiredAccessors; + // Names of the memory regions we currently have registered on m_data ("debugger" for the blanket + // fallback, or "debugger:" for the per-entry mirror). + std::vector m_debuggerRegionNames; + // The memory map currently reflected in m_data, used to no-op when nothing changed between stops. + std::vector m_appliedMemoryRegions; // This is the start address of the first file segments in the m_data. Unlike the return value of GetStart(), // this does not change even if we add the debugger memory region. In the future, this should be provided by // the binary view -- we will no longer need to track it ourselves @@ -667,6 +680,17 @@ namespace BinaryNinjaDebugger { bool RemoveDebuggerMemoryRegion(); bool ReAddDebuggerMemoryRegion(); + // Re-sync the BinaryView memory regions with the backend's current memory map. Called on every + // stop; no-ops when the map is unchanged. When the backend reports a map, mirrors it as bounded + // regions so search only scans mapped memory; otherwise falls back to the blanket overlay (search + // stays disabled -- see the "debugger" region gate in BinaryView::FindAll*). + void SyncMemoryRegions(); + // Register memory regions on m_data from m_appliedMemoryRegions. Assumes any prior regions were + // already removed. Empty map -> single blanket "debugger" region backed by m_accessor. + void AddDebuggerMemoryRegions(); + // Remove every region we registered (blanket or per-entry) and retire the per-entry accessors. + void RemoveDebuggerMemoryRegions(); + uint64_t GetViewFileSegmentsStart() { return m_viewStart; } bool ComputeExprValueAPI(const LowLevelILInstruction& instr, intx::uint512& value); diff --git a/core/debuggerfileaccessor.cpp b/core/debuggerfileaccessor.cpp index f837a211..91bb8ef7 100644 --- a/core/debuggerfileaccessor.cpp +++ b/core/debuggerfileaccessor.cpp @@ -40,9 +40,20 @@ DebuggerFileAccessor::DebuggerFileAccessor(BinaryView* parent) } +DebuggerFileAccessor::DebuggerFileAccessor(BinaryView* parent, uint64_t base, uint64_t length) +{ + (void)base; + m_length = length; + m_aggressiveAnalysisUpdate = Settings::Instance()->Get("debugger.aggressiveAnalysisUpdate"); + m_controller = DebuggerController::GetController(parent); + // Per-region accessor: no view-refresh subscription (the primary accessor owns that). + m_eventCallback = DEBUGGER_NO_EVENT_CALLBACK; +} + + DebuggerFileAccessor::~DebuggerFileAccessor() { - if (m_controller) + if (m_controller && m_eventCallback != DEBUGGER_NO_EVENT_CALLBACK) m_controller->RemoveEventCallback(m_eventCallback); } diff --git a/core/debuggerfileaccessor.h b/core/debuggerfileaccessor.h index 4f7f5adc..23a0f3b7 100644 --- a/core/debuggerfileaccessor.h +++ b/core/debuggerfileaccessor.h @@ -31,12 +31,23 @@ namespace BinaryNinjaDebugger uint64_t m_length; DbgRef m_controller; + // The stop-event subscription that refreshes the views. Only the "primary" accessor (the one + // covering the whole address space) registers it; the per-region accessors leave it as + // DEBUGGER_NO_EVENT_CALLBACK so we do not fire N redundant view refreshes on every stop. size_t m_eventCallback; bool m_aggressiveAnalysisUpdate; + static constexpr size_t DEBUGGER_NO_EVENT_CALLBACK = (size_t)-1; + public: + // Primary accessor: spans the whole address space and owns the view-refresh subscription. Used + // as the blanket overlay when the backend does not report a memory map. DebuggerFileAccessor(BinaryView* parent); + // Bounded accessor: backs a single memory-map region [base, base + length). Reads still resolve + // absolute addresses (the region is created in absolute-address mode), so this only differs from + // the primary accessor in its reported length and in not owning an event subscription. + DebuggerFileAccessor(BinaryView* parent, uint64_t base, uint64_t length); ~DebuggerFileAccessor(); bool IsValid() const override { return true; } uint64_t GetLength() const override; From 6cfa2e5df049051e23497276e688e3b3c4b5feae Mon Sep 17 00:00:00 2001 From: Xusheng Date: Fri, 17 Jul 2026 14:38:30 -0400 Subject: [PATCH 2/2] Add debugger.useMemoryMapSegments setting to toggle new vs old overlay Add a boolean setting (default on) that selects whether the debugger mirrors the backend memory map into bounded segments (new behavior, lets Find work while debugging) or keeps the single blanket overlay spanning the whole address space (previous behavior). The value is sampled once when the debugger view is created and cached in the controller, since SyncMemoryRegions consults it on every stop; toggling it takes effect on the next launch/attach. Co-Authored-By: Claude Opus 4.8 (1M context) --- core/debugger.cpp | 9 +++++++++ core/debuggercontroller.cpp | 17 ++++++++++++++--- core/debuggercontroller.h | 4 ++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/core/debugger.cpp b/core/debugger.cpp index 4aea0bdb..d37ef548 100644 --- a/core/debugger.cpp +++ b/core/debugger.cpp @@ -146,6 +146,15 @@ static void RegisterSettings() "ignore" : ["SettingsProjectScope", "SettingsResourceScope"] })"); + settings->RegisterSetting("debugger.useMemoryMapSegments", + R"({ + "title" : "Apply the target memory map as segments", + "type" : "boolean", + "default" : true, + "description" : "When enabled and the debug adapter reports a memory map, the debugger mirrors it into the binary view as one bounded segment per mapped region, which lets Find work during debugging (it only scans mapped memory). When disabled, the debugger falls back to a single overlay spanning the whole address space (the previous behavior). Adapters that do not report a memory map always use the overlay.", + "ignore" : ["SettingsProjectScope", "SettingsResourceScope"] + })"); + settings->RegisterSetting("debugger.safeMode", R"({ "title" : "Safe Mode", diff --git a/core/debuggercontroller.cpp b/core/debuggercontroller.cpp index 8369e57e..bdad2351 100644 --- a/core/debuggercontroller.cpp +++ b/core/debuggercontroller.cpp @@ -1446,6 +1446,10 @@ bool DebuggerController::CreateDebuggerBinaryView() m_state->GetMemory()->PrefillValueCache(); + // Sample the segment-application mode once for this debug session (SyncMemoryRegions consults it on + // every stop). Toggling the setting takes effect on the next launch/attach. + m_useMemoryMapSegments = Settings::Instance()->Get("debugger.useMemoryMapSegments"); + // The primary accessor spans the whole address space and owns the stop-event view-refresh // subscription. It backs the blanket "debugger" region in the fallback case, and stays alive for its // refresh role even when we mirror a real memory map into bounded regions. @@ -1536,9 +1540,16 @@ void DebuggerController::RemoveDebuggerMemoryRegions() void DebuggerController::SyncMemoryRegions() { - // GetMemoryMap() returns the backend's cached map, refreshed lazily (it was marked dirty by the stop - // that led here). Empty means the adapter does not support memory maps. - std::vector regions = GetMemoryMap(); + // The cached setting selects the new bounded-segment behavior or the old blanket overlay. Leaving + // `regions` empty makes the logic below fall back to the single "debugger" overlay + // (AddDebuggerMemoryRegions treats an empty map that way). + std::vector regions; + if (m_useMemoryMapSegments) + { + // GetMemoryMap() returns the backend's cached map, refreshed lazily (it was marked dirty by the + // stop that led here). Empty means the adapter does not support memory maps. + regions = GetMemoryMap(); + } // The user's requirement: do nothing when the map has not changed between stops. This keeps the // common single-step case free of any memory-map churn. diff --git a/core/debuggercontroller.h b/core/debuggercontroller.h index c0edf1d6..c30221dd 100644 --- a/core/debuggercontroller.h +++ b/core/debuggercontroller.h @@ -115,6 +115,10 @@ namespace BinaryNinjaDebugger { std::vector m_debuggerRegionNames; // The memory map currently reflected in m_data, used to no-op when nothing changed between stops. std::vector m_appliedMemoryRegions; + // Cached "debugger.useMemoryMapSegments" setting, sampled once when the debugger view is created + // (SyncMemoryRegions runs on every stop, so we do not want to hit Settings each time). When false, + // the debugger keeps the old blanket overlay instead of mirroring the backend memory map. + bool m_useMemoryMapSegments = true; // This is the start address of the first file segments in the m_data. Unlike the return value of GetStart(), // this does not change even if we add the debugger memory region. In the future, this should be provided by // the binary view -- we will no longer need to track it ourselves