Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debugger: Add conditional data breakpoints, and a few minor bug fixes #2473

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
8489e1f
Decrement bp counter when removing a bp from the list
mateusfavarin Aug 1, 2021
cb06da7
Fix default bp counter when cleared
mateusfavarin Aug 1, 2021
f98ccfd
Fix bp list UI not updating when clearing all bps
mateusfavarin Aug 1, 2021
d3d1a41
Fix bp ids not updating when a bp is removed
mateusfavarin Aug 1, 2021
12b29dd
Add conditional bp UI
mateusfavarin Aug 2, 2021
5364eab
Fix typo
mateusfavarin Aug 2, 2021
e66b633
UI tweaks, add data structs that will encapsulate the new debugging l…
mateusfavarin Aug 2, 2021
11e06cc
Change address size field to be a line
mateusfavarin Aug 2, 2021
e8d7441
Update data structure for s_breakpoints
mateusfavarin Aug 2, 2021
511e55c
Implement condition breakpoints
mateusfavarin Aug 3, 2021
a83f259
Add default textline values for the DebugPrompt
mateusfavarin Aug 3, 2021
f511bcc
Fix bp not disabling via checkbox
mateusfavarin Aug 3, 2021
411852d
Change instruction format to hex always use Hex for consistency
mateusfavarin Aug 3, 2021
a40286e
Add the possibility to edit current breakpoints
mateusfavarin Aug 3, 2021
1253f69
Breakpoint editor: fix cancel button not working
mateusfavarin Aug 3, 2021
18d93de
Add menu to edit and delete breakpoints
mateusfavarin Aug 3, 2021
5cec416
Conditional data bp: add support for lwl, lwr, swl, swr
mateusfavarin Aug 3, 2021
6e5f938
Fix breakpoint being created in the edit mode
mateusfavarin Aug 4, 2021
e4690b3
Make CPU registers editable
mateusfavarin Aug 5, 2021
c77da8b
Make disasm comments follow the selected addr in the codeView
mateusfavarin Aug 6, 2021
71b5f37
CPU Reg edit: only scroll to PC if PC was changed
mateusfavarin Aug 6, 2021
90b5e4b
Make stack editable
mateusfavarin Aug 6, 2021
8a7efb2
Add string as a search pattern in the memory view
mateusfavarin Aug 7, 2021
157662d
Change LookAhead memread function
mateusfavarin Aug 7, 2021
09bf376
Add a range check in DataBreakpointCheck
mateusfavarin Aug 8, 2021
844ab25
Change bp status bar message
mateusfavarin Aug 9, 2021
a39ddd4
Fix range address tests
mateusfavarin Aug 25, 2021
e74d0ee
Merge branch 'stenzek:master' into debugger
mateusfavarin Aug 25, 2021
5b97601
Merge branch 'stenzek:master' into debugger
mateusfavarin Sep 7, 2021
2228e33
Fix crash when closing the debugger
mateusfavarin Sep 9, 2021
d9334eb
Merge branch 'stenzek:master' into debugger
mateusfavarin Sep 9, 2021
8c46371
Remove unnecessary import
mateusfavarin Sep 9, 2021
1a95f1f
Fix crash when loading a save state with the debugger unpaused
mateusfavarin Sep 9, 2021
45a9c10
Merge branch 'stenzek:master' into debugger
mateusfavarin Sep 10, 2021
e5bd628
Merge branch 'stenzek:master' into debugger
mateusfavarin Sep 25, 2021
752a4f4
Make instructions editable
mateusfavarin Sep 25, 2021
997e290
Add memory size selector in the mem viewer
mateusfavarin Sep 25, 2021
ed83c6f
Always highlight selected address
mateusfavarin Sep 26, 2021
1a78ff2
Add memory display UI
mateusfavarin Sep 26, 2021
9fbea5f
Merge branch 'stenzek:master' into debugger
mateusfavarin Sep 26, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
308 changes: 265 additions & 43 deletions src/core/cpu_core.cpp

Large diffs are not rendered by default.

11 changes: 7 additions & 4 deletions src/core/cpu_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ using BreakpointCallback = bool (*)(VirtualMemoryAddress address);

struct Breakpoint
{
VirtualMemoryAddress address;
DebugAddress dbg;
BreakpointCallback callback;
u32 number;
u32 hit_count;
Expand All @@ -181,12 +181,15 @@ using BreakpointList = std::vector<Breakpoint>;
bool HasAnyBreakpoints();
bool HasBreakpointAtAddress(VirtualMemoryAddress address);
BreakpointList GetBreakpointList(bool include_auto_clear = false, bool include_callbacks = false);
bool AddBreakpoint(VirtualMemoryAddress address, bool auto_clear = false, bool enabled = true);
bool AddBreakpointWithCallback(VirtualMemoryAddress address, BreakpointCallback callback);
bool RemoveBreakpoint(VirtualMemoryAddress address);
bool AddBreakpoint(DebugAddress dbg, bool auto_clear = false, bool enabled = true);
bool AddBreakpointWithCallback(DebugAddress dbg, BreakpointCallback callback);
bool RemoveBreakpoint(DebugAddress dbg);
void ClearBreakpoints();
bool AddStepOverBreakpoint();
bool AddStepOutBreakpoint(u32 max_instructions_to_search = 1000);
void SetBreakpointEnable(int index, bool is_enable);
DebugAddress GetBreakpointDebugAddress(int index);
void SetBreakpointDebugAddress(int index, DebugAddress dbg);

extern bool TRACE_EXECUTION;

Expand Down
25 changes: 19 additions & 6 deletions src/core/cpu_disasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ static void FormatInstruction(String* dest, const Instruction inst, u32 pc, cons
{
dest->Clear();

if (inst.bits == 0)
{
dest->AppendString("nop");
return;
}

const char* str = format;
while (*str != '\0')
{
Expand Down Expand Up @@ -197,36 +203,43 @@ static void FormatInstruction(String* dest, const Instruction inst, u32 pc, cons
}
else if (std::strncmp(str, "shamt", 5) == 0)
{
dest->AppendFormattedString("%d", ZeroExtend32(inst.r.shamt.GetValue()));
const s32 shamt = static_cast<s32>(ZeroExtend32(inst.r.shamt.GetValue()));
if (shamt < 0)
dest->AppendFormattedString("-%x", shamt * -1);
else
dest->AppendFormattedString("%x", shamt);
str += 5;
}
else if (std::strncmp(str, "immu", 4) == 0)
{
dest->AppendFormattedString("%u", inst.i.imm_zext32());
dest->AppendFormattedString("0x%04x", inst.i.imm_zext32());
str += 4;
}
else if (std::strncmp(str, "imm", 3) == 0)
{
// dest->AppendFormattedString("%d", static_cast<int>(inst.i.imm_sext32()));
dest->AppendFormattedString("%04x", inst.i.imm_zext32());
dest->AppendFormattedString("0x%04x", inst.i.imm_zext32());
str += 3;
}
else if (std::strncmp(str, "rel", 3) == 0)
{
const u32 target = (pc + UINT32_C(4)) + (inst.i.imm_sext32() << 2);
dest->AppendFormattedString("%08x", target);
dest->AppendFormattedString("0x%08x", target);
str += 3;
}
else if (std::strncmp(str, "offsetrs", 8) == 0)
{
const s32 offset = static_cast<s32>(inst.i.imm_sext32());
dest->AppendFormattedString("%d(%s)", offset, GetRegName(inst.i.rs));
if (offset < 0)
dest->AppendFormattedString("-0x%x(%s)", offset * -1, GetRegName(inst.i.rs));
else
dest->AppendFormattedString("0x%x(%s)", offset, GetRegName(inst.i.rs));
str += 8;
}
else if (std::strncmp(str, "jt", 2) == 0)
{
const u32 target = ((pc + UINT32_C(4)) & UINT32_C(0xF0000000)) | (inst.j.target << 2);
dest->AppendFormattedString("%08x", target);
dest->AppendFormattedString("0x%08x", target);
str += 2;
}
else if (std::strncmp(str, "copcc", 5) == 0)
Expand Down
12 changes: 10 additions & 2 deletions src/core/gdb_protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ static std::optional<std::string> Cmd$z1(const std::string_view& data)
{
auto address = StringUtil::FromChars<VirtualMemoryAddress>(data, 16);
if (address) {
CPU::RemoveBreakpoint(*address);
DebugAddress dbg;
dbg.address = *address;
dbg.debug_type = DebugType::Executed;
dbg.size = 4;
CPU::RemoveBreakpoint(dbg);
return { "OK" };
}
else {
Expand All @@ -226,7 +230,11 @@ static std::optional<std::string> Cmd$Z1(const std::string_view& data)
{
auto address = StringUtil::FromChars<VirtualMemoryAddress>(data, 16);
if (address) {
CPU::AddBreakpoint(*address, false);
DebugAddress dbg;
dbg.address = *address;
dbg.debug_type = DebugType::Executed;
dbg.size = 4;
CPU::AddBreakpoint(dbg, false);
return { "OK" };
}
else {
Expand Down
15 changes: 15 additions & 0 deletions src/core/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@ enum class MemoryAccessSize : u32
Word
};

struct DebugAddress
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than combining the breakpoints, it'd be better to store the execution and data breakpoints in separate arrays (fewer loops/comparisons for every guest instruction).

{
VirtualMemoryAddress address;
u32 size;
u8 debug_type = 0;
};

enum DebugType : u8
{
Read = 1,
Written = 2,
Changed = 4,
Executed = 8
};

using TickCount = s32;

enum class ConsoleRegion
Expand Down
2 changes: 1 addition & 1 deletion src/duckstation-qt/cheatmanagerdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ void CheatManagerDialog::addToWatchClicked()

void CheatManagerDialog::addManualWatchAddressClicked()
{
std::optional<unsigned> address = QtUtils::PromptForAddress(this, windowTitle(), tr("Enter manual address:"), false);
std::optional<unsigned> address = QtUtils::PromptForAddress(this, windowTitle(), false);
if (!address.has_value())
return;

Expand Down
107 changes: 104 additions & 3 deletions src/duckstation-qt/debuggermodels.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
#include "debuggermodels.h"
#include "core/cpu_core.h"
#include "core/cpu_core.cpp"
#include "core/cpu_core_private.h"
#include "core/cpu_disasm.h"
#include <QtGui/QColor>
#include <QtGui/QIcon>
#include <QtGui/QPalette>
#include <QtWidgets/QApplication>
#include <QtWidgets/QTreeView>

static constexpr int NUM_COLUMNS = 5;
static constexpr int STACK_RANGE = 128;
Expand Down Expand Up @@ -95,7 +97,7 @@ QVariant DebuggerCodeModel::data(const QModelIndex& index, int role /*= Qt::Disp
case 4:
{
// Comment
if (address != m_last_pc)
if (address != m_last_pc && address != selected_address)
return QVariant();

u32 instruction_bits;
Expand Down Expand Up @@ -241,6 +243,11 @@ void DebuggerCodeModel::setPC(VirtualMemoryAddress pc)
}
}

void DebuggerCodeModel::setCurrentSelectedAddress(VirtualMemoryAddress address)
{
selected_address = address;
}

void DebuggerCodeModel::ensureAddressVisible(VirtualMemoryAddress address)
{
updateRegion(address);
Expand Down Expand Up @@ -288,6 +295,16 @@ DebuggerRegistersModel::DebuggerRegistersModel(QObject* parent /*= nullptr*/) :

DebuggerRegistersModel::~DebuggerRegistersModel() {}

void DebuggerRegistersModel::setCodeModel(DebuggerCodeModel* model)
{
code_model = model;
}

void DebuggerRegistersModel::setCodeView(QTreeView* view)
{
code_view = view;
}

int DebuggerRegistersModel::rowCount(const QModelIndex& parent /*= QModelIndex()*/) const
{
return static_cast<int>(CPU::Reg::count);
Expand Down Expand Up @@ -327,6 +344,8 @@ QVariant DebuggerRegistersModel::data(const QModelIndex& index, int role /*= Qt:
if (CPU::g_state.regs.r[reg_index] != m_old_reg_values[reg_index])
return QColor(255, 50, 50);
}
else if (role == Qt::EditRole)
return QString::asprintf("%08X", CPU::g_state.regs.r[reg_index]);
}
break;

Expand Down Expand Up @@ -357,6 +376,55 @@ QVariant DebuggerRegistersModel::headerData(int section, Qt::Orientation orienta
}
}

Qt::ItemFlags DebuggerRegistersModel::flags(const QModelIndex& index) const
{
if (index.column() == 1)
return Qt::ItemIsEditable | QAbstractItemModel::flags(index);
return QAbstractItemModel::flags(index);
}

bool DebuggerRegistersModel::setData(const QModelIndex& index, const QVariant& value, int role)
{
if (index.isValid() && role == Qt::EditRole)
{
bool ok;
QString value_str = value.toString();
u32 reg_value;
if (value_str.startsWith("0x"))
reg_value = value_str.mid(2).toUInt(&ok, 16);
else
reg_value = value_str.toUInt(&ok, 16);
if (ok)
{
u32 reg_index = static_cast<u32>(index.row());
if (reg_index == 34) // special case for PC
{
CPU::g_state.regs.npc = reg_value & 0xFFFFFFFC; // making sure it's divisible by 4
CPU::FlushPipeline();
}
else if (reg_index == 35) // special case for NPC
CPU::g_state.regs.npc = reg_value & 0xFFFFFFFC; // making sure it's divisible by 4
else
CPU::g_state.regs.r[reg_index] = reg_value;

// Update code view comments with the new register value
code_model->setPC(CPU::g_state.regs.pc);
code_model->ensureAddressVisible(CPU::g_state.regs.pc);
int row = code_model->getRowForAddress(CPU::g_state.regs.pc);
if (row >= 0)
{
qApp->processEvents(QEventLoop::ExcludeUserInputEvents);
code_view->scrollTo(code_model->index(row, 0));
}

emit dataChanged(index, index, {role});

return true;
}
}
return false;
}

void DebuggerRegistersModel::invalidateView()
{
beginResetModel();
Expand Down Expand Up @@ -388,7 +456,7 @@ QVariant DebuggerStackModel::data(const QModelIndex& index, int role /*= Qt::Dis
if (index.column() < 0 || index.column() > 1)
return QVariant();

if (role != Qt::DisplayRole)
if (role != Qt::DisplayRole && role != Qt::EditRole)
return QVariant();

const u32 sp = CPU::g_state.regs.sp;
Expand All @@ -402,7 +470,33 @@ QVariant DebuggerStackModel::data(const QModelIndex& index, int role /*= Qt::Dis
if (!CPU::SafeReadMemoryWord(address, &value))
return tr("<invalid>");

return QString::asprintf("0x%08X", ZeroExtend32(value));
if (role == Qt::DisplayRole)
return QString::asprintf("0x%08X", ZeroExtend32(value));

return QString::asprintf("%08X", ZeroExtend32(value));
}

bool DebuggerStackModel::setData(const QModelIndex& index, const QVariant& value, int role)
{
if (index.isValid() && role == Qt::EditRole)
{
bool ok;
QString value_str = value.toString();
u32 stack_value;
if (value_str.startsWith("0x"))
stack_value = value_str.mid(2).toUInt(&ok, 16);
else
stack_value = value_str.toUInt(&ok, 16);
if (ok)
{
const u32 sp = CPU::g_state.regs.sp;
const VirtualMemoryAddress address =
(sp - static_cast<u32>(STACK_RANGE * STACK_VALUE_SIZE)) + static_cast<u32>(index.row()) * STACK_VALUE_SIZE;
std::memcpy(&Bus::g_ram[address & Bus::g_ram_mask], &stack_value, sizeof(stack_value));
return true;
}
}
return false;
}

QVariant DebuggerStackModel::headerData(int section, Qt::Orientation orientation, int role /*= Qt::DisplayRole*/) const
Expand All @@ -424,6 +518,13 @@ QVariant DebuggerStackModel::headerData(int section, Qt::Orientation orientation
}
}

Qt::ItemFlags DebuggerStackModel::flags(const QModelIndex& index) const
{
if (index.column() == 1)
return Qt::ItemIsEditable | QAbstractItemModel::flags(index);
return QAbstractItemModel::flags(index);
}

void DebuggerStackModel::invalidateView()
{
beginResetModel();
Expand Down
11 changes: 11 additions & 0 deletions src/duckstation-qt/debuggermodels.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <QtCore/QAbstractListModel>
#include <QtCore/QAbstractTableModel>
#include <QtGui/QPixmap>
#include <QtWidgets/QTreeView>
#include <map>

class DebuggerCodeModel : public QAbstractTableModel
Expand All @@ -27,6 +28,7 @@ class DebuggerCodeModel : public QAbstractTableModel
VirtualMemoryAddress getAddressForIndex(QModelIndex index) const;
void setPC(VirtualMemoryAddress pc);
void ensureAddressVisible(VirtualMemoryAddress address);
void setCurrentSelectedAddress(VirtualMemoryAddress address);
void setBreakpointList(std::vector<VirtualMemoryAddress> bps);
void setBreakpointState(VirtualMemoryAddress address, bool enabled);
void clearBreakpoints();
Expand All @@ -41,6 +43,7 @@ class DebuggerCodeModel : public QAbstractTableModel
VirtualMemoryAddress m_code_region_start = 0;
VirtualMemoryAddress m_code_region_end = 0;
VirtualMemoryAddress m_last_pc = 0;
VirtualMemoryAddress selected_address = 0xFFFFFFFF;
std::vector<VirtualMemoryAddress> m_breakpoints;

QPixmap m_pc_pixmap;
Expand All @@ -54,17 +57,23 @@ class DebuggerRegistersModel : public QAbstractListModel
public:
DebuggerRegistersModel(QObject* parent = nullptr);
virtual ~DebuggerRegistersModel();
void setCodeModel(DebuggerCodeModel* model);
void setCodeView(QTreeView* view);

virtual int rowCount(const QModelIndex& parent = QModelIndex()) const override;
virtual int columnCount(const QModelIndex& parent = QModelIndex()) const override;
virtual QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override;
virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override;
virtual Qt::ItemFlags flags(const QModelIndex& index) const override;
virtual bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override;

void invalidateView();
void saveCurrentValues();

private:
u32 m_old_reg_values[static_cast<u32>(CPU::Reg::count)] = {};
DebuggerCodeModel* code_model;
QTreeView* code_view;
};

class DebuggerStackModel : public QAbstractListModel
Expand All @@ -78,7 +87,9 @@ class DebuggerStackModel : public QAbstractListModel
virtual int rowCount(const QModelIndex& parent = QModelIndex()) const override;
virtual int columnCount(const QModelIndex& parent = QModelIndex()) const override;
virtual QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override;
virtual bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override;
virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override;
virtual Qt::ItemFlags flags(const QModelIndex& index) const override;

void invalidateView();
};