[DSC] Miscellaneous correctness, performance, and memory fixes

view/sharedcache/core/SharedCache.cpp

@@ -488,17 +488,17 @@ std::optional<CacheEntry> SharedCache::GetEntryWithImage(const CacheImage& image
 std::optional<CacheRegion> 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<CacheRegion> 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<CacheImage> SharedCache::GetImageAt(const uint64_t address) const

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();

view/sharedcache/ui/symboltable.cpp

@@ -11,16 +11,17 @@ 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);
 }
 
 
 int SymbolTableModel::rowCount(const QModelIndex& parent) const {
 	Q_UNUSED(parent);
-	return static_cast<int>(m_modelSymbols.size());
+	return static_cast<int>(m_displaySymbols->size());
 }
 
 
@@ -111,61 +112,69 @@ 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<CacheSymbol>&& symbols)
+void SymbolTableModel::updateSymbols(std::vector<CacheSymbol> 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);
 }
 
 
 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)
 {

view/sharedcache/ui/symboltable.h

@@ -19,9 +19,12 @@ Q_OBJECT
 	SymbolTableView* m_parent;
 	QFont m_font;
 	std::string m_filter;
-	std::vector<SharedCacheAPI::CacheSymbol> m_preparedSymbols{};
-	// These are the symbols we actually use
-	std::vector<SharedCacheAPI::CacheSymbol> m_modelSymbols{};
+
+	std::vector<SharedCacheAPI::CacheSymbol> m_symbols;
+	std::vector<SharedCacheAPI::CacheSymbol> m_filteredSymbols;
+
+	// A pointer to either m_symbols or m_filteredSymbols, depending on whether a filter is applied.
+	std::vector<SharedCacheAPI::CacheSymbol> *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<SharedCacheAPI::CacheSymbol>&& symbols);
+
+	void updateSymbols(std::vector<SharedCacheAPI::CacheSymbol> symbols);
 	void setFilter(std::string text);
-	const SharedCacheAPI::CacheSymbol& symbolAt(int row) const;
 
+	const SharedCacheAPI::CacheSymbol& symbolAt(int row) const;
 };