Debugger: Be smarter about deciding when functions should be hashed

3rdparty/ccc/src/ccc/symbol_database.cpp

@@ -753,7 +753,7 @@ void SourceFile::check_functions_match(const SymbolDatabase& database)
 	u32 modified = 0;
 	for(FunctionHandle function_handle : functions()) {
 		const ccc::Function* function = database.functions.symbol_from_handle(function_handle);
-		if(!function || function->original_hash() == 0) {
+		if(!function || function->current_hash() == 0 || function->original_hash() == 0) {
 			continue;
 		}
 		

pcsx2-qt/Debugger/SymbolTree/SymbolTreeNode.cpp

@@ -41,7 +41,7 @@ bool SymbolTreeNode::readFromVM(DebugInterface& cpu, const ccc::SymbolDatabase&
 
 	data_changed |= updateDisplayString(cpu, database);
 	data_changed |= updateLiveness(cpu);
-	data_changed |= updateHash(cpu);
+	data_changed |= updateMatchesMemory(cpu, database);
 
 	return data_changed;
 }
@@ -479,7 +479,7 @@ bool SymbolTreeNode::updateLiveness(DebugInterface& cpu)
 	return true;
 }
 
-bool SymbolTreeNode::updateHash(DebugInterface& cpu)
+bool SymbolTreeNode::updateMatchesMemory(DebugInterface& cpu, const ccc::SymbolDatabase& database)
 {
 	bool matching = true;
 
@@ -487,66 +487,54 @@ bool SymbolTreeNode::updateHash(DebugInterface& cpu)
 	{
 		case ccc::SymbolDescriptor::FUNCTION:
 		{
-			cpu.GetSymbolGuardian().Read([&](const ccc::SymbolDatabase& database) -> void {
 			const ccc::Function* function = database.functions.symbol_from_handle(symbol.handle());
-				if (!function || function->original_hash() == 0)
+			if (!function || function->current_hash() == 0 || function->original_hash() == 0)
-					return;
+				return false;
 
 			matching = function->current_hash() == function->original_hash();
-			});
+
 			break;
 		}
 		case ccc::SymbolDescriptor::GLOBAL_VARIABLE:
 		{
-			cpu.GetSymbolGuardian().Read([&](const ccc::SymbolDatabase& database) -> void {
 			const ccc::GlobalVariable* global_variable = database.global_variables.symbol_from_handle(symbol.handle());
 			if (!global_variable)
-					return;
+				return false;
 
 			const ccc::SourceFile* source_file = database.source_files.symbol_from_handle(global_variable->source_file());
 			if (!source_file)
-					return;
+				return false;
 
 			matching = source_file->functions_match();
-			});
+
 			break;
 		}
 		case ccc::SymbolDescriptor::LOCAL_VARIABLE:
 		{
-			cpu.GetSymbolGuardian().Read([&](const ccc::SymbolDatabase& database) -> void {
 			const ccc::LocalVariable* local_variable = database.local_variables.symbol_from_handle(symbol.handle());
 			if (!local_variable)
-					return;
+				return false;
 
 			const ccc::Function* function = database.functions.symbol_from_handle(local_variable->function());
-				if (!function)
+			if (!function || function->current_hash() == 0 || function->original_hash() == 0)
-					return;
+				return false;
 
-				const ccc::SourceFile* source_file = database.source_files.symbol_from_handle(function->source_file());
+			matching = function->current_hash() == function->original_hash();
-				if (!source_file)
-					return;
 
-				matching = source_file->functions_match();
-			});
 			break;
 		}
 		case ccc::SymbolDescriptor::PARAMETER_VARIABLE:
 		{
-			cpu.GetSymbolGuardian().Read([&](const ccc::SymbolDatabase& database) -> void {
 			const ccc::ParameterVariable* parameter_variable = database.parameter_variables.symbol_from_handle(symbol.handle());
 			if (!parameter_variable)
-					return;
+				return false;
 
 			const ccc::Function* function = database.functions.symbol_from_handle(parameter_variable->function());
-				if (!function)
+			if (!function || function->current_hash() == 0 || function->original_hash() == 0)
-					return;
+				return false;
 
-				const ccc::SourceFile* source_file = database.source_files.symbol_from_handle(function->source_file());
+			matching = function->current_hash() == function->original_hash();
-				if (!source_file)
-					return;
 
-				matching = source_file->functions_match();
-			});
 			break;
 		}
 		default:
@@ -567,6 +555,85 @@ bool SymbolTreeNode::matchesMemory() const
 	return m_matches_memory;
 }
 
+void SymbolTreeNode::updateSymbolHashes(std::span<const SymbolTreeNode*> nodes, DebugInterface& cpu, ccc::SymbolDatabase& database)
+{
+	std::set<ccc::FunctionHandle> functions;
+	std::set<ccc::SourceFile*> source_files;
+
+	// Determine which functions we need to hash again, and in the case of
+	// global variables, which source files are associated with those functions
+	// so that we can check if they still match.
+	for (const SymbolTreeNode* node : nodes)
+	{
+		switch (node->symbol.descriptor())
+		{
+			case ccc::SymbolDescriptor::FUNCTION:
+			{
+				functions.emplace(node->symbol.handle());
+				break;
+			}
+			case ccc::SymbolDescriptor::GLOBAL_VARIABLE:
+			{
+				const ccc::GlobalVariable* global_variable = database.global_variables.symbol_from_handle(node->symbol.handle());
+				if (!global_variable)
+					continue;
+
+				ccc::SourceFile* source_file = database.source_files.symbol_from_handle(global_variable->source_file());
+				if (!source_file)
+					continue;
+
+				for (ccc::FunctionHandle function : source_file->functions())
+					functions.emplace(function);
+
+				source_files.emplace(source_file);
+
+				break;
+			}
+			case ccc::SymbolDescriptor::LOCAL_VARIABLE:
+			{
+				const ccc::LocalVariable* local_variable = database.local_variables.symbol_from_handle(node->symbol.handle());
+				if (!local_variable)
+					continue;
+
+				functions.emplace(local_variable->function());
+
+				break;
+			}
+			case ccc::SymbolDescriptor::PARAMETER_VARIABLE:
+			{
+				const ccc::ParameterVariable* parameter_variable = database.parameter_variables.symbol_from_handle(node->symbol.handle());
+				if (!parameter_variable)
+					continue;
+
+				functions.emplace(parameter_variable->function());
+
+				break;
+			}
+			default:
+			{
+			}
+		}
+	}
+
+	// Update the hashes for the enumerated functions.
+	for (ccc::FunctionHandle function_handle : functions)
+	{
+		ccc::Function* function = database.functions.symbol_from_handle(function_handle);
+		if (!function || function->original_hash() == 0)
+			continue;
+
+		std::optional<ccc::FunctionHash> hash = SymbolGuardian::HashFunction(*function, cpu);
+		if (!hash.has_value())
+			continue;
+
+		function->set_current_hash(*hash);
+	}
+
+	// Check that the enumerated source files still have matching functions.
+	for (ccc::SourceFile* source_file : source_files)
+		source_file->check_functions_match(database);
+}
+
 bool SymbolTreeNode::anySymbolsValid(const ccc::SymbolDatabase& database) const
 {
 	if (symbol.lookup_symbol(database))

pcsx2-qt/Debugger/SymbolTree/SymbolTreeNode.h

@@ -62,9 +62,11 @@ public:
 
 	bool updateLiveness(DebugInterface& cpu);
 
-	bool updateHash(DebugInterface& cpu);
+	bool updateMatchesMemory(DebugInterface& cpu, const ccc::SymbolDatabase& database);
 	bool matchesMemory() const;
 
+	static void updateSymbolHashes(std::span<const SymbolTreeNode*> nodes, DebugInterface& cpu, ccc::SymbolDatabase& database);
+
 	bool anySymbolsValid(const ccc::SymbolDatabase& database) const;
 
 	const SymbolTreeNode* parent() const;

pcsx2-qt/Debugger/SymbolTree/SymbolTreeWidgets.cpp

@@ -24,17 +24,29 @@ SymbolTreeWidget::SymbolTreeWidget(u32 flags, s32 symbol_address_alignment, Debu
 
 	setupMenu();
 
-	connect(m_ui.refreshButton, &QPushButton::clicked, this, &SymbolTreeWidget::reset);
+	connect(m_ui.refreshButton, &QPushButton::clicked, this, [&]() {
+		m_cpu.GetSymbolGuardian().ReadWrite([&](ccc::SymbolDatabase& database) {
+			m_cpu.GetSymbolGuardian().UpdateFunctionHashes(database, m_cpu);
+		});
+
+		reset();
+	});
+
 	connect(m_ui.filterBox, &QLineEdit::textEdited, this, &SymbolTreeWidget::reset);
 
 	connect(m_ui.newButton, &QPushButton::clicked, this, &SymbolTreeWidget::onNewButtonPressed);
 	connect(m_ui.deleteButton, &QPushButton::clicked, this, &SymbolTreeWidget::onDeleteButtonPressed);
 
-	connect(m_ui.treeView->verticalScrollBar(), &QScrollBar::valueChanged, this, &SymbolTreeWidget::updateVisibleNodes);
+	connect(m_ui.treeView->verticalScrollBar(), &QScrollBar::valueChanged, this, [&]() {
+		updateVisibleNodes(false);
+	});
 
 	m_ui.treeView->setContextMenuPolicy(Qt::CustomContextMenu);
 	connect(m_ui.treeView, &QTreeView::customContextMenuRequested, this, &SymbolTreeWidget::openMenu);
-	connect(m_ui.treeView, &QTreeView::expanded, this, &SymbolTreeWidget::updateVisibleNodes);
+
+	connect(m_ui.treeView, &QTreeView::expanded, this, [&]() {
+		updateVisibleNodes(true);
+	});
 }
 
 SymbolTreeWidget::~SymbolTreeWidget() = default;
@@ -43,15 +55,15 @@ void SymbolTreeWidget::resizeEvent(QResizeEvent* event)
 {
 	QWidget::resizeEvent(event);
 
-	updateVisibleNodes();
+	updateVisibleNodes(false);
 }
 
 void SymbolTreeWidget::updateModel()
 {
 	if (!m_model || m_model->needsReset())
 		reset();
-
+	else
-	updateVisibleNodes();
+		updateVisibleNodes(true);
 }
 
 void SymbolTreeWidget::reset()
@@ -61,10 +73,6 @@ void SymbolTreeWidget::reset()
 
 	m_ui.treeView->setColumnHidden(SymbolTreeModel::SIZE, !m_show_size_column || !m_show_size_column->isChecked());
 
-	m_cpu.GetSymbolGuardian().ReadWrite([&](ccc::SymbolDatabase& database) {
-		m_cpu.GetSymbolGuardian().UpdateFunctionHashes(database, m_cpu);
-	});
-
 	SymbolFilters filters;
 	std::unique_ptr<SymbolTreeNode> root;
 	m_cpu.GetSymbolGuardian().Read([&](const ccc::SymbolDatabase& database) -> void {
@@ -82,25 +90,39 @@ void SymbolTreeWidget::reset()
 		m_model->reset(std::move(root));
 
 		// Read the initial values for visible nodes.
-		updateVisibleNodes();
+		updateVisibleNodes(true);
 
 		if (!filters.string.isEmpty())
 			expandGroups(QModelIndex());
 	}
 }
 
-void SymbolTreeWidget::updateVisibleNodes()
+void SymbolTreeWidget::updateVisibleNodes(bool update_hashes)
 {
 	if (!m_model)
 		return;
 
+	// Enumerate visible symbol nodes.
+	std::vector<const SymbolTreeNode*> nodes;
 	QModelIndex index = m_ui.treeView->indexAt(m_ui.treeView->rect().topLeft());
 	while (m_ui.treeView->visualRect(index).intersects(m_ui.treeView->viewport()->rect()))
 	{
-		m_model->setData(index, QVariant(), SymbolTreeModel::UPDATE_FROM_MEMORY_ROLE);
+		nodes.emplace_back(m_model->nodeFromIndex(index));
 		index = m_ui.treeView->indexBelow(index);
 	}
 
+	// Hash functions for symbols with visible nodes.
+	if (update_hashes)
+	{
+		m_cpu.GetSymbolGuardian().ReadWrite([&](ccc::SymbolDatabase& database) {
+			SymbolTreeNode::updateSymbolHashes(nodes, m_cpu, database);
+		});
+	}
+
+	// Update the values of visible nodes from memory.
+	for (const SymbolTreeNode* node : nodes)
+		m_model->setData(m_model->indexFromNode(*node), QVariant(), SymbolTreeModel::UPDATE_FROM_MEMORY_ROLE);
+
 	m_ui.treeView->update();
 }
 

pcsx2-qt/Debugger/SymbolTree/SymbolTreeWidgets.h

@@ -21,7 +21,7 @@ public:
 
 	void updateModel();
 	void reset();
-	void updateVisibleNodes();
+	void updateVisibleNodes(bool update_hashes);
 	void expandGroups(QModelIndex index);
 
 signals:
@@ -41,7 +41,7 @@ protected:
 		const ccc::SourceFile* source_file = nullptr;
 	};
 
-	explicit SymbolTreeWidget(u32 flags, s32 symbol_address_alignment, DebugInterface& cpu, QWidget* parent = nullptr);
+	SymbolTreeWidget(u32 flags, s32 symbol_address_alignment, DebugInterface& cpu, QWidget* parent = nullptr);
 
 	void resizeEvent(QResizeEvent* event) override;
 

pcsx2/DebugTools/SymbolGuardian.cpp

@@ -192,7 +192,14 @@ std::optional<ccc::FunctionHash> SymbolGuardian::HashFunction(const ccc::Functio
 	ccc::FunctionHash hash;
 
 	for (u32 i = 0; i < function.size() / 4; i++)
-		hash.update(reader.read32(function.address().value + i * 4));
+	{
+		bool valid;
+		u32 value = reader.read32(function.address().value + i * 4, valid);
+		if (!valid)
+			return std::nullopt;
+
+		hash.update(value);
+	}
 
 	return hash;
 }