Fixes for multiple issues in DSC/KC Triage views.

1. Crash on kernel cache image load
2. Duplicate Load Image buttons in kernel cache view
3. Improper selection behavior in both views
4. Address column should no longer resize to be smaller than contents
5. Symbol tables should now be properly sortable

2 files changed, 177 insertions, 240 deletions

diff --git a/view/kernelcache/ui/kctriage.cpp b/view/kernelcache/ui/kctriage.cpp
index 7b3cf9f0..aa2d7bb1 100644
--- a/view/kernelcache/ui/kctriage.cpp
+++ b/view/kernelcache/ui/kctriage.cpp
@@ -1,152 +1,14 @@
-#include "globalarea.h"
-#include "kctriage.h"
-#include "progresstask.h"
-#include "ui/fontsettings.h"
 #include <QMessageBox>
 #include <QPainter>
 #include <cmath>
+#include "globalarea.h"
+#include "kctriage.h"
+#include "ui/fontsettings.h"
 
 using namespace BinaryNinja;
 using namespace KernelCacheAPI;
 
 
-SymbolTableModel::SymbolTableModel(SymbolTableView* parent)
-	: QAbstractTableModel(parent), m_parent(parent) {
-	// 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_symbols.size());
-}
-
-
-int SymbolTableModel::columnCount(const QModelIndex& parent) const {
-	Q_UNUSED(parent);
-	// We have 3 columns: Address, Name, and Image
-	return 3;
-}
-
-
-QVariant SymbolTableModel::data(const QModelIndex& index, int role) const {
-	if (!index.isValid() || (role != Qt::DisplayRole && role != Qt::FontRole)) {
-		return QVariant();
-	}
-
-	const KCSymbol& symbol = m_symbols.at(index.row());
-
-	switch (role)
-	{
-	case Qt::DisplayRole:
-	{
-		switch (index.column()) {
-		case 0: // Address column
-			return QString("0x%1").arg(symbol.address, 0, 16); // Display address as hexadecimal
-		case 1: // Name column
-			return QString::fromStdString(symbol.name);
-		case 2: // Image column
-			return QString::fromStdString(symbol.image);
-		default:
-			return QVariant();
-		}
-	}
-	case Qt::FontRole:
-		return m_font;
-	default:
-		return QVariant();
-	}
-}
-
-
-QVariant SymbolTableModel::headerData(int section, Qt::Orientation orientation, int role) const {
-	if (role != Qt::DisplayRole || orientation != Qt::Horizontal) {
-		return QVariant();
-	}
-
-	switch (section) {
-	case 0:
-		return QString("Address");
-	case 1:
-		return QString("Name");
-	case 2:
-		return QString("Image");
-	default:
-		return QVariant();
-	}
-}
-
-
-void SymbolTableModel::updateSymbols() {
-	m_symbols = m_parent->m_symbols;
-	setFilter(m_filter);
-}
-
-
-const KCSymbol& SymbolTableModel::symbolAt(int row) const {
-	return m_symbols.at(row);
-}
-
-
-void SymbolTableModel::setFilter(std::string text)
-{
-	beginResetModel();
-
-	m_filter = text;
-	m_symbols.clear();
-
-	if (!m_filter.empty())
-	{
-		m_symbols.reserve(m_parent->m_symbols.size());
-		for (const auto& symbol : m_parent->m_symbols)
-			if (symbol.name.find(m_filter) != std::string::npos)
-				m_symbols.push_back(symbol);
-		m_symbols.shrink_to_fit();
-	}
-	else
-	{
-		m_symbols = m_parent->m_symbols;
-	}
-
-	endResetModel();
-}
-
-
-SymbolTableView::SymbolTableView(QWidget* parent, Ref<KernelCache> cache)
-	: QTableView(parent), m_model(new SymbolTableModel(this)) {
-
-	// Set up the filter model
-	setModel(m_model);
-
-	// Configure view settings
-	horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch);
-	setEditTriggers(QAbstractItemView::NoEditTriggers);
-	setSelectionBehavior(QAbstractItemView::SelectRows);
-	setSelectionMode(QAbstractItemView::SingleSelection);
-
-	setSortingEnabled(true);
-
-	BackgroundThread::create(this)->thenBackground([this, cache](){
-		// LogInfo("Symbol Search: Loading symbols...");
-		m_symbols = cache->LoadAllSymbolsAndWait();
-		// LogInfo("Symbol Search: Loaded 0x%zx symbols", m_symbols.size());
-	})->thenMainThread([this](){
-		m_model->updateSymbols();
-	})->start();
-}
-
-
-SymbolTableView::~SymbolTableView() {
-	delete m_model;
-}
-
-
-void SymbolTableView::setFilter(const std::string& filter) {
-	m_model->setFilter(filter);
-}
-
-
 KCTriageViewType::KCTriageViewType()
 	: ViewType("KCTriage", "Kernel Cache Triage")
 {}
@@ -194,8 +56,6 @@ KCTriageView::KCTriageView(QWidget* parent, BinaryViewRef data) : QWidget(parent
 	m_layout->addWidget(m_triageTabs);
 	setLayout(m_layout);
 
-	// XXX: RefreshData
-
 	m_triageTabs->selectWidget(defaultWidget);
 }
 
@@ -261,18 +121,18 @@ QWidget* KCTriageView::initImageTable()
 	auto loadImageButton = new QPushButton();
 	connect(loadImageButton, &QPushButton::clicked, [this](bool) {
 		// Collect only visible selected rows
-		QModelIndexList visibleSelectedRows;
+		QModelIndexList selected;
 		for (const auto& index : m_imageTable->selectionModel()->selectedRows()) {
 			if (!m_imageTable->isRowHidden(index.row())) {
-				visibleSelectedRows.append(index);
+				selected.append(index);
 			}
 		}
 
-		if (visibleSelectedRows.empty())
+		if (selected.empty())
 			return;
 
-		for (const auto& selection : visibleSelectedRows) {
-			auto name = selection.data().toString().toStdString();
+		for (const auto& selection : selected) {
+			auto name = m_imageModel->item(selection.row(), 1)->text().toStdString();
 			WorkerPriorityEnqueue([this, name]() { m_cache->LoadImageWithInstallName(name); });
 		}
 	});
@@ -284,7 +144,7 @@ QWidget* KCTriageView::initImageTable()
 	});
 
 	connect(m_imageTable, &FilterableTableView::activated, this, [=](const QModelIndex& index) {
-		auto selected = m_imageModel->item(index.row(), 0);
+		auto selected = m_imageModel->item(index.row(), 1);
 		auto name = selected->text().toStdString();
 		WorkerPriorityEnqueue([this, name]() { m_cache->LoadImageWithInstallName(name); });
 	});
@@ -292,12 +152,11 @@ QWidget* KCTriageView::initImageTable()
 	auto loadImageLayout = new QVBoxLayout;
 	loadImageLayout->addWidget(loadImageFilterEdit);
 	loadImageLayout->addWidget(m_imageTable);
-	loadImageLayout->addWidget(loadImageButton);
 
-	auto buttonLayout = new QHBoxLayout;
-	buttonLayout->addWidget(loadImageButton);
-	buttonLayout->setAlignment(Qt::AlignLeft);
-	loadImageLayout->addLayout(buttonLayout);
+	auto loadImageFooterLayout = new QHBoxLayout;
+	loadImageFooterLayout->addWidget(loadImageButton);
+	loadImageFooterLayout->setAlignment(Qt::AlignLeft);
+	loadImageLayout->addLayout(loadImageFooterLayout);
 
 	auto loadImageWidget = new QWidget;
 	loadImageWidget->setLayout(loadImageLayout);
@@ -306,7 +165,7 @@ QWidget* KCTriageView::initImageTable()
 
 	m_imageTable->setEditTriggers(QAbstractItemView::NoEditTriggers);
 
-	m_imageTable->horizontalHeader()->setSectionResizeMode(0, QHeaderView::Fixed);
+	m_imageTable->horizontalHeader()->setSectionResizeMode(0, QHeaderView::ResizeToContents);
 	m_imageTable->horizontalHeader()->setSectionResizeMode(1, QHeaderView::Stretch);
 
 	m_imageTable->setSelectionBehavior(QAbstractItemView::SelectRows);
@@ -332,9 +191,6 @@ void KCTriageView::initSymbolTable()
 		m_symbolTable->setFilter(filter.toStdString());
 	});
 
-	// Apply custom column styling
-	m_symbolTable->setItemDelegateForColumn(0, new AddressColorDelegate(m_symbolTable));
-
 	auto symbolLayout = new QVBoxLayout;
 	symbolLayout->addWidget(symbolFilterEdit);
 	symbolLayout->addWidget(m_symbolTable);
@@ -342,13 +198,6 @@ void KCTriageView::initSymbolTable()
 	auto symbolWidget = new QWidget;
 	symbolWidget->setLayout(symbolLayout);
 
-	m_symbolTable->horizontalHeader()->setSectionResizeMode(0, QHeaderView::ResizeToContents); // Address
-	m_symbolTable->horizontalHeader()->setSectionResizeMode(1, QHeaderView::Stretch);          // Name
-	m_symbolTable->horizontalHeader()->setSectionResizeMode(2, QHeaderView::Stretch);          // Image
-
-	m_symbolTable->setSelectionBehavior(QAbstractItemView::SelectRows);
-	m_symbolTable->setSelectionMode(QAbstractItemView::SingleSelection);
-
 	std::function<void(uint64_t)> navigateToAddress = [=](uint64_t addr) {
 		ExecuteOnMainThread([addr, this](){
 			if (Settings::Instance()->Get<bool>("ui.view.graph.preferred"))
diff --git a/view/kernelcache/ui/kctriage.h b/view/kernelcache/ui/kctriage.h
index 07c24fad..7d8bdfc1 100644
--- a/view/kernelcache/ui/kctriage.h
+++ b/view/kernelcache/ui/kctriage.h
@@ -1,25 +1,26 @@
-#include <kernelcacheapi.h>
-#include <binaryninjaapi.h>
-#include "uitypes.h"
-#include "viewframe.h"
-#include "animation.h"
-#include "uicontext.h"
-
-#include <QTableView>
-#include <QStandardItemModel>
-#include <QSortFilterProxyModel>
 #include <QHeaderView>
-#include "filter.h"
+#include <QSortFilterProxyModel>
+#include <QStandardItemModel>
 #include <QStyledItemDelegate>
-
+#include <QTableView>
+#include <binaryninjaapi.h>
+#include <kernelcacheapi.h>
+#include <progresstask.h>
+#include "filter.h"
 #include "ui/fontsettings.h"
+#include "uicontext.h"
+#include "uitypes.h"
+#include "viewframe.h"
 
 #ifndef BINARYNINJA_KCTRIAGE_H
 #define BINARYNINJA_KCTRIAGE_H
 
+
+using namespace KernelCacheAPI;
+
+
 class AddressColorDelegate : public QStyledItemDelegate
 {
-
 public:
 	explicit AddressColorDelegate(QObject* parent = nullptr) : QStyledItemDelegate(parent) {}
 
@@ -28,6 +29,7 @@ public:
 		QStyleOptionViewItem opt = option;
 		initStyleOption(&opt, index);
 
+		opt.font = getMonospaceFont(qobject_cast<QWidget*>(parent()));
 		opt.palette.setColor(QPalette::Text, getThemeColor(BNThemeColor::AddressColor));
 		opt.displayAlignment = Qt::AlignCenter | Qt::AlignVCenter;
 
@@ -36,6 +38,21 @@ public:
 };
 
 
+class MonospaceFontDelegate : public QStyledItemDelegate {
+public:
+	explicit MonospaceFontDelegate(QObject* parent = nullptr) : QStyledItemDelegate(parent) {}
+
+	void paint(QPainter* painter, const QStyleOptionViewItem& option, const QModelIndex& index) const override {
+		QStyleOptionViewItem opt = option;
+		initStyleOption(&opt, index);
+
+		opt.font = getMonospaceFont(qobject_cast<QWidget*>(parent()));
+
+		QStyledItemDelegate::paint(painter, opt, index);
+	}
+};
+
+
 class FilterableTableView : public QTableView, public FilterTarget {
 	Q_OBJECT
 
@@ -73,7 +90,10 @@ public:
 
 	void scrollToFirstItem() override {
 		if (model()->rowCount() > 0) {
-			scrollTo(model()->index(0, 0));
+			QModelIndex top = indexAt(rect().topLeft());
+			if (top.isValid()) {
+				scrollTo(top);
+			}
 		}
 	}
 
@@ -86,109 +106,177 @@ public:
 
 	void selectFirstItem() override {
 		if (model()->rowCount() > 0) {
-			QModelIndex firstIndex = model()->index(0, 0);
-			selectionModel()->select(firstIndex, QItemSelectionModel::ClearAndSelect);
+			QModelIndex top = indexAt(rect().topLeft());
+			if (top.isValid()) {
+				selectionModel()->select(top, QItemSelectionModel::ClearAndSelect);
+				setCurrentIndex(top);
+			}
 		}
 	}
 
 	void activateFirstItem() override {
 		if (model()->rowCount() > 0) {
-			QModelIndex firstIndex = model()->index(0, 0);
-			setCurrentIndex(firstIndex);
-			emit activated(firstIndex);
-		}
-	}
-
-	bool eventFilter(QObject* obj, QEvent* event) override {
-		if (event->type() == QEvent::KeyPress) {
-			auto keyEvent = dynamic_cast<QKeyEvent*>(event);
-			if (keyEvent->key() == Qt::Key_Escape) {
-				clearSelection();
-				return true;
-			}
-			if (keyEvent->key() == Qt::Key_Enter || keyEvent->key() == Qt::Key_Return) {
-				emit activated(currentIndex());
-				return true;
+			QModelIndex topLeft = indexAt(rect().topLeft());
+			if (topLeft.isValid()) {
+				setCurrentIndex(topLeft);
+				emit activated(topLeft);
 			}
 		}
-		return QTableView::eventFilter(obj, event);
 	}
 
 signals:
 	void filterTextChanged(const QString& text);
 };
 
-class SymbolTableView;
-
-class SymbolTableModel : public QAbstractTableModel {
-	Q_OBJECT
 
-	SymbolTableView* m_parent;
-	QFont m_font;
-	std::string m_filter;
-	std::vector<KernelCacheAPI::KCSymbol> m_symbols;
+class SymbolTableProxyModel : public QSortFilterProxyModel
+{
+Q_OBJECT
 
 public:
-	explicit SymbolTableModel(SymbolTableView* parent);
+	SymbolTableProxyModel(QObject* parent = nullptr) : QSortFilterProxyModel(parent), m_timer(new QTimer(this))
+	{
+		m_timer->setSingleShot(true);
+		connect(m_timer, &QTimer::timeout, this, &SymbolTableProxyModel::delayedFilterChanged);
+	}
+
+	void setFilterString(const QString& filter)
+	{
+		QRegularExpression newRegEx(QRegularExpression::escape(filter), QRegularExpression::CaseInsensitiveOption);
+		if (m_filter != newRegEx) {
+			m_filter = std::move(newRegEx);
+			m_timer->start(200);
+		}
+	}
 
-	int rowCount(const QModelIndex& parent) const override;
-	int columnCount(const QModelIndex& parent) const override;
-	QVariant data(const QModelIndex& index, int role) const override;
-	QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
-	void updateSymbols();
-	void setFilter(std::string text);
+protected:
+	bool filterAcceptsRow(int source_row, const QModelIndex& source_parent) const override
+	{
+		if (m_filter.pattern().isEmpty())
+			return true;
 
-	const KernelCacheAPI::KCSymbol& symbolAt(int row) const;
+		for (int column = 0; column < sourceModel()->columnCount(source_parent); ++column)
+		{
+			QModelIndex index = sourceModel()->index(source_row, column, source_parent);
+			QString data = sourceModel()->data(index).toString();
+			if (m_filter.match(data).hasMatch())
+				return true;
+		}
+		return false;
+	}
+
+private slots:
+	void delayedFilterChanged()
+	{
+		invalidateFilter();
+	}
+
+private:
+	QRegularExpression m_filter;
+	QTimer* m_timer;
 };
 
 
 class SymbolTableView : public QTableView, public FilterTarget
 {
-	Q_OBJECT
+Q_OBJECT
 	friend class SymbolTableModel;
 
 	std::vector<KernelCacheAPI::KCSymbol> m_symbols;
-
-	SymbolTableModel* m_model;
+	QStandardItemModel* m_model;
+	SymbolTableProxyModel* m_proxyModel;
 
 public:
-	SymbolTableView(QWidget* parent, Ref<KernelCacheAPI::KernelCache> cache);
-	~SymbolTableView() override;
+	SymbolTableView(QWidget* parent, Ref<KernelCache>& cache)
+		: QTableView(parent), m_model(new QStandardItemModel(this)), m_proxyModel(new SymbolTableProxyModel(this))
+	{
+		m_proxyModel->setSourceModel(m_model);
+		setModel(m_proxyModel);
 
-	void scrollToFirstItem() override {
-		if (model()->rowCount() > 0) {
-			scrollTo(model()->index(0, 0));
-		}
+		// Set up the headers
+		m_model->setColumnCount(3);
+		m_model->setHorizontalHeaderLabels({"Address", "Name", "Image"});
+		setFont(getMonospaceFont(parent));
+		setItemDelegateForColumn(0, new AddressColorDelegate(this));
+		setItemDelegateForColumn(1, new MonospaceFontDelegate(this));
+		setItemDelegateForColumn(2, new MonospaceFontDelegate(this));
+
+		// Configure view settings
+		horizontalHeader()->setSectionResizeMode(0, QHeaderView::ResizeToContents);
+		horizontalHeader()->setSectionResizeMode(1, QHeaderView::Stretch);
+		horizontalHeader()->setSectionResizeMode(2, QHeaderView::Stretch);
+		setEditTriggers(QAbstractItemView::NoEditTriggers);
+		setSelectionBehavior(QAbstractItemView::SelectRows);
+		setSelectionMode(QAbstractItemView::SingleSelection);
+		verticalHeader()->setVisible(false);
+
+		setSortingEnabled(true);
+
+		BackgroundThread::create(this)->thenBackground([this, cache](){
+			m_symbols = cache->LoadAllSymbolsAndWait();
+		})->thenMainThread([this](){
+			updateSymbols();
+		})->start();
 	}
 
-	void scrollToCurrentItem() override {
-		QModelIndex currentIndex = selectionModel()->currentIndex();
-		if (currentIndex.isValid()) {
-			scrollTo(currentIndex);
+	~SymbolTableView() override = default;
+
+	void updateSymbols()
+	{
+		m_model->removeRows(0, m_model->rowCount());
+		for (const auto& symbol : m_symbols)
+		{
+			QList<QStandardItem*> row;
+			row << new QStandardItem(QString("0x%1").arg(symbol.address, 0, 16))
+				<< new QStandardItem(QString::fromStdString(symbol.name))
+				<< new QStandardItem(QString::fromStdString(symbol.image));
+			m_model->appendRow(row);
 		}
 	}
 
-	void selectFirstItem() override {
-		if (model()->rowCount() > 0) {
-			QModelIndex firstIndex = model()->index(0, 0);
-			selectionModel()->select(firstIndex, QItemSelectionModel::ClearAndSelect);
-		}
+	KernelCacheAPI::KCSymbol getSymbolAtRow(int row) const
+	{
+		QModelIndex proxyIndex = m_proxyModel->index(row, 0);
+		QModelIndex sourceIndex = m_proxyModel->mapToSource(proxyIndex);
+		return m_symbols[sourceIndex.row()];
 	}
 
-	void activateFirstItem() override {
-		if (model()->rowCount() > 0) {
-			QModelIndex firstIndex = model()->index(0, 0);
-			setCurrentIndex(firstIndex);
-			emit activated(firstIndex);
+	void scrollToFirstItem() override
+	{
+		scrollToTop();
+	}
+
+	void scrollToCurrentItem() override
+	{
+		scrollTo(selectionModel()->currentIndex());
+	}
+
+	void selectFirstItem() override
+	{
+		if (m_proxyModel->rowCount() > 0) {
+			QModelIndex idx = m_proxyModel->index(0, 0);
+			if (idx.isValid()) {
+				selectionModel()->select(idx, QItemSelectionModel::ClearAndSelect);
+				setCurrentIndex(idx);
+			}
 		}
 	}
 
-	KernelCacheAPI::KCSymbol getSymbolAtRow(int row) const
+	void activateFirstItem() override
 	{
-		return m_model->symbolAt(row);
+		if (m_proxyModel->rowCount() > 0) {
+			QModelIndex idx = m_proxyModel->index(0, 0);
+			if (idx.isValid()) {
+				setCurrentIndex(idx);
+				emit activated(idx);
+			}
+		}
 	}
 
-	void setFilter(const std::string& filter) override;
+	void setFilter(const std::string& text) override
+	{
+		m_proxyModel->setFilterString(QString::fromStdString(text));
+	}
 };