Show which bookmarks have breakpoints (#859)

* Bookmarks now show symbol name alongside address

* Add icon beside bookmarks to show whether they are a breakpoint, the current instruction, or both

* Add keybindings "B" and "Shift+B" to toggle breakpoint or add conditional breakpoint to currently selected bookmark

include/edb.h

@@ -98,6 +98,7 @@ EDB_EXPORT void set_breakpoint_condition(address_t address, const QString &condi
 EDB_EXPORT void toggle_breakpoint(address_t address);
 
 EDB_EXPORT address_t current_data_view_address();
+EDB_EXPORT address_t instruction_pointer_address();
 
 // change what the various views show
 EDB_EXPORT bool dump_data_range(address_t address, address_t end_address, bool new_tab);

plugins/Bookmarks/BookmarkWidget.cpp

@@ -20,6 +20,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "BookmarksModel.h"
 #include "Expression.h"
 #include "edb.h"
+#include "IBreakpoint.h"
 #include <QInputDialog>
 #include <QMenu>
 #include <QMessageBox>
@@ -41,9 +42,13 @@ BookmarkWidget::BookmarkWidget(QWidget *parent, Qt::WindowFlags f)
 	ui.tableView->setModel(model_);
 
 	connect(edb::v1::debugger_ui, SIGNAL(detachEvent()), model_, SLOT(clearBookmarks()));
+	connect(edb::v1::disassembly_widget(), SIGNAL(signalUpdated()), model_, SLOT(updateList()));
 	connect(ui.buttonAdd, &QPushButton::clicked, this, &BookmarkWidget::buttonAddClicked);
 	connect(ui.buttonDel, &QPushButton::clicked, this, &BookmarkWidget::buttonDelClicked);
 	connect(ui.buttonClear, &QPushButton::clicked, this, &BookmarkWidget::buttonClearClicked);
+
+	toggleBreakpointAction_      = createAction(tr("&Toggle Breakpoint"), QKeySequence(tr("B")), &BookmarkWidget::toggleBreakpoint);
+	conditionalBreakpointAction_ = createAction(tr("Add &Conditional Breakpoint"), QKeySequence(tr("Shift+B")), &BookmarkWidget::addConditionalBreakpoint);
 }
 
 /**
@@ -123,6 +128,44 @@ void BookmarkWidget::buttonClearClicked() {
 	model_->clearBookmarks();
 }
 
+/**
+ * @brief BookmarkWidget::toggleBreakpoint
+ */
+void BookmarkWidget::toggleBreakpoint() {
+
+	const QItemSelectionModel *const selModel = ui.tableView->selectionModel();
+	const QModelIndexList selections          = selModel->selectedRows();
+
+	for (const auto index : selections) {
+		auto item = static_cast<BookmarksModel::Bookmark *>(index.internalPointer());
+		edb::v1::toggle_breakpoint(item->address);
+	}
+}
+
+/**
+ * @brief BookmarkWidget::addConditionalBreakpoint
+ */
+void BookmarkWidget::addConditionalBreakpoint() {
+
+	const QItemSelectionModel *const selModel = ui.tableView->selectionModel();
+	const QModelIndexList selections          = selModel->selectedRows();
+
+	if (selections.size() == 1) {
+		bool ok;
+		const QString condition = QInputDialog::getText(this, tr("Set Breakpoint Condition"), tr("Expression:"), QLineEdit::Normal, QString(), &ok);
+		auto item = static_cast<BookmarksModel::Bookmark *>(selections[0].internalPointer());
+
+		if (ok) {
+			if (std::shared_ptr<IBreakpoint> bp = edb::v1::create_breakpoint(item->address)) {
+				if (!condition.isEmpty()) {
+					bp->condition = condition;
+				}
+			}
+		}
+	}
+}
+
+
 /**
  * @brief BookmarkWidget::addAddress
  * @param address
@@ -185,6 +228,8 @@ void BookmarkWidget::on_tableView_customContextMenuRequested(const QPoint &pos)
 	menu.addSeparator();
 	QAction *const actionComment = menu.addAction(tr("&Set Comment"));
 	QAction *const actionType    = menu.addAction(tr("Set &Type"));
+	menu.addAction(toggleBreakpointAction_);
+	menu.addAction(conditionalBreakpointAction_);
 	QAction *const chosen        = menu.exec(ui.tableView->mapToGlobal(pos));
 
 	if (chosen == actionAdd) {
@@ -230,6 +275,10 @@ void BookmarkWidget::on_tableView_customContextMenuRequested(const QPoint &pos)
 				}
 			}
 		}
+	} else if (chosen == toggleBreakpointAction_) {
+		toggleBreakpoint();
+	} else if (chosen == conditionalBreakpointAction_) {
+		addConditionalBreakpoint();
 	}
 }
 
@@ -242,4 +291,16 @@ QList<BookmarksModel::Bookmark> BookmarkWidget::entries() const {
 	return bookmarks.toList();
 }
 
+// This is copied from Debugger::createAction, so really there should either be a class that implements
+// this that both inherit from, or this should be a generic function that takes the QObject* to act upon
+// as a parameter.
+template <class F>
+QAction *BookmarkWidget::createAction(const QString &text, const QKeySequence &keySequence, F func) {
+	auto action = new QAction(text, this);
+	action->setShortcut(keySequence);
+	addAction(action);
+	connect(action, &QAction::triggered, this, func);
+	return action;
+}
+
 }

plugins/Bookmarks/BookmarkWidget.h

@@ -50,10 +50,18 @@ public Q_SLOTS:
 	void buttonAddClicked();
 	void buttonDelClicked();
 	void buttonClearClicked();
+	void toggleBreakpoint();
+	void addConditionalBreakpoint();
+
+private:
+	template <class F>
+	QAction *createAction(const QString &text, const QKeySequence &keySequence, F func);
 
 private:
 	Ui::BookmarkWidget ui;
 	BookmarksModel *model_ = nullptr;
+	QAction *toggleBreakpointAction_;
+	QAction *conditionalBreakpointAction_;
 };
 
 }

plugins/Bookmarks/BookmarksModel.cpp

@@ -26,7 +26,10 @@ namespace BookmarksPlugin {
  * @param parent
  */
 BookmarksModel::BookmarksModel(QObject *parent)
-	: QAbstractItemModel(parent) {
+	: QAbstractItemModel(parent),
+	  breakpointIcon_(QLatin1String(":/debugger/images/breakpoint.svg")),
+	  currentIcon_(QLatin1String(":/debugger/images/arrow-right.svg")),
+	  currentBpIcon_(QLatin1String(":/debugger/images/arrow-right-red.svg")) {
 }
 
 /**
@@ -68,8 +71,17 @@ QVariant BookmarksModel::data(const QModelIndex &index, int role) const {
 
 	if (role == Qt::DisplayRole) {
 		switch (index.column()) {
-		case 0:
-			return edb::v1::format_pointer(bookmark.address);
+		case 0: {
+			// Return the address with symbol name, if there is one
+			const QString symname = edb::v1::find_function_symbol(bookmark.address);
+			const QString address = edb::v1::format_pointer(bookmark.address);
+
+			if (!symname.isEmpty()) {
+				return tr("%1 <%2>").arg(address, symname);
+			}
+
+			return address;
+		}
 		case 1:
 			switch (bookmark.type) {
 			case Bookmark::Code:
@@ -87,6 +99,36 @@ QVariant BookmarksModel::data(const QModelIndex &index, int role) const {
 		}
 	}
 
+	if (role == Qt::DecorationRole) {
+		switch (index.column()) {
+		case 0: {
+			// Puts the correct icon (BP, current BP, current instruction) next to the address
+
+			// TODO: This is mostly copied from QDisassemblyView::drawSidebarElements, and both
+			// should really be factored out into a common location (although this uses icons
+			// and the other uses SVG renderers and painting).
+			const bool is_eip = (bookmark.address == edb::v1::instruction_pointer_address());
+			const bool has_breakpoint = (edb::v1::find_breakpoint(bookmark.address) != nullptr);
+
+			const QIcon *icon = nullptr;
+			if (is_eip) {
+				icon = has_breakpoint ? &currentBpIcon_ : &currentIcon_;
+			} else if (has_breakpoint) {
+				icon = &breakpointIcon_;
+			}
+
+			if (icon) {
+				return *icon;
+			}
+
+			// This acts as a dummy icon so even addresses with no icon will be aligned properly
+			return QColor(0, 0, 0, 0);
+		}
+		default:
+			return QVariant();
+		}
+	}
+
 	return QVariant();
 }
 
@@ -200,6 +242,20 @@ void BookmarksModel::setType(const QModelIndex &index, const QString &type) {
 	Q_EMIT dataChanged(index, index);
 }
 
+/**
+ * @brief BookmarksModel::updateList
+ */
+void BookmarksModel::updateList() {
+	// Every time the disassembly view changes, all the bookmark data is invalidated.
+	// This is not super expensive (unless you have a million bookmarks) but is not
+	// optimal either. Ideally we could factor out eip updates with the signalUpdated
+	// signal, and breakpoint updates with the toggleBreakPoint signal, but the latter
+	// can't use the SIGNAL/SLOT macros which means Debugger.h would need to be moved
+	// into the include directory.
+	beginResetModel();
+	endResetModel();
+}
+
 /**
  * @brief BookmarksModel::deleteBookmark
  * @param index

plugins/Bookmarks/BookmarksModel.h

@@ -22,6 +22,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "Types.h"
 #include <QAbstractItemModel>
 #include <QVector>
+#include <QIcon>
 
 namespace BookmarksPlugin {
 
@@ -88,12 +89,16 @@ public Q_SLOTS:
 	void deleteBookmark(const QModelIndex &index);
 	void setComment(const QModelIndex &index, const QString &comment);
 	void setType(const QModelIndex &index, const QString &type);
+	void updateList();
 
 public:
 	[[nodiscard]] const QVector<Bookmark> &bookmarks() const { return bookmarks_; }
 
 private:
 	QVector<Bookmark> bookmarks_;
+	QIcon breakpointIcon_;
+	QIcon currentIcon_;
+	QIcon currentBpIcon_;
 };
 
 }

src/edb.cpp

@@ -1303,6 +1303,20 @@ address_t current_data_view_address() {
 	return qobject_cast<QHexView *>(ui()->tabWidget_->currentWidget())->firstVisibleAddress();
 }
 
+//------------------------------------------------------------------------------
+// Name: instruction_pointer_address
+// Desc: Returns the address pointed to by the instruction pointer.
+//------------------------------------------------------------------------------
+address_t instruction_pointer_address() {
+	if (IProcess *process = debugger_core->process()) {
+		State state;
+		process->currentThread()->getState(&state);
+		return state.instructionPointer();
+	}
+
+	return address_t{};
+}
+
 //------------------------------------------------------------------------------
 // Name: set_status
 // Desc: