Skip to content

Auto-fill the instruction text when adding a TTD bookmark (Fixes #1108) #1151

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.

Sign up for GitHub

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

Open
xusheng6 wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Auto-fill the instruction text when adding a TTD bookmark (Fixes #1108) #1151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 170 additions & 51 deletions ui/ttdbookmarkwidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,117 @@ static uint64_t PositionSortValue(const TTDPosition& position)
}


// Parses a hex address, with or without the 0x prefix. Returns 0 if the text cannot be parsed
static uint64_t ParseHexAddress(const QString& text)
{
QString clean = text.trimmed();
if (clean.isEmpty())
return 0;

if (clean.startsWith("0x") || clean.startsWith("0X"))
clean = clean.mid(2);

bool ok;
uint64_t address = clean.toULongLong(&ok, 16);
return ok ? address : 0;
}


static QString FormatTTDPosition(const TTDPosition& position)
{
return QString("%1:%2").arg(position.sequence, 0, 16).arg(position.step, 0, 16);
}


// Parses a "sequence:step" position, both in hex. Returns false if the text cannot be parsed
static bool ParseTTDPositionText(const QString& text, TTDPosition& result)
{
QStringList parts = text.trimmed().split(':');
if (parts.size() != 2)
return false;

bool ok1, ok2;
uint64_t sequence = parts[0].toULongLong(&ok1, 16);
uint64_t step = parts[1].toULongLong(&ok2, 16);
if (!ok1 || !ok2)
return false;

result = TTDPosition(sequence, step);
return true;
}


static QString FlattenTokens(const std::vector<InstructionTextToken>& tokens)
{
QString result;
for (const auto& token : tokens)
result += QString::fromStdString(token.text);

// The tokens are padded so that the operands line up in the disassembly view, which we do not want here
return result.simplified();
}


// Returns the text of the instruction at the given address, rendered the same way the disassembly view renders it,
// e.g., "call sub_140001000". Returns an empty string if the address cannot be read or disassembled
static QString GetInstructionTextAtAddress(BinaryViewRef data, uint64_t address)
{
if (!data || (address == 0))
return "";

auto functions = data->GetAnalysisFunctionsContainingAddress(address);
if (functions.empty() || !functions[0])
return "";

// Passing no settings makes the renderer use the default disassembly settings
Ref<DisassemblyTextRenderer> renderer = new DisassemblyTextRenderer(functions[0]);

size_t length = 0;
std::vector<DisassemblyTextLine> lines;
if (!renderer->GetDisassemblyText(address, length, lines) || lines.empty())
return "";

return FlattenTokens(lines[0].tokens);
}


// Returns a description of the given address that does not depend on the memory contents at any particular TTD
// position, e.g., "sub_140001000+0x12". Returns an empty string if nothing is known about the address
static QString GetSymbolTextAtAddress(BinaryViewRef data, uint64_t address)
{
if (!data || (address == 0))
return "";

SymbolRef symbol;
uint64_t start = address;

auto functions = data->GetAnalysisFunctionsContainingAddress(address);
if (!functions.empty() && functions[0])
{
symbol = functions[0]->GetSymbol();
start = functions[0]->GetStart();
}
else if ((symbol = data->GetSymbolByAddress(address)))
{
start = symbol->GetAddress();
}

if (!symbol)
return "";

QString result = QString::fromStdString(symbol->GetShortName());
if (address > start)
result += QString("+0x%1").arg(address - start, 0, 16);

return result;
}


// TTDBookmarkEditDialog implementation

TTDBookmarkEditDialog::TTDBookmarkEditDialog(QWidget* parent, const QString& position, const QString& note,
const QString& viewAddress)
: QDialog(parent)
const QString& viewAddress, DbgRef<DebuggerController> controller)
: QDialog(parent), m_controller(controller), m_noteEdited(!note.isEmpty())
{
setWindowTitle(position.isEmpty() ? "Add TTD Bookmark" : "Edit TTD Bookmark");
setModal(true);
Expand All @@ -54,6 +160,14 @@ TTDBookmarkEditDialog::TTDBookmarkEditDialog(QWidget* parent, const QString& pos
m_noteEdit->setMinimumWidth(400);
layout->addRow("Note:", m_noteEdit);

// Pre-fill the note with the instruction at the view address, so the bookmark is meaningful without the user

@fuzyll fuzyll Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the logic here correct? I haven't built this and ran it, but if m_noteEdited is !note.isEmpty (above, line 140), doesn't that mean that deleting a note and leaving it empty will cause it to auto-fill?

If this is intentional/desired, awesome, I just didn't understand that from the comment here. Seemed like an edge-case that wasn't addressed instead.

// having to type anything. Keep it in sync while the user edits the position or the view address, until they
// edit the note themselves, at which point the tooltip no longer describes what is in the field
connect(m_noteEdit, &QLineEdit::textEdited, this, [&]() { m_noteEdited = true; m_noteEdit->setToolTip(""); });
connect(m_positionEdit, &QLineEdit::textEdited, this, [&]() { updateAutoNote(); });
connect(m_viewAddressEdit, &QLineEdit::textEdited, this, [&]() { updateAutoNote(); });
updateAutoNote();

auto buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept);
connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject);
Expand All @@ -64,6 +178,48 @@ TTDBookmarkEditDialog::TTDBookmarkEditDialog(QWidget* parent, const QString& pos
m_noteEdit->selectAll();
}

QString TTDBookmarkEditDialog::getDefaultNote()
{
m_noteEdit->setToolTip("");

if (!m_controller)
return "";

BinaryViewRef data = m_controller->GetData();
uint64_t address = ParseHexAddress(m_viewAddressEdit->text());
if (!data || (address == 0))
return "";

QString note = GetInstructionTextAtAddress(data, address);
if (note.isEmpty())
return GetSymbolTextAtAddress(data, address);

if (!m_controller->IsConnected() || !m_controller->IsTTD())
return note;

// The instruction is disassembled from the memory the target has right now, i.e., at the current position. If the
// bookmark is for a different position, the code at this address may have been different back then, so say which
// position the disassembly is from and explain it in the tooltip
TTDPosition current = m_controller->GetCurrentTTDPosition();
TTDPosition position;
if (ParseTTDPositionText(m_positionEdit->text(), position) && (position == current))
return note;

m_noteEdit->setToolTip(QString("The instruction is disassembled from the memory of the target at the current "
"position %1, which is not the position being bookmarked. If the code at this address changed in between, "
"this is not the instruction that executed at the bookmarked position.").arg(FormatTTDPosition(current)));

return note + QString(" (disassembled at %1)").arg(FormatTTDPosition(current));
}

void TTDBookmarkEditDialog::updateAutoNote()
{
if (m_noteEdited)
return;

m_noteEdit->setText(getDefaultNote());
}

QString TTDBookmarkEditDialog::getPosition() const { return m_positionEdit->text().trimmed(); }
QString TTDBookmarkEditDialog::getNote() const { return m_noteEdit->text().trimmed(); }
QString TTDBookmarkEditDialog::getViewAddress() const { return m_viewAddressEdit->text().trimmed(); }
Expand Down Expand Up @@ -212,7 +368,7 @@ void TTDBookmarkWidget::refreshTable()

m_resultsTable->setItem(i, IndexColumn, new NumericalTableWidgetItem(QString::number(i + 1), i + 1));

QString posStr = QString("%1:%2").arg(bookmark.position.sequence, 0, 16).arg(bookmark.position.step, 0, 16);
QString posStr = FormatTTDPosition(bookmark.position);
m_resultsTable->setItem(i, PositionColumn, new NumericalTableWidgetItem(posStr, PositionSortValue(bookmark.position)));

if (bookmark.viewAddress != 0)
Expand Down Expand Up @@ -334,7 +490,7 @@ void TTDBookmarkWidget::showContextMenu(const QPoint& position)

void TTDBookmarkWidget::addBookmarkFromDialog()
{
TTDBookmarkEditDialog dialog(this);
TTDBookmarkEditDialog dialog(this, "", "", "", m_controller);
if (dialog.exec() != QDialog::Accepted)
return;

Expand All @@ -356,20 +512,8 @@ void TTDBookmarkWidget::addBookmarkFromDialog()
return;
}

uint64_t viewAddress = 0;
QString viewAddrStr = dialog.getViewAddress();
if (!viewAddrStr.isEmpty())
{
QString cleanText = viewAddrStr.trimmed();
if (cleanText.startsWith("0x") || cleanText.startsWith("0X"))
cleanText = cleanText.mid(2);
bool ok;
viewAddress = cleanText.toULongLong(&ok, 16);
if (!ok)
viewAddress = 0;
}

addBookmark(TTDPosition(sequence, step), dialog.getNote().toStdString(), viewAddress);
addBookmark(TTDPosition(sequence, step), dialog.getNote().toStdString(),
ParseHexAddress(dialog.getViewAddress()));
}

void TTDBookmarkWidget::addBookmarkFromCurrentPosition()
Expand All @@ -395,10 +539,10 @@ void TTDBookmarkWidget::addBookmarkFromCurrentPosition()
viewAddress = frame->getCurrentOffset();

// Show dialog pre-filled with current position
QString posStr = QString("%1:%2").arg(currentPos.sequence, 0, 16).arg(currentPos.step, 0, 16);
QString posStr = FormatTTDPosition(currentPos);
QString viewAddrStr = viewAddress != 0 ? QString("0x%1").arg(viewAddress, 0, 16) : "";

TTDBookmarkEditDialog dialog(this, posStr, "", viewAddrStr);
TTDBookmarkEditDialog dialog(this, posStr, "", viewAddrStr, m_controller);
if (dialog.exec() == QDialog::Accepted)
{
// Re-parse in case user edited the position
Expand All @@ -410,21 +554,8 @@ void TTDBookmarkWidget::addBookmarkFromCurrentPosition()
uint64_t seq = parts[0].toULongLong(&ok1, 16);
uint64_t stp = parts[1].toULongLong(&ok2, 16);
if (ok1 && ok2)
{
uint64_t addr = 0;
QString addrStr = dialog.getViewAddress();
if (!addrStr.isEmpty())
{
QString clean = addrStr.trimmed();
if (clean.startsWith("0x") || clean.startsWith("0X"))
clean = clean.mid(2);
bool ok;
addr = clean.toULongLong(&ok, 16);
if (!ok)
addr = 0;
}
addBookmark(TTDPosition(seq, stp), dialog.getNote().toStdString(), addr);
}
addBookmark(TTDPosition(seq, stp), dialog.getNote().toStdString(),
ParseHexAddress(dialog.getViewAddress()));
}
}
}
Expand All @@ -445,10 +576,10 @@ void TTDBookmarkWidget::editSelectedBookmark()
return;

const auto& bookmark = m_bookmarks[bookmarkIndex];
QString posStr = QString("%1:%2").arg(bookmark.position.sequence, 0, 16).arg(bookmark.position.step, 0, 16);
QString posStr = FormatTTDPosition(bookmark.position);
QString viewAddrStr = bookmark.viewAddress != 0 ? QString("0x%1").arg(bookmark.viewAddress, 0, 16) : "";

TTDBookmarkEditDialog dialog(this, posStr, QString::fromStdString(bookmark.note), viewAddrStr);
TTDBookmarkEditDialog dialog(this, posStr, QString::fromStdString(bookmark.note), viewAddrStr, m_controller);
if (dialog.exec() != QDialog::Accepted)
return;

Expand All @@ -463,20 +594,8 @@ void TTDBookmarkWidget::editSelectedBookmark()
if (!ok1 || !ok2)
return;

uint64_t addr = 0;
QString addrStr = dialog.getViewAddress();
if (!addrStr.isEmpty())
{
QString clean = addrStr.trimmed();
if (clean.startsWith("0x") || clean.startsWith("0X"))
clean = clean.mid(2);
bool ok;
addr = clean.toULongLong(&ok, 16);
if (!ok)
addr = 0;
}

updateBookmark(bookmarkIndex, TTDPosition(seq, stp), dialog.getNote().toStdString(), addr);
updateBookmark(bookmarkIndex, TTDPosition(seq, stp), dialog.getNote().toStdString(),
ParseHexAddress(dialog.getViewAddress()));
}

void TTDBookmarkWidget::removeSelectedBookmark()
Expand Down
9 changes: 8 additions & 1 deletion ui/ttdbookmarkwidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,23 @@ class TTDBookmarkEditDialog : public QDialog
Q_OBJECT

public:
// If a controller is provided, an empty note is auto-filled with a description of the view address
TTDBookmarkEditDialog(QWidget* parent, const QString& position = "", const QString& note = "",
const QString& viewAddress = "");
const QString& viewAddress = "", DbgRef<DebuggerController> controller = nullptr);
QString getPosition() const;
QString getNote() const;
QString getViewAddress() const;

private:
DbgRef<DebuggerController> m_controller;
QLineEdit* m_positionEdit;
QLineEdit* m_noteEdit;
QLineEdit* m_viewAddressEdit;
// Once the user types into the note field, we stop overwriting it
bool m_noteEdited;

QString getDefaultNote();
void updateAutoNote();
};


Expand Down
2 changes: 1 addition & 1 deletion ui/ttdcallswidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ void TTDCallsQueryWidget::setupUIActions()
defaultNote = funcItem->text();

@fuzyll fuzyll Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re: Earlier comment about auto-fill, it looks like this will prevent that. Again, if that's desired, awesome. Just stuck out as a difference.


TTDBookmarkEditDialog dialog(this,
posStr, defaultNote, viewAddress != 0 ? QString("0x%1").arg(viewAddress, 0, 16) : "");
posStr, defaultNote, viewAddress != 0 ? QString("0x%1").arg(viewAddress, 0, 16) : "", m_controller);
if (dialog.exec() == QDialog::Accepted)
{
QString editedPosStr = dialog.getPosition();
Expand Down
2 changes: 1 addition & 1 deletion ui/ttdeventswidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ void TTDEventsQueryWidget::setupUIActions()
return;
QString posStr = posItem->text();

TTDBookmarkEditDialog dialog(this, posStr, "", "");
TTDBookmarkEditDialog dialog(this, posStr, "", "", m_controller);

@fuzyll fuzyll Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also ran an LLM review on this PR as I had some questions on how certain things worked in the debugger after finishing my review. When I did, it said there is a problem here. I asked it for more information, and it said:

Exception bookmarks omit the available program counter. The dialog receives an empty view address even though exception rows contain a PC, so these bookmarks neither autofill the instruction nor retain the relevant navigation address.

The bookmark dialog can only generate instruction text when it receives a View Address:
uint64_t address = ParseHexAddress(m_viewAddressEdit->text());
if (!data || (address == 0))
return "";
In TTDEventsQueryWidget, exception rows already display the exception’s program counter. For example, ui/ttdeventswidget.cpp:731 populates the Program Counter column from event.exception->programCounter.
However, the bookmark action always constructs the dialog with an empty view address:
TTDBookmarkEditDialog dialog(this, posStr, "", "", m_controller);
That third empty string means:

  1. The dialog cannot disassemble the instruction at the exception PC.
  2. The generated note remains empty.
  3. The saved bookmark’s viewAddress remains zero.
  4. Navigating to that bookmark later changes the TTD position but cannot return the UI to the exception instruction.
    For example, an exception at position 123:4 with PC 0x140001234 creates a bookmark for 123:4, but discards 0x140001234.

Looking over this, I think its logic holds?

if (dialog.exec() == QDialog::Accepted)
{
QString editedPosStr = dialog.getPosition();
Expand Down
2 changes: 1 addition & 1 deletion ui/ttdmemorywidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ void TTDMemoryQueryWidget::setupUIActions()
}

TTDBookmarkEditDialog dialog(this,
posStr, "", viewAddress != 0 ? QString("0x%1").arg(viewAddress, 0, 16) : "");
posStr, "", viewAddress != 0 ? QString("0x%1").arg(viewAddress, 0, 16) : "", m_controller);
if (dialog.exec() == QDialog::Accepted)
{
QString editedPosStr = dialog.getPosition();
Expand Down
2 changes: 1 addition & 1 deletion ui/ui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ void GlobalDebuggerUI::SetupMenu(UIContext* context)
QString viewAddrStr = viewAddress != 0 ? QString("0x%1").arg(viewAddress, 0, 16) : "";

QWidget* parent = ctxt.context->mainWindow();
TTDBookmarkEditDialog dialog(parent, posStr, "", viewAddrStr);
TTDBookmarkEditDialog dialog(parent, posStr, "", viewAddrStr, controller);
if (dialog.exec() == QDialog::Accepted)
{
QString editedPosStr = dialog.getPosition();
Expand Down