Skip to content

Commit

Permalink
Debugger: Be smarter about deciding when functions should be hashed
Browse files Browse the repository at this point in the history
  • Loading branch information
@chaoticgd @F0bes
chaoticgd authored and F0bes committed Oct 18, 2024
1 parent ed4fbb4 commit f77bf1e
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 59 deletions.
2 changes: 1 addition & 1 deletion 3rdparty/ccc/src/ccc/symbol_database.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
149 changes: 108 additions & 41 deletions pcsx2-qt/Debugger/SymbolTree/SymbolTreeNode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -479,74 +479,62 @@ bool SymbolTreeNode::updateLiveness(DebugInterface& cpu)
return true;
}

bool SymbolTreeNode::updateHash(DebugInterface& cpu)
bool SymbolTreeNode::updateMatchesMemory(DebugInterface& cpu, const ccc::SymbolDatabase& database)
{
bool matching = true;

switch (symbol.descriptor())
{
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)
return;
const ccc::Function* function = database.functions.symbol_from_handle(symbol.handle());
if (!function || function->current_hash() == 0 || function->original_hash() == 0)
return false;

matching = function->current_hash() == function->original_hash();

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;
const ccc::GlobalVariable* global_variable = database.global_variables.symbol_from_handle(symbol.handle());
if (!global_variable)
return false;

const ccc::SourceFile* source_file = database.source_files.symbol_from_handle(global_variable->source_file());
if (!source_file)
return;
const ccc::SourceFile* source_file = database.source_files.symbol_from_handle(global_variable->source_file());
if (!source_file)
return false;

matching = source_file->functions_match();

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;
const ccc::LocalVariable* local_variable = database.local_variables.symbol_from_handle(symbol.handle());
if (!local_variable)
return false;

const ccc::Function* function = database.functions.symbol_from_handle(local_variable->function());
if (!function)
return;
const ccc::Function* function = database.functions.symbol_from_handle(local_variable->function());
if (!function || function->current_hash() == 0 || function->original_hash() == 0)
return false;

const ccc::SourceFile* source_file = database.source_files.symbol_from_handle(function->source_file());
if (!source_file)
return;
matching = function->current_hash() == function->original_hash();

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;
const ccc::ParameterVariable* parameter_variable = database.parameter_variables.symbol_from_handle(symbol.handle());
if (!parameter_variable)
return false;

const ccc::Function* function = database.functions.symbol_from_handle(parameter_variable->function());
if (!function)
return;
const ccc::Function* function = database.functions.symbol_from_handle(parameter_variable->function());
if (!function || function->current_hash() == 0 || function->original_hash() == 0)
return false;

const ccc::SourceFile* source_file = database.source_files.symbol_from_handle(function->source_file());
if (!source_file)
return;
matching = function->current_hash() == function->original_hash();

matching = source_file->functions_match();
});
break;
}
default:
Expand All @@ -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))
Expand Down
4 changes: 3 additions & 1 deletion pcsx2-qt/Debugger/SymbolTree/SymbolTreeNode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,11 @@ struct SymbolTreeNode

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;
Expand Down
48 changes: 35 additions & 13 deletions pcsx2-qt/Debugger/SymbolTree/SymbolTreeWidgets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();

updateVisibleNodes();
else
updateVisibleNodes(true);
}

void SymbolTreeWidget::reset()
Expand All @@ -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 {
Expand All @@ -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();
}

Expand Down
4 changes: 2 additions & 2 deletions pcsx2-qt/Debugger/SymbolTree/SymbolTreeWidgets.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class SymbolTreeWidget : public QWidget

void updateModel();
void reset();
void updateVisibleNodes();
void updateVisibleNodes(bool update_hashes);
void expandGroups(QModelIndex index);

signals:
Expand All @@ -41,7 +41,7 @@ class SymbolTreeWidget : public QWidget
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;

Expand Down
9 changes: 8 additions & 1 deletion pcsx2/DebugTools/SymbolGuardian.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit f77bf1e

Please sign in to comment.