Breakpoint editor dialog (#1975)

* Add breakpoint dialog for editing breakpoint properties
* Allow editing breakpoint using context menu from breakpointWidget and disassembly menu.

docs/source/shortcuts.rst

@@ -104,16 +104,18 @@ Graph view shortcuts
 Debug shortcuts
 ---------------
 
-+-----------------+----------------+
++-----------------+------------------------------------------+
-| Shortcut        | Function       |
+| Shortcut        | Function                                 |
-+=================+================+
++=================+==========================================+
-| F9              | Start debug    |
+| F9              | Start debug                              |
-+-----------------+----------------+
++-----------------+------------------------------------------+
-| F7              | Step into      |
+| F7              | Step into                                |
-+-----------------+----------------+
++-----------------+------------------------------------------+
-| F8              | Step over      |
+| F8              | Step over                                |
-+-----------------+----------------+
++-----------------+------------------------------------------+
-| F5              | Continue       |
+| F5              | Continue                                 |
-+-----------------+----------------+
++-----------------+------------------------------------------+
-| F2/(Ctrl/Cmd)+B | Add breakpoint |
+| F2/(Ctrl/Cmd)+B | Add or Remove breakpoint                 |
-+-----------------+----------------+
++-----------------+------------------------------------------+
+| (Ctrl/Cmd)+F2   | Edit or open Advanced breakpoint dialog  |
++-----------------+------------------------------------------+

src/common/Helpers.cpp

@@ -13,6 +13,7 @@
 #include <QAbstractButton>
 #include <QDockWidget>
 #include <QMenu>
+#include <QComboBox>
 
 static QAbstractItemView::ScrollMode scrollMode()
 {
@@ -257,5 +258,16 @@ qreal devicePixelRatio(const QPaintDevice *p)
 #endif
 }
 
+void selectIndexByData(QComboBox *widget, QVariant data, int defaultIndex)
+{
+    for (int i = 0; i < widget->count(); i++) {
+        if (widget->itemData(i) == data) {
+            widget->setCurrentIndex(i);
+            return;
+        }
+    }
+    widget->setCurrentIndex(defaultIndex);
+}
+
 
 } // end namespace

src/common/Helpers.h

@@ -19,6 +19,7 @@ class QTreeView;
 class QAction;
 class QMenu;
 class QPaintDevice;
+class QComboBox;
 
 namespace qhelpers {
 QString formatBytecount(const uint64_t bytecount);
@@ -55,6 +56,13 @@ void setThemeIcons(QList<QPair<void*, QString>> supportedIconsNames, std::functi
 
 void prependQAction(QAction *action, QMenu *menu);
 qreal devicePixelRatio(const QPaintDevice *p);
+/**
+ * @brief Select comboBox item by value in Qt::UserRole.
+ * @param comboBox
+ * @param data - value to search in combobox item data
+ * @param defaultIndex - item to select in case no match
+ */
+void selectIndexByData(QComboBox *comboBox, QVariant data, int defaultIndex = -1);
 
 } // qhelpers
 

src/core/Cutter.cpp

@@ -104,6 +104,13 @@ namespace RJsonKey {
 
 #undef R_JSON_KEY
 
+static void updateOwnedCharPtr(char *&variable, const QString &newValue)
+{
+    auto data = newValue.toUtf8();
+    R_FREE(variable)
+    variable = strdup(data.data());
+}
+
 RCoreLocked::RCoreLocked(CutterCore *core)
     : core(core)
 {
@@ -457,7 +464,7 @@ QStringList CutterCore::autocomplete(const QString &cmd, RLinePromptType promptT
 
     QStringList r;
     r.reserve(r_pvector_len(&completion.args));
-    for (size_t i = 0; i< r_pvector_len(&completion.args); i++) {
+    for (size_t i = 0; i < r_pvector_len(&completion.args); i++) {
         r.push_back(QString::fromUtf8(reinterpret_cast<const char *>(r_pvector_at(&completion.args, i))));
     }
 
@@ -1468,7 +1475,7 @@ void CutterCore::attachDebug(int pid)
 
 void CutterCore::suspendDebug()
 {
-    debugTask->breakTask(); 
+    debugTask->breakTask();
 }
 
 void CutterCore::stopDebug()
@@ -1749,6 +1756,73 @@ void CutterCore::addBreakpoint(QString addr)
     emit breakpointsChanged();
 }
 
+void CutterCore::addBreakpoint(const BreakpointDescription &config)
+{
+    CORE_LOCK();
+    RBreakpointItem *breakpoint = nullptr;
+    int watchpoint_prot = 0;
+    if (config.hw) {
+        watchpoint_prot = config.permission & ~(R_BP_PROT_EXEC);
+    }
+
+    auto address = config.addr;
+    char *module = nullptr;
+    QByteArray moduleNameData;
+    if (config.type == BreakpointDescription::Named) {
+        address = Core()->math(config.positionExpression);
+    } else if (config.type == BreakpointDescription::Module) {
+        address = 0;
+        moduleNameData = config.positionExpression.toUtf8();
+        module = moduleNameData.data();
+    }
+    breakpoint = r_debug_bp_add(core->dbg, address, (config.hw && watchpoint_prot == 0),
+                                watchpoint_prot, watchpoint_prot,
+                                module, config.moduleDelta);
+    if (config.type == BreakpointDescription::Named) {
+        updateOwnedCharPtr(breakpoint->expr, config.positionExpression);
+    }
+
+    if (config.hw) {
+        breakpoint->size = config.size;
+    }
+    if (config.type == BreakpointDescription::Named) {
+        updateOwnedCharPtr(breakpoint->name, config.positionExpression);
+    }
+
+    if (!breakpoint) {
+        QMessageBox::critical(nullptr, tr("Breakpoint error"), tr("Failed to create breakpoint"));
+        return;
+    }
+    int index = std::find(core->dbg->bp->bps_idx,
+                          core->dbg->bp->bps_idx + core->dbg->bp->bps_idx_count,
+                          breakpoint) - core->dbg->bp->bps_idx;
+
+    breakpoint->enabled = config.enabled;
+    if (config.trace) {
+        setBreakpointTrace(index, config.trace);
+    }
+    if (!config.condition.isEmpty()) {
+        updateOwnedCharPtr(breakpoint->cond, config.condition);
+    }
+    if (!config.command.isEmpty()) {
+        updateOwnedCharPtr(breakpoint->data, config.command);
+    }
+    emit instructionChanged(breakpoint->addr);
+    emit breakpointsChanged();
+}
+
+void CutterCore::updateBreakpoint(int index, const BreakpointDescription &config)
+{
+    CORE_LOCK();
+    if (auto bp = r_bp_get_index(core->dbg->bp, index)) {
+        r_bp_del(core->dbg->bp, bp->addr);
+    }
+    // Delete by index currently buggy,
+    // required for breakpoints with non address based position
+    //r_bp_del_index(core->dbg->bp, index);
+    addBreakpoint(config);
+}
+
 void CutterCore::delBreakpoint(RVA addr)
 {
     cmd("db- " + RAddressString(addr));
@@ -1785,24 +1859,52 @@ void CutterCore::setBreakpointTrace(int index, bool enabled)
     }
 }
 
+static BreakpointDescription breakpointDescriptionFromR2(int index, r_bp_item_t *bpi)
+{
+    BreakpointDescription bp;
+    bp.addr = bpi->addr;
+    bp.index = index;
+    bp.size = bpi->size;
+    if (bpi->expr) {
+        bp.positionExpression = bpi->expr;
+        bp.type = BreakpointDescription::Named;
+    }
+    bp.name = bpi->name;
+    bp.permission = bpi->perm;
+    bp.command = bpi->data;
+    bp.condition = bpi->cond;
+    bp.hw = bpi->hw;
+    bp.trace = bpi->trace;
+    bp.enabled = bpi->enabled;
+    return bp;
+}
+
+int CutterCore::breakpointIndexAt(RVA addr)
+{
+    CORE_LOCK();
+    return r_bp_get_index_at(core->dbg->bp, addr);
+}
+
+BreakpointDescription CutterCore::getBreakpointAt(RVA addr)
+{
+    CORE_LOCK();
+    int index = breakpointIndexAt(addr);
+    auto bp = r_bp_get_index(core->dbg->bp, index);
+    if (bp) {
+        return breakpointDescriptionFromR2(index, bp);
+    }
+    return BreakpointDescription();
+}
+
 QList<BreakpointDescription> CutterCore::getBreakpoints()
 {
+    CORE_LOCK();
     QList<BreakpointDescription> ret;
-    QJsonArray breakpointArray = cmdj("dbj").array();
+    //TODO: use higher level API, don't touch r2 bps_idx directly
-
+    for (int i = 0; i < core->dbg->bp->bps_idx_count; i++) {
-    for (const QJsonValue &value : breakpointArray) {
+        if (auto bpi = core->dbg->bp->bps_idx[i]) {
-        QJsonObject bpObject = value.toObject();
+            ret.push_back(breakpointDescriptionFromR2(i, bpi));
-
+        }
-        BreakpointDescription bp;
-
-        bp.addr = bpObject[RJsonKey::addr].toVariant().toULongLong();
-        bp.size = bpObject[RJsonKey::size].toInt();
-        bp.permission = bpObject[RJsonKey::prot].toString();
-        bp.hw = bpObject[RJsonKey::hw].toBool();
-        bp.trace = bpObject[RJsonKey::trace].toBool();
-        bp.enabled = bpObject[RJsonKey::enabled].toBool();
-
-        ret << bp;
     }
 
     return ret;

src/core/Cutter.h

@@ -319,7 +319,10 @@ public:
     void stepDebug();
     void stepOverDebug();
     void stepOutDebug();
+
     void addBreakpoint(QString addr);
+    void addBreakpoint(const BreakpointDescription &config);
+    void updateBreakpoint(int index, const BreakpointDescription &config);
     void toggleBreakpoint(RVA addr);
     void toggleBreakpoint(QString addr);
     void delBreakpoint(RVA addr);
@@ -332,6 +335,8 @@ public:
      * @param enabled - true if tracing should be enabled
      */
     void setBreakpointTrace(int index, bool enabled);
+    int breakpointIndexAt(RVA addr);
+    BreakpointDescription getBreakpointAt(RVA addr);
 
     bool isBreakpoint(const QList<RVA> &breakpoints, RVA addr);
     QList<RVA> getBreakpointsAddresses();

src/core/CutterDescriptions.h

@@ -276,12 +276,25 @@ struct MemoryMapDescription {
 };
 
 struct BreakpointDescription {
-    RVA addr;
+    enum PositionType {
-    int size;
+        Address,
-    QString permission;
+        Named,
-    bool hw;
+        Module,
-    bool trace;
+    };
-    bool enabled;
+
+    RVA addr = 0;
+    int64_t moduleDelta = 0;
+    int index = -1;
+    PositionType type = Address;
+    int size = 0;
+    int permission = 0;
+    QString positionExpression;
+    QString name;
+    QString command;
+    QString condition;
+    bool hw = false;
+    bool trace = false;
+    bool enabled = true;
 };
 
 struct ProcessDescription {
@@ -338,6 +351,7 @@ Q_DECLARE_METATYPE(SectionDescription)
 Q_DECLARE_METATYPE(SegmentDescription)
 Q_DECLARE_METATYPE(MemoryMapDescription)
 Q_DECLARE_METATYPE(BreakpointDescription)
+Q_DECLARE_METATYPE(BreakpointDescription::PositionType)
 Q_DECLARE_METATYPE(ProcessDescription)
 Q_DECLARE_METATYPE(RegisterRefDescription)
 Q_DECLARE_METATYPE(VariableDescription)

src/dialogs/BreakpointsDialog.cpp

@@ -1,47 +1,213 @@
 #include "BreakpointsDialog.h"
 #include "ui_BreakpointsDialog.h"
+#include "Cutter.h"
+#include "Helpers.h"
 
-BreakpointsDialog::BreakpointsDialog(QWidget *parent) :
+#include <QPushButton>
+#include <QCompleter>
+#include <QCheckBox>
+
+BreakpointsDialog::BreakpointsDialog(bool editMode, QWidget *parent) :
     QDialog(parent),
-    ui(new Ui::BreakpointsDialog)
+    ui(new Ui::BreakpointsDialog),
+    editMode(editMode)
 {
     ui->setupUi(this);
     setWindowFlags(windowFlags() & (~Qt::WindowContextHelpButtonHint));
 
-    // Event filter for capturing Ctrl/Cmd+Return
+    connect(ui->breakpointPosition, &QLineEdit::textChanged, this, &BreakpointsDialog::refreshOkButton);
-    ui->textEdit->installEventFilter(this);
+    refreshOkButton();
+
+    if (editMode) {
+        setWindowTitle(tr("Edit breakpoint"));
+    } else {
+        setWindowTitle(tr("New breakpoint"));
+    }
+
+
+    struct {
+        QString label;
+        QString tooltip;
+        BreakpointDescription::PositionType type;
+    } positionTypes[] = {
+        {tr("Address"), tr("Address or expression calculated when creating breakpoint"), BreakpointDescription::Address},
+        {tr("Named"), tr("Expression - stored as expression"), BreakpointDescription::Named},
+        {tr("Module offset"), tr("Offset relative to module"), BreakpointDescription::Module},
+    };
+    int index = 0;
+    for (auto &item : positionTypes) {
+        ui->positionType->addItem(item.label, item.type);
+        ui->positionType->setItemData(index, item.tooltip, Qt::ToolTipRole);
+        index++;
+    }
+
+    connect(ui->positionType, static_cast<void (QComboBox::*)(int)>(&QComboBox::currentIndexChanged),
+            this, &BreakpointsDialog::onTypeChanged);
+    onTypeChanged();
+
+    auto modules = Core()->getMemoryMap();
+    QSet<QString> moduleNames;
+    for (const auto &module : modules) {
+        moduleNames.insert(module.fileName);
+    }
+    for (const auto& module : moduleNames) {
+        ui->moduleName->addItem(module);
+    }
+    ui->moduleName->setCurrentText("");
+    // Suggest completion when user tries to enter file name not only full path
+    ui->moduleName->completer()->setFilterMode(Qt::MatchContains);
+
+    ui->breakpointCondition->setCompleter(nullptr); // Don't use examples for completing
+    configureCheckboxRestrictions();
+}
+
+BreakpointsDialog::BreakpointsDialog(const BreakpointDescription &breakpoint, QWidget *parent)
+    : BreakpointsDialog(true, parent)
+{
+    switch (breakpoint.type) {
+    case BreakpointDescription::Address:
+        ui->breakpointPosition->setText(RAddressString(breakpoint.addr));
+        break;
+    case BreakpointDescription::Named:
+        ui->breakpointPosition->setText(breakpoint.positionExpression);
+        break;
+    case BreakpointDescription::Module:
+        ui->breakpointPosition->setText(QString::number(breakpoint.moduleDelta));
+        ui->moduleName->setCurrentText(breakpoint.positionExpression);
+        break;
+    }
+    for (int i = 0; i < ui->positionType->count(); i++) {
+        if (ui->positionType->itemData(i) == breakpoint.type) {
+            ui->positionType->setCurrentIndex(i);
+        }
+    }
+    ui->breakpointCommand->setPlainText(breakpoint.command);
+    ui->breakpointCondition->setEditText(breakpoint.condition);
+    if (breakpoint.hw) {
+        ui->radioHardware->setChecked(true);
+        ui->hwRead->setChecked(breakpoint.permission & R_BP_PROT_READ);
+        ui->hwWrite->setChecked(breakpoint.permission & R_BP_PROT_WRITE);
+        ui->hwExecute->setChecked(breakpoint.permission & R_BP_PROT_EXEC);
+        ui->breakpointSize->setCurrentText(QString::number(breakpoint.size));
+    } else {
+        ui->radioSoftware->setChecked(true);
+    }
+    ui->checkTrace->setChecked(breakpoint.trace);
+    ui->checkEnabled->setChecked(breakpoint.enabled);
+    refreshOkButton();
+}
+
+BreakpointsDialog::BreakpointsDialog(RVA address, QWidget *parent)
+    : BreakpointsDialog(false, parent)
+{
+    if (address != RVA_INVALID) {
+        ui->breakpointPosition->setText(RAddressString(address));
+    }
+    refreshOkButton();
 }
 
 BreakpointsDialog::~BreakpointsDialog() {}
 
-void BreakpointsDialog::on_buttonBox_accepted()
+BreakpointDescription BreakpointsDialog::getDescription()
 {
-}
+    BreakpointDescription breakpoint;
-
+    auto positionType = ui->positionType->currentData().value<BreakpointDescription::PositionType>();
-void BreakpointsDialog::on_buttonBox_rejected()
+    switch (positionType) {
-{
+    case BreakpointDescription::Address:
-    close();
+        breakpoint.addr = Core()->math(ui->breakpointPosition->text());
-}
+        break;
-
+    case BreakpointDescription::Named:
-QString BreakpointsDialog::getBreakpoints()
+        breakpoint.positionExpression = ui->breakpointPosition->text().trimmed();
-{
+        break;
-    QString ret = ui->textEdit->document()->toPlainText();
+    case BreakpointDescription::Module:
-    return ret;
+        breakpoint.moduleDelta = static_cast<int64_t>(Core()->math(ui->breakpointPosition->text()));
-}
+        breakpoint.positionExpression = ui->moduleName->currentText().trimmed();
-
+        break;
-bool BreakpointsDialog::eventFilter(QObject *obj, QEvent *event)
-{
-    Q_UNUSED(obj);
-    if (event -> type() == QEvent::KeyPress) {
-        QKeyEvent *keyEvent = static_cast <QKeyEvent *> (event);
-
-        // Confirm comment by pressing Ctrl/Cmd+Return
-        if ((keyEvent -> modifiers() & Qt::ControlModifier) &&
-                ((keyEvent -> key() == Qt::Key_Enter) || (keyEvent -> key() == Qt::Key_Return))) {
-            this->accept();
-            return true;
-        }
     }
+    breakpoint.type = positionType;
 
-    return false;
+    breakpoint.size = Core()->num(ui->breakpointSize->currentText());
+    breakpoint.condition = ui->breakpointCondition->currentText().trimmed();
+    breakpoint.command = ui->breakpointCommand->toPlainText().trimmed();
+    if (ui->radioHardware->isChecked()) {
+        breakpoint.hw = true;
+        breakpoint.permission = getHwPermissions();
+    } else {
+        breakpoint.hw = false;
+    }
+    breakpoint.trace = ui->checkTrace->isChecked();
+    breakpoint.enabled = ui->checkEnabled->isChecked();
+    return breakpoint;
+}
+
+void BreakpointsDialog::createNewBreakpoint(RVA address, QWidget *parent)
+{
+    BreakpointsDialog editDialog(address, parent);
+    if (editDialog.exec() == QDialog::Accepted) {
+        Core()->addBreakpoint(editDialog.getDescription());
+    }
+}
+
+void BreakpointsDialog::editBreakpoint(const BreakpointDescription &breakpoint, QWidget *parent)
+{
+    BreakpointsDialog editDialog(breakpoint, parent);
+    if (editDialog.exec() == QDialog::Accepted) {
+        Core()->updateBreakpoint(breakpoint.index, editDialog.getDescription());
+    }
+}
+
+void BreakpointsDialog::refreshOkButton()
+{
+    auto button = ui->buttonBox->button(QDialogButtonBox::StandardButton::Ok);
+    button->setDisabled(ui->breakpointPosition->text().isEmpty());
+}
+
+void BreakpointsDialog::onTypeChanged()
+{
+    bool moduleEnabled = ui->positionType->currentData() == QVariant(BreakpointDescription::Module);
+    ui->moduleLabel->setEnabled(moduleEnabled);
+    ui->moduleName->setEnabled(moduleEnabled);
+    ui->breakpointPosition->setPlaceholderText(ui->positionType->currentData(Qt::ToolTipRole).toString());
+}
+
+void BreakpointsDialog::configureCheckboxRestrictions()
+{
+    auto atLeastOneChecked = [this]() {
+        if (this->getHwPermissions() == 0) {
+            this->ui->hwExecute->setChecked(true);
+        }
+    };
+    auto rwRule = [this, atLeastOneChecked](bool checked) {
+        if (checked) {
+            this->ui->hwExecute->setChecked(false);
+        } else {
+            atLeastOneChecked();
+        }
+    };
+    connect(ui->hwRead, &QCheckBox::toggled, this, rwRule);
+    connect(ui->hwWrite, &QCheckBox::toggled, this, rwRule);
+    auto execRule = [this, atLeastOneChecked](bool checked) {
+        if (checked) {
+            this->ui->hwRead->setChecked(false);
+            this->ui->hwWrite->setChecked(false);
+        } else {
+            atLeastOneChecked();
+        }
+    };
+    connect(ui->hwExecute, &QCheckBox::toggled, this, execRule);
+}
+
+int BreakpointsDialog::getHwPermissions()
+{
+    int result = 0;
+    if (ui->hwRead->isChecked()) {
+        result |= R_BP_PROT_READ;
+    }
+    if (ui->hwWrite->isChecked()) {
+        result |= R_BP_PROT_WRITE;
+    }
+    if (ui->hwExecute->isChecked()) {
+        result |= R_BP_PROT_EXEC;
+    }
+    return result;
 }

src/dialogs/BreakpointsDialog.h

@@ -2,6 +2,7 @@
 
 #include <QDialog>
 #include <memory>
+#include "CutterDescriptions.h"
 
 namespace Ui {
 class BreakpointsDialog;
@@ -12,17 +13,21 @@ class BreakpointsDialog : public QDialog
     Q_OBJECT
 
 public:
-    explicit BreakpointsDialog(QWidget *parent = nullptr);
+    explicit BreakpointsDialog(bool editMode = false, QWidget *parent = nullptr);
+    BreakpointsDialog(const BreakpointDescription &editableBreakpoint, QWidget *parent = nullptr);
+    BreakpointsDialog(RVA address, QWidget *parent = nullptr);
     ~BreakpointsDialog();
 
-    QString getBreakpoints();
+    BreakpointDescription getDescription();
-
-private slots:
-    void on_buttonBox_accepted();
-    void on_buttonBox_rejected();
 
+    static void createNewBreakpoint(RVA address = RVA_INVALID, QWidget *parent = nullptr);
+    static void editBreakpoint(const BreakpointDescription& breakpoint, QWidget *parent = nullptr);
 private:
     std::unique_ptr<Ui::BreakpointsDialog> ui;
+    bool editMode = false;
 
-    bool eventFilter(QObject *obj, QEvent *event);
+    void refreshOkButton();
+    void onTypeChanged();
+    void configureCheckboxRestrictions();
+    int getHwPermissions();
 };

src/dialogs/BreakpointsDialog.ui

@@ -6,36 +6,287 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>400</width>
+    <width>610</width>
-    <height>118</height>
+    <height>437</height>
    </rect>
   </property>
   <property name="windowTitle">
-   <string>Add breakpoints</string>
+   <string>Add/Edit breakpoint</string>
   </property>
-  <layout class="QVBoxLayout" name="verticalLayout_2">
+  <layout class="QVBoxLayout" name="verticalLayout">
-   <property name="spacing">
+   <item>
-    <number>2</number>
+    <layout class="QFormLayout" name="topGroup">
-   </property>
+     <item row="0" column="0">
-   <property name="leftMargin">
+      <widget class="QLabel" name="label">
-    <number>2</number>
+       <property name="text">
-   </property>
+        <string>Position</string>
-   <property name="topMargin">
+       </property>
-    <number>5</number>
+       <property name="buddy">
-   </property>
+        <cstring>breakpointPosition</cstring>
-   <property name="rightMargin">
+       </property>
-    <number>2</number>
+      </widget>
-   </property>
+     </item>
-   <property name="bottomMargin">
+     <item row="0" column="1">
-    <number>2</number>
+      <widget class="QWidget" name="widget" native="true">
-   </property>
+       <layout class="QHBoxLayout" name="horizontalLayout_2">
+        <property name="spacing">
+         <number>0</number>
+        </property>
+        <property name="leftMargin">
+         <number>0</number>
+        </property>
+        <property name="topMargin">
+         <number>0</number>
+        </property>
+        <property name="rightMargin">
+         <number>0</number>
+        </property>
+        <property name="bottomMargin">
+         <number>0</number>
+        </property>
+        <item>
+         <widget class="QComboBox" name="positionType"/>
+        </item>
+        <item>
+         <widget class="QLineEdit" name="breakpointPosition"/>
+        </item>
+       </layout>
+      </widget>
+     </item>
+     <item row="3" column="0">
+      <widget class="QLabel" name="label_2">
+       <property name="text">
+        <string>Condition</string>
+       </property>
+       <property name="buddy">
+        <cstring>breakpointCondition</cstring>
+       </property>
+      </widget>
+     </item>
+     <item row="3" column="1">
+      <widget class="QComboBox" name="breakpointCondition">
+       <property name="sizePolicy">
+        <sizepolicy hsizetype="Expanding" vsizetype="Fixed">
+         <horstretch>0</horstretch>
+         <verstretch>0</verstretch>
+        </sizepolicy>
+       </property>
+       <property name="editable">
+        <bool>true</bool>
+       </property>
+       <property name="currentText">
+        <string/>
+       </property>
+       <property name="currentIndex">
+        <number>-1</number>
+       </property>
+       <property name="insertPolicy">
+        <enum>QComboBox::NoInsert</enum>
+       </property>
+       <property name="frame">
+        <bool>true</bool>
+       </property>
+       <item>
+        <property name="text">
+         <string>?v $.rax-0x6  # break when rax is 6</string>
+        </property>
+       </item>
+      </widget>
+     </item>
+     <item row="2" column="0">
+      <widget class="QLabel" name="moduleLabel">
+       <property name="text">
+        <string>Module</string>
+       </property>
+       <property name="buddy">
+        <cstring>moduleName</cstring>
+       </property>
+      </widget>
+     </item>
+     <item row="2" column="1">
+      <widget class="QComboBox" name="moduleName">
+       <property name="sizePolicy">
+        <sizepolicy hsizetype="Expanding" vsizetype="Fixed">
+         <horstretch>0</horstretch>
+         <verstretch>0</verstretch>
+        </sizepolicy>
+       </property>
+       <property name="editable">
+        <bool>true</bool>
+       </property>
+      </widget>
+     </item>
+    </layout>
+   </item>
    <item>
     <layout class="QHBoxLayout" name="horizontalLayout">
-     <property name="topMargin">
-      <number>0</number>
-     </property>
      <item>
-      <widget class="QPlainTextEdit" name="textEdit"/>
+      <widget class="QGroupBox" name="groupBox">
+       <property name="title">
+        <string>Type/Options</string>
+       </property>
+       <layout class="QVBoxLayout" name="verticalLayout_2">
+        <item>
+         <widget class="QCheckBox" name="checkEnabled">
+          <property name="text">
+           <string>Enabled</string>
+          </property>
+          <property name="checked">
+           <bool>true</bool>
+          </property>
+         </widget>
+        </item>
+        <item>
+         <widget class="QRadioButton" name="radioSoftware">
+          <property name="text">
+           <string>Software</string>
+          </property>
+          <property name="checked">
+           <bool>true</bool>
+          </property>
+         </widget>
+        </item>
+        <item>
+         <widget class="QRadioButton" name="radioHardware">
+          <property name="text">
+           <string>Hardware</string>
+          </property>
+         </widget>
+        </item>
+        <item>
+         <widget class="QWidget" name="hwConfigBox">
+          <property name="enabled">
+           <bool>false</bool>
+          </property>
+          <layout class="QVBoxLayout" name="verticalLayout_5">
+           <property name="spacing">
+            <number>0</number>
+           </property>
+           <property name="leftMargin">
+            <number>0</number>
+           </property>
+           <property name="topMargin">
+            <number>0</number>
+           </property>
+           <property name="rightMargin">
+            <number>0</number>
+           </property>
+           <property name="bottomMargin">
+            <number>0</number>
+           </property>
+           <item>
+            <widget class="QCheckBox" name="hwRead">
+             <property name="text">
+              <string>Read</string>
+             </property>
+            </widget>
+           </item>
+           <item>
+            <widget class="QCheckBox" name="hwWrite">
+             <property name="text">
+              <string>Write</string>
+             </property>
+            </widget>
+           </item>
+           <item>
+            <widget class="QCheckBox" name="hwExecute">
+             <property name="text">
+              <string>Execute</string>
+             </property>
+             <property name="checked">
+              <bool>true</bool>
+             </property>
+            </widget>
+           </item>
+           <item>
+            <layout class="QHBoxLayout" name="horizontalLayout_3">
+             <item>
+              <widget class="QLabel" name="label_4">
+               <property name="text">
+                <string>Size</string>
+               </property>
+               <property name="buddy">
+                <cstring>breakpointSize</cstring>
+               </property>
+              </widget>
+             </item>
+             <item>
+              <widget class="QComboBox" name="breakpointSize">
+               <item>
+                <property name="text">
+                 <string>1</string>
+                </property>
+               </item>
+               <item>
+                <property name="text">
+                 <string>2</string>
+                </property>
+               </item>
+               <item>
+                <property name="text">
+                 <string>4</string>
+                </property>
+               </item>
+               <item>
+                <property name="text">
+                 <string>8</string>
+                </property>
+               </item>
+              </widget>
+             </item>
+            </layout>
+           </item>
+          </layout>
+         </widget>
+        </item>
+        <item>
+         <spacer name="verticalSpacer">
+          <property name="orientation">
+           <enum>Qt::Vertical</enum>
+          </property>
+          <property name="sizeHint" stdset="0">
+           <size>
+            <width>20</width>
+            <height>40</height>
+           </size>
+          </property>
+         </spacer>
+        </item>
+       </layout>
+      </widget>
+     </item>
+     <item>
+      <widget class="QGroupBox" name="groupBox_2">
+       <property name="title">
+        <string>Action</string>
+       </property>
+       <layout class="QVBoxLayout" name="verticalLayout_4">
+        <item>
+         <widget class="QCheckBox" name="checkTrace">
+          <property name="text">
+           <string>Trace</string>
+          </property>
+         </widget>
+        </item>
+        <item>
+         <layout class="QFormLayout" name="formLayout_3">
+          <item row="0" column="0">
+           <widget class="QLabel" name="label_5">
+            <property name="text">
+             <string>Command</string>
+            </property>
+            <property name="buddy">
+             <cstring>breakpointCommand</cstring>
+            </property>
+           </widget>
+          </item>
+          <item row="0" column="1">
+           <widget class="QPlainTextEdit" name="breakpointCommand"/>
+          </item>
+         </layout>
+        </item>
+       </layout>
+      </widget>
      </item>
     </layout>
    </item>
@@ -51,6 +302,17 @@
    </item>
   </layout>
  </widget>
+ <tabstops>
+  <tabstop>positionType</tabstop>
+  <tabstop>breakpointPosition</tabstop>
+  <tabstop>moduleName</tabstop>
+  <tabstop>breakpointCondition</tabstop>
+  <tabstop>checkEnabled</tabstop>
+  <tabstop>radioSoftware</tabstop>
+  <tabstop>radioHardware</tabstop>
+  <tabstop>checkTrace</tabstop>
+  <tabstop>breakpointCommand</tabstop>
+ </tabstops>
  <resources/>
  <connections>
   <connection>
@@ -60,8 +322,8 @@
    <slot>accept()</slot>
    <hints>
     <hint type="sourcelabel">
-     <x>248</x>
+     <x>260</x>
-     <y>254</y>
+     <y>450</y>
     </hint>
     <hint type="destinationlabel">
      <x>157</x>
@@ -76,8 +338,8 @@
    <slot>reject()</slot>
    <hints>
     <hint type="sourcelabel">
-     <x>316</x>
+     <x>328</x>
-     <y>260</y>
+     <y>450</y>
     </hint>
     <hint type="destinationlabel">
      <x>286</x>
@@ -85,5 +347,21 @@
     </hint>
    </hints>
   </connection>
+  <connection>
+   <sender>radioHardware</sender>
+   <signal>toggled(bool)</signal>
+   <receiver>hwConfigBox</receiver>
+   <slot>setEnabled(bool)</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>77</x>
+     <y>246</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>77</x>
+     <y>320</y>
+    </hint>
+   </hints>
+  </connection>
  </connections>
 </ui>

src/menus/DisassemblyContextMenu.cpp

@@ -10,6 +10,7 @@
 #include "dialogs/EditFunctionDialog.h"
 #include "dialogs/LinkTypeDialog.h"
 #include "dialogs/EditStringDialog.h"
+#include "dialogs/BreakpointsDialog.h"
 #include "MainWindow.h"
 
 #include <QtCore>
@@ -56,6 +57,7 @@ DisassemblyContextMenu::DisassemblyContextMenu(QWidget *parent, MainWindow *main
         actionSetBits64(this),
         actionContinueUntil(this),
         actionAddBreakpoint(this),
+        actionAdvancedBreakpoint(this),
         actionSetPC(this),
         actionSetToCode(this),
         actionSetAsStringAuto(this),
@@ -292,6 +294,9 @@ void DisassemblyContextMenu::addDebugMenu()
     initAction(&actionAddBreakpoint, tr("Add/remove breakpoint"),
                SLOT(on_actionAddBreakpoint_triggered()), getAddBPSequence());
     debugMenu->addAction(&actionAddBreakpoint);
+    initAction(&actionAdvancedBreakpoint, tr("Advanced breakpoint"),
+               SLOT(on_actionAdvancedBreakpoint_triggered()), QKeySequence(Qt::CTRL+Qt::Key_F2));
+    debugMenu->addAction(&actionAdvancedBreakpoint);
 
     initAction(&actionContinueUntil, tr("Continue until line"),
                SLOT(on_actionContinueUntil_triggered()));
@@ -503,9 +508,13 @@ void DisassemblyContextMenu::aboutToShowSlot()
 
     // Only show debug options if we are currently debugging
     debugMenu->menuAction()->setVisible(Core()->currentlyDebugging);
+    bool hasBreakpoint = Core()->breakpointIndexAt(offset) > -1;
+    actionAddBreakpoint.setText(hasBreakpoint ?
+                                     tr("Remove breakpoint") : tr("Add breakpoint"));
+    actionAdvancedBreakpoint.setText(hasBreakpoint ?
+                                     tr("Edit breakpoint") : tr("Advanced breakpoint"));
     QString progCounterName = Core()->getRegisterName("PC").toUpper();
     actionSetPC.setText("Set " + progCounterName + " here");
-
 }
 
 QKeySequence DisassemblyContextMenu::getCopySequence() const
@@ -731,6 +740,16 @@ void DisassemblyContextMenu::on_actionAddBreakpoint_triggered()
     Core()->toggleBreakpoint(offset);
 }
 
+void DisassemblyContextMenu::on_actionAdvancedBreakpoint_triggered()
+{
+    int index = Core()->breakpointIndexAt(offset);
+    if (index >= 0) {
+        BreakpointsDialog::editBreakpoint(Core()->getBreakpointAt(offset), this);
+    } else {
+        BreakpointsDialog::createNewBreakpoint(offset, this);
+    }
+}
+
 void DisassemblyContextMenu::on_actionContinueUntil_triggered()
 {
     Core()->continueUntilDebug(RAddressString(offset));

src/menus/DisassemblyContextMenu.h

@@ -53,6 +53,7 @@ private slots:
     void on_actionDeleteFunction_triggered();
 
     void on_actionAddBreakpoint_triggered();
+    void on_actionAdvancedBreakpoint_triggered();
     void on_actionContinueUntil_triggered();
     void on_actionSetPC_triggered();
 
@@ -157,6 +158,7 @@ private:
     QMenu *debugMenu;
     QAction actionContinueUntil;
     QAction actionAddBreakpoint;
+    QAction actionAdvancedBreakpoint;
     QAction actionSetPC;
 
     QAction actionSetToCode;

src/widgets/BreakpointWidget.cpp

@@ -30,6 +30,20 @@ int BreakpointModel::columnCount(const QModelIndex &) const
     return BreakpointModel::ColumnCount;
 }
 
+static QString formatHwBreakpoint(int permission) {
+    char data[] = "rwx";
+    if ((permission & (R_BP_PROT_READ | R_BP_PROT_ACCESS)) == 0) {
+        data[0] = '-';
+    }
+    if ((permission & (R_BP_PROT_WRITE | R_BP_PROT_ACCESS)) == 0) {
+        data[1] = '-';
+    }
+    if ((permission & R_BP_PROT_EXEC) == 0) {
+        data[2] = '-';
+    }
+    return data;
+}
+
 QVariant BreakpointModel::data(const QModelIndex &index, int role) const
 {
     if (index.row() >= breakpoints.count())
@@ -42,10 +56,15 @@ QVariant BreakpointModel::data(const QModelIndex &index, int role) const
         switch (index.column()) {
         case AddrColumn:
             return RAddressString(breakpoint.addr);
-        case PermColumn:
+        case NameColumn:
-            return breakpoint.permission;
+            return breakpoint.name;
-        case HwColumn:
+        case TypeColumn:
-            return breakpoint.hw;
+
+            if (breakpoint.hw) {
+                return tr("HW %1").arg(formatHwBreakpoint(breakpoint.permission));
+            } else {
+                return tr("SW");
+            }
         case TraceColumn:
             return breakpoint.trace;
         case EnabledColumn:
@@ -55,6 +74,8 @@ QVariant BreakpointModel::data(const QModelIndex &index, int role) const
         }
     case Qt::EditRole:
         switch (index.column()) {
+        case AddrColumn:
+            return breakpoint.addr;
         case TraceColumn:
             return breakpoint.trace;
         case EnabledColumn:
@@ -76,14 +97,14 @@ QVariant BreakpointModel::headerData(int section, Qt::Orientation, int role) con
         switch (section) {
         case AddrColumn:
             return tr("Offset");
-        case PermColumn:
+        case NameColumn:
-            return tr("Permissions");
+            return tr("Name");
-        case HwColumn:
+        case TypeColumn:
-            return tr("Hardware bp");
+            return tr("Type");
         case TraceColumn:
             return tr("Tracing");
         case EnabledColumn:
-            return tr("Active");
+            return tr("Enabled");
         default:
             return QVariant();
         }
@@ -104,7 +125,7 @@ bool BreakpointModel::setData(const QModelIndex &index, const QVariant &value, i
         switch (index.column()) {
         case TraceColumn:
             breakpoint.trace = value.toBool();
-            Core()->setBreakpointTrace(index.row(), breakpoint.trace);
+            Core()->setBreakpointTrace(breakpoint.index, breakpoint.trace);
             emit dataChanged(index, index, {role, Qt::DisplayRole});
             return true;
         case EnabledColumn:
@@ -148,37 +169,8 @@ RVA BreakpointModel::address(const QModelIndex &index) const
 BreakpointProxyModel::BreakpointProxyModel(BreakpointModel *sourceModel, QObject *parent)
     : AddressableFilterProxyModel(sourceModel, parent)
 {
-}
+     // Use numeric values instead of numbers converted to strings if available
-
+    this->setSortRole(Qt::EditRole);
-bool BreakpointProxyModel::filterAcceptsRow(int row, const QModelIndex &parent) const
-{
-    QModelIndex index = sourceModel()->index(row, 0, parent);
-    BreakpointDescription item = index.data(
-                                     BreakpointModel::BreakpointDescriptionRole).value<BreakpointDescription>();
-    return item.permission.contains(filterRegExp());
-}
-
-bool BreakpointProxyModel::lessThan(const QModelIndex &left, const QModelIndex &right) const
-{
-    BreakpointDescription leftBreakpt = left.data(
-                                            BreakpointModel::BreakpointDescriptionRole).value<BreakpointDescription>();
-    BreakpointDescription rightBreakpt = right.data(
-                                             BreakpointModel::BreakpointDescriptionRole).value<BreakpointDescription>();
-
-    switch (left.column()) {
-    case BreakpointModel::AddrColumn:
-        return leftBreakpt.addr < rightBreakpt.addr;
-    case BreakpointModel::HwColumn:
-        return leftBreakpt.hw < rightBreakpt.hw;
-    case BreakpointModel::PermColumn:
-        return leftBreakpt.permission < rightBreakpt.permission;
-    case BreakpointModel::EnabledColumn:
-        return leftBreakpt.enabled < rightBreakpt.enabled;
-    default:
-        break;
-    }
-
-    return leftBreakpt.addr < rightBreakpt.addr;
 }
 
 BreakpointWidget::BreakpointWidget(MainWindow *main, QAction *action) :
@@ -212,7 +204,11 @@ BreakpointWidget::BreakpointWidget(MainWindow *main, QAction *action) :
     connect(actionToggleBreakpoint, &QAction::triggered, this, &BreakpointWidget::toggleBreakpoint);
     ui->breakpointTreeView->addAction(actionToggleBreakpoint);
 
+    actionEditBreakpoint = new QAction(tr("Edit"), this);
+    connect(actionEditBreakpoint, &QAction::triggered, this, &BreakpointWidget::editBreakpoint);
+
     auto contextMenu = ui->breakpointTreeView->getItemContextMenu();
+    contextMenu->addAction(actionEditBreakpoint);
     contextMenu->addAction(actionToggleBreakpoint);
     contextMenu->addAction(actionDelBreakpoint);
 
@@ -247,17 +243,7 @@ void BreakpointWidget::setScrollMode()
 
 void BreakpointWidget::addBreakpointDialog()
 {
-    BreakpointsDialog dialog(this);
+    BreakpointsDialog::createNewBreakpoint(RVA_INVALID, this);
-
-    if (dialog.exec()) {
-        QString bps = dialog.getBreakpoints();
-        if (!bps.isEmpty()) {
-            QStringList bpList = bps.split(QLatin1Char(' '), QString::SkipEmptyParts);
-            for (const QString &bp : bpList) {
-                Core()->addBreakpoint(bp);
-            }
-        }
-    }
 }
 
 QVector<RVA> BreakpointWidget::getSelectedAddresses() const
@@ -289,3 +275,15 @@ void BreakpointWidget::toggleBreakpoint()
     }
     editing = false;
 }
+
+void BreakpointWidget::editBreakpoint()
+{
+    auto index = ui->breakpointTreeView->currentIndex();
+    if (index.isValid()) {
+        auto data = breakpointProxyModel->data(index, BreakpointModel::BreakpointDescriptionRole);
+        if (!data.isNull()) {
+            auto breakpoint = data.value<BreakpointDescription>();
+            BreakpointsDialog::editBreakpoint(breakpoint, this);
+        }
+    }
+}

src/widgets/BreakpointWidget.h

@@ -31,7 +31,7 @@ private:
     QList<BreakpointDescription> breakpoints;
 
 public:
-    enum Column { AddrColumn = 0, PermColumn, HwColumn, TraceColumn, EnabledColumn, ColumnCount };
+    enum Column { AddrColumn = 0, NameColumn, TypeColumn, TraceColumn, EnabledColumn, ColumnCount };
     enum Role { BreakpointDescriptionRole = Qt::UserRole };
 
     BreakpointModel(QObject *parent = nullptr);
@@ -60,9 +60,6 @@ class BreakpointProxyModel : public AddressableFilterProxyModel
 public:
     BreakpointProxyModel(BreakpointModel *sourceModel, QObject *parent = nullptr);
 
-protected:
-    bool filterAcceptsRow(int row, const QModelIndex &parent) const override;
-    bool lessThan(const QModelIndex &left, const QModelIndex &right) const override;
 };
 
 
@@ -78,6 +75,7 @@ public:
 private slots:
     void delBreakpoint();
     void toggleBreakpoint();
+    void editBreakpoint();
     void addBreakpointDialog();
     void refreshBreakpoint();
 
@@ -89,6 +87,7 @@ private:
     QList<BreakpointDescription> breakpoints;
     QAction *actionDelBreakpoint = nullptr;
     QAction *actionToggleBreakpoint = nullptr;
+    QAction *actionEditBreakpoint = nullptr;
 
     void setScrollMode();
     QVector<RVA> getSelectedAddresses() const;