From a2ccfa22aa3f04484cba5dd8e58aeda51d518287 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Wed, 9 Jul 2025 18:20:47 -0700 Subject: [PATCH 1/3] [DSC] Fix GetRegionAt / optimize GetRegionContaining `GetRegionAt` was really providing the semantics of `GetRegionContaining` due to `m_regions` being an `AddressRangeMap` and using transparent comparators to allow `find` to work with any address in the range. `GetRegionAt` is updated to verify that the start address of the region that was found matches the requested address. `GetRegionContaining` is updated to use `AddressRangeMap::find` rather than doing a linear search over all regions. --- view/sharedcache/core/SharedCache.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/view/sharedcache/core/SharedCache.cpp b/view/sharedcache/core/SharedCache.cpp index 0198ac4887..c36c9ede11 100644 --- a/view/sharedcache/core/SharedCache.cpp +++ b/view/sharedcache/core/SharedCache.cpp @@ -488,17 +488,17 @@ std::optional SharedCache::GetEntryWithImage(const CacheImage& image std::optional SharedCache::GetRegionAt(const uint64_t address) const { const auto it = m_regions.find(address); - if (it == m_regions.end()) + if (it == m_regions.end() || it->second.start != address) return std::nullopt; return it->second; } std::optional SharedCache::GetRegionContaining(const uint64_t address) const { - for (const auto& [range, region] : m_regions) - if (address >= range.start && address < range.end) - return region; - return std::nullopt; + const auto it = m_regions.find(address); + if (it == m_regions.end()) + return std::nullopt; + return it->second; } std::optional SharedCache::GetImageAt(const uint64_t address) const From 83684ec8fd3aae8a4db3f0d35251677fd82954ac Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Wed, 9 Jul 2025 19:03:14 -0700 Subject: [PATCH 2/3] [DSC] Use BNAllocStringWithLength when copying strings for the API We're copying `std::string` objects that know their length. There's no reason to force a call to `strlen`. --- view/sharedcache/core/ffi.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/view/sharedcache/core/ffi.cpp b/view/sharedcache/core/ffi.cpp index 1ae561ae09..81c3acf7a1 100644 --- a/view/sharedcache/core/ffi.cpp +++ b/view/sharedcache/core/ffi.cpp @@ -7,7 +7,7 @@ using namespace BinaryNinja::DSC; BNSharedCacheImage ImageToApi(const CacheImage& image) { BNSharedCacheImage apiImage; - apiImage.name = BNAllocString(image.path.c_str()); + apiImage.name = BNAllocStringWithLength(image.path.c_str(), image.path.size()); apiImage.headerAddress = image.headerAddress; apiImage.regionStartCount = image.regionStarts.size(); uint64_t* regionStarts = new uint64_t[image.regionStarts.size()]; @@ -65,7 +65,7 @@ BNSharedCacheRegion RegionToApi(const CacheRegion& region) { BNSharedCacheRegion apiRegion; apiRegion.vmAddress = region.start; - apiRegion.name = BNAllocString(region.name.c_str()); + apiRegion.name = BNAllocStringWithLength(region.name.c_str(), region.name.size()); apiRegion.size = region.size; apiRegion.flags = region.flags; apiRegion.regionType = RegionTypeToApi(region.type); @@ -88,7 +88,7 @@ CacheRegion RegionFromApi(const BNSharedCacheRegion& apiRegion) BNSharedCacheSymbol SymbolToApi(const CacheSymbol& symbol) { BNSharedCacheSymbol apiSymbol; - apiSymbol.name = BNAllocString(symbol.name.c_str()); + apiSymbol.name = BNAllocStringWithLength(symbol.name.data(), symbol.name.size()); apiSymbol.address = symbol.address; apiSymbol.symbolType = symbol.type; return apiSymbol; @@ -133,8 +133,10 @@ BNSharedCacheMappingInfo MappingToApi(const dyld_cache_mapping_info& mapping) BNSharedCacheEntry EntryToApi(const CacheEntry& entry) { BNSharedCacheEntry apiEntry; - apiEntry.path = BNAllocString(entry.GetFilePath().c_str()); - apiEntry.name = BNAllocString(entry.GetFileName().c_str()); + auto path = entry.GetFilePath(); + auto name = entry.GetFileName(); + apiEntry.path = BNAllocStringWithLength(path.c_str(), path.size()); + apiEntry.name = BNAllocStringWithLength(name.c_str(), name.size()); apiEntry.entryType = EntryTypeToApi(entry.GetType()); const auto& mappings = entry.GetMappings(); apiEntry.mappingCount = mappings.size(); From 55d3bda18a8f929d19557ec0d215eef66c68506b Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Wed, 9 Jul 2025 19:04:30 -0700 Subject: [PATCH 3/3] [DSC] Performance improvements and clean-up in SymbolTableView Avoid copying the vector of symbols where possible. When no filter string is applied we no longer copy the entire vector symbols by having the symbols used for display be indirected via a pointer. It points either to the unfiltered symbols or the filtered symbols. Some unncessary copies have been removed by using `std::move` where appropriate. Filtering has been updated to avoid `std::vector::reserve` / `std::vector::shrink_to_fit` in favor of letting the filtered vector control its own growth and reuse its buffer. This avoids allocating a ~100MB buffer every time we update the filter only to later reallocate and move the contents into a smaller buffer. Instead the buffer grows via `std::vector`'s usual growth algorithm and it is preserved across filter calls to avoid unnecessarily reallocating it, and only shrink the buffer if it has shrunk significantly vs previous iterations. --- view/sharedcache/ui/symboltable.cpp | 59 ++++++++++++++++------------- view/sharedcache/ui/symboltable.h | 14 ++++--- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/view/sharedcache/ui/symboltable.cpp b/view/sharedcache/ui/symboltable.cpp index b0247f2be9..e4c5dcd8b7 100644 --- a/view/sharedcache/ui/symboltable.cpp +++ b/view/sharedcache/ui/symboltable.cpp @@ -11,8 +11,9 @@ using namespace BinaryNinja; using namespace SharedCacheAPI; -SymbolTableModel::SymbolTableModel(SymbolTableView* parent) - : QAbstractTableModel(parent), m_parent(parent) { +SymbolTableModel::SymbolTableModel(SymbolTableView* parent) : + QAbstractTableModel(parent), m_parent(parent), m_displaySymbols(&m_symbols) +{ // TODO: Need to implement updating this font if it is changed by the user m_font = getMonospaceFont(parent); } @@ -20,7 +21,7 @@ SymbolTableModel::SymbolTableModel(SymbolTableView* parent) int SymbolTableModel::rowCount(const QModelIndex& parent) const { Q_UNUSED(parent); - return static_cast(m_modelSymbols.size()); + return static_cast(m_displaySymbols->size()); } @@ -111,30 +112,28 @@ void SymbolTableModel::sort(int column, Qt::SortOrder order) if (order == Qt::DescendingOrder) { - std::sort(m_modelSymbols.begin(), m_modelSymbols.end(), - [&comparator](const CacheSymbol& a, const CacheSymbol& b) { - return comparator(b, a); - }); + std::sort(m_displaySymbols->begin(), m_displaySymbols->end(), + [&comparator](const CacheSymbol& a, const CacheSymbol& b) { return comparator(b, a); }); } else { - std::sort(m_modelSymbols.begin(), m_modelSymbols.end(), comparator); + std::sort(m_displaySymbols->begin(), m_displaySymbols->end(), comparator); } endResetModel(); } -void SymbolTableModel::updateSymbols(std::vector&& symbols) +void SymbolTableModel::updateSymbols(std::vector symbols) { - m_preparedSymbols = symbols; + m_symbols = std::move(symbols); setFilter(m_filter); } const CacheSymbol& SymbolTableModel::symbolAt(int row) const { - return m_modelSymbols.at(row); + return m_displaySymbols->at(row); } @@ -142,30 +141,40 @@ void SymbolTableModel::setFilter(std::string text) { beginResetModel(); - m_filter = text; - m_modelSymbols = {}; + m_filter = std::move(text); // Skip filtering if no filter applied. - if (!m_filter.empty()) + if (m_filter.empty()) { - m_modelSymbols.reserve(m_preparedSymbols.size()); - for (const auto& symbol : m_preparedSymbols) - if (((std::string_view)symbol.name).find(m_filter) != std::string::npos) - m_modelSymbols.push_back(symbol); - m_modelSymbols.shrink_to_fit(); + m_filteredSymbols = {}; + m_displaySymbols = &m_symbols; } else { - m_modelSymbols = m_preparedSymbols; + // Clear the filtered symbols while preserving the capacity + m_filteredSymbols.clear(); + + for (const auto& symbol : m_symbols) + { + if (symbol.name.find(m_filter) != std::string::npos) + m_filteredSymbols.push_back(symbol); + } + + // If the filtered vector is using less than 25% of its capacity, + // shrink it to reduce memory usage. + if (m_filteredSymbols.size() < m_filteredSymbols.capacity() / 4) + m_filteredSymbols.shrink_to_fit(); + + m_displaySymbols = &m_filteredSymbols; } endResetModel(); } -SymbolTableView::SymbolTableView(QWidget* parent) - : QTableView(parent), m_model(new SymbolTableModel(this)) { - +SymbolTableView::SymbolTableView(QWidget* parent) : + QTableView(parent), m_model(new SymbolTableModel(this)) +{ // Set up the filter model setModel(m_model); @@ -181,9 +190,7 @@ SymbolTableView::SymbolTableView(QWidget* parent) setSortingEnabled(true); } -SymbolTableView::~SymbolTableView() { - delete m_model; -} +SymbolTableView::~SymbolTableView() = default; void SymbolTableView::populateSymbols(BinaryView &view) { diff --git a/view/sharedcache/ui/symboltable.h b/view/sharedcache/ui/symboltable.h index 51ba88f0bc..9c7a1eabc9 100644 --- a/view/sharedcache/ui/symboltable.h +++ b/view/sharedcache/ui/symboltable.h @@ -19,9 +19,12 @@ Q_OBJECT SymbolTableView* m_parent; QFont m_font; std::string m_filter; - std::vector m_preparedSymbols{}; - // These are the symbols we actually use - std::vector m_modelSymbols{}; + + std::vector m_symbols; + std::vector m_filteredSymbols; + + // A pointer to either m_symbols or m_filteredSymbols, depending on whether a filter is applied. + std::vector *m_displaySymbols = nullptr; public: explicit SymbolTableModel(SymbolTableView* parent); @@ -31,10 +34,11 @@ Q_OBJECT QVariant data(const QModelIndex& index, int role) const override; QVariant headerData(int section, Qt::Orientation orientation, int role) const override; void sort(int column, Qt::SortOrder order) override; - void updateSymbols(std::vector&& symbols); + + void updateSymbols(std::vector symbols); void setFilter(std::string text); - const SharedCacheAPI::CacheSymbol& symbolAt(int row) const; + const SharedCacheAPI::CacheSymbol& symbolAt(int row) const; };