From 2dc2ce163975156f103b28392a5d65e6dbbd3ba3 Mon Sep 17 00:00:00 2001 From: Ivan Sorokin Date: Sun, 16 Feb 2020 18:50:16 +0300 Subject: [PATCH 1/2] introduce a plugin API: IPlugin::globalShortcuts() --- include/IPlugin.h | 4 ++++ plugins/Assembler/Assembler.cpp | 24 ++++++++++++++++-------- plugins/Assembler/Assembler.h | 3 +++ src/Debugger.cpp | 6 +----- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/include/IPlugin.h b/include/IPlugin.h index baa7a961f..c98ce3d1e 100644 --- a/include/IPlugin.h +++ b/include/IPlugin.h @@ -42,6 +42,10 @@ class IPlugin { public: virtual QMenu *menu(QWidget *parent = nullptr) = 0; +public: + // optional, overload this to provide actions that are available by global shortcuts + virtual QList globalShortcuts() { return {}; } + public: // optional, overload these to have there contents added to a view's context menu virtual QList cpuContextMenu() { return {}; } diff --git a/plugins/Assembler/Assembler.cpp b/plugins/Assembler/Assembler.cpp index c3772b532..a5347df0f 100644 --- a/plugins/Assembler/Assembler.cpp +++ b/plugins/Assembler/Assembler.cpp @@ -33,7 +33,10 @@ namespace AssemblerPlugin { * @param parent */ Assembler::Assembler(QObject *parent) - : QObject(parent) { + : QObject(parent), + action_assemble_(tr("&Assemble..."), this) { + action_assemble_.setShortcut(QKeySequence(tr("Space"))); + connect(&action_assemble_, &QAction::triggered, this, &Assembler::showDialog); } /** @@ -44,19 +47,24 @@ Assembler::~Assembler() { } /** - * @brief Assembler::cpuContextMenu + * @brief Assembler::globalShortcuts * @return */ -QList Assembler::cpuContextMenu() { +QList Assembler::globalShortcuts() { QList ret; + ret << &action_assemble_; + return ret; +} - auto action_assemble = new QAction(tr("&Assemble..."), this); - action_assemble->setShortcut(QKeySequence(tr("Space"))); - - connect(action_assemble, &QAction::triggered, this, &Assembler::showDialog); - ret << action_assemble; +/** + * @brief Assembler::cpuContextMenu + * @return + */ +QList Assembler::cpuContextMenu() { + QList ret; + ret << &action_assemble_; return ret; } diff --git a/plugins/Assembler/Assembler.h b/plugins/Assembler/Assembler.h index 1811a5fbd..31fd083f0 100644 --- a/plugins/Assembler/Assembler.h +++ b/plugins/Assembler/Assembler.h @@ -20,6 +20,7 @@ along with this program. If not, see . #define ASSEMBLER_H_20130611_ #include "IPlugin.h" +#include class QMenu; class QDialog; @@ -39,6 +40,7 @@ class Assembler : public QObject, public IPlugin { public: QMenu *menu(QWidget *parent = nullptr) override; + QList globalShortcuts() override; QList cpuContextMenu() override; QWidget *optionsPage() override; @@ -47,6 +49,7 @@ class Assembler : public QObject, public IPlugin { private: QPointer dialog_ = nullptr; + QAction action_assemble_; }; } diff --git a/src/Debugger.cpp b/src/Debugger.cpp index e3d573f82..ead308ed7 100644 --- a/src/Debugger.cpp +++ b/src/Debugger.cpp @@ -858,11 +858,7 @@ void Debugger::finishPluginSetup() { } // setup the shortcuts for these actions - const QList register_actions = p->registerContextMenu(); - const QList cpu_actions = p->cpuContextMenu(); - const QList stack_actions = p->stackContextMenu(); - const QList data_actions = p->dataContextMenu(); - const QList actions = register_actions + cpu_actions + stack_actions + data_actions; + const QList actions = p->globalShortcuts(); for (QAction *action : actions) { QKeySequence shortcut = action->shortcut(); From 36125f4aad162228c0585988da67b966673808e0 Mon Sep 17 00:00:00 2001 From: Ivan Sorokin Date: Tue, 18 Feb 2020 02:56:46 +0300 Subject: [PATCH 2/2] an attempt to fix issue #744 The issue was about leaks reported by leak sanitizer: Indirect leak of 48 byte(s) in 1 object(s) allocated from: #0 0x7f5c95377d80 in operator new(unsigned long) #1 0x7f5c7e695267 in HardwareBreakpointsPlugin::HardwareBreakpoints::stackContextMenu() HardwareBreakpoints.cpp:253 #2 0x694aac in Debugger::Debugger(QWidget*) Debugger.cpp:485 #3 0x4e683a in start_debugger main.cpp:96 #4 0x4e683a in main main.cpp:255 Indirect leak of 48 byte(s) in 1 object(s) allocated from: #0 0x7f5c95377d80 in operator new(unsigned long) #1 0x7f5c7e69ba38 in HardwareBreakpointsPlugin::HardwareBreakpoints::cpuContextMenu() HardwareBreakpoints.cpp:324 #2 0x694aac in Debugger::Debugger(QWidget*) Debugger.cpp:485 #3 0x4e683a in start_debugger main.cpp:96 #4 0x4e683a in main main.cpp:255 Indirect leak of 48 byte(s) in 1 object(s) allocated from: #0 0x7f5c95377d80 in operator new(unsigned long) #1 0x7f5c7e6985d7 in HardwareBreakpointsPlugin::HardwareBreakpoints::dataContextMenu() HardwareBreakpoints.cpp:288 #2 0x694aac in Debugger::Debugger(QWidget*) Debugger.cpp:485 #3 0x4e683a in start_debugger main.cpp:96 #4 0x4e683a in main main.cpp:255 The most idiomatic way to fix the issue was to assign a parent to the submenus created in these functions. Unfortunately there is no suitable parent available in these functions. This commit fixes the issue in the most straight-forward way possible: it just adds QMenu* parent parameter to the functions * IPlugin::cpuContextMenu * IPlugin::registerContextMenu * IPlugin::stackContextMenu * IPlugin::dataContextMenu This allows plugins to set proper parent to all submenus they choose to create in order for them to be properly destroyed when root menu is destroyed. This commit also sets parents to the QActions that are created in these functions so they are timedly destroyed. As a possible follow-up these QActions can be created once in plugin constructor and not everytime the menu is created. --- include/IPlugin.h | 8 ++++---- plugins/Analyzer/Analyzer.cpp | 12 ++++++------ plugins/Analyzer/Analyzer.h | 2 +- plugins/Assembler/Assembler.cpp | 2 +- plugins/Assembler/Assembler.h | 2 +- plugins/BinarySearcher/BinarySearcher.cpp | 4 ++-- plugins/BinarySearcher/BinarySearcher.h | 2 +- plugins/Bookmarks/Bookmarks.cpp | 4 ++-- plugins/Bookmarks/Bookmarks.h | 2 +- .../HardwareBreakpoints.cpp | 18 +++++++++--------- .../HardwareBreakpoints/HardwareBreakpoints.h | 6 +++--- plugins/InstructionInspector/Plugin.cpp | 2 +- plugins/InstructionInspector/Plugin.h | 2 +- plugins/ODbgRegisterView/RegisterView.cpp | 2 +- src/Debugger.cpp | 10 +++++----- src/Debugger.h | 4 ++-- 16 files changed, 41 insertions(+), 41 deletions(-) diff --git a/include/IPlugin.h b/include/IPlugin.h index c98ce3d1e..b764920be 100644 --- a/include/IPlugin.h +++ b/include/IPlugin.h @@ -48,10 +48,10 @@ class IPlugin { public: // optional, overload these to have there contents added to a view's context menu - virtual QList cpuContextMenu() { return {}; } - virtual QList registerContextMenu() { return {}; } - virtual QList stackContextMenu() { return {}; } - virtual QList dataContextMenu() { return {}; } + virtual QList cpuContextMenu(QMenu */*parent*/) { return {}; } + virtual QList registerContextMenu(QMenu */*parent*/) { return {}; } + virtual QList stackContextMenu(QMenu */*parent*/) { return {}; } + virtual QList dataContextMenu(QMenu */*parent*/) { return {}; } // optional, overload this to add a page to the options dialog virtual QWidget *optionsPage() { return nullptr; } diff --git a/plugins/Analyzer/Analyzer.cpp b/plugins/Analyzer/Analyzer.cpp index 0fcb0ffd6..cc054a2cf 100644 --- a/plugins/Analyzer/Analyzer.cpp +++ b/plugins/Analyzer/Analyzer.cpp @@ -316,15 +316,15 @@ void Analyzer::gotoFunctionEnd() { * @brief Analyzer::cpuContextMenu * @return */ -QList Analyzer::cpuContextMenu() { +QList Analyzer::cpuContextMenu(QMenu *parent) { QList ret; - auto action_find = new QAction(tr("Analyze Here"), this); - auto action_goto_function_start = new QAction(tr("Goto Function Start"), this); - auto action_goto_function_end = new QAction(tr("Goto Function End"), this); - auto action_mark_function_start = new QAction(tr("Mark As Function Start"), this); - auto action_xrefs = new QAction(tr("Show X-Refs"), this); + auto action_find = new QAction(tr("Analyze Here"), parent); + auto action_goto_function_start = new QAction(tr("Goto Function Start"), parent); + auto action_goto_function_end = new QAction(tr("Goto Function End"), parent); + auto action_mark_function_start = new QAction(tr("Mark As Function Start"), parent); + auto action_xrefs = new QAction(tr("Show X-Refs"), parent); connect(action_find, &QAction::triggered, this, &Analyzer::doViewAnalysis); connect(action_goto_function_start, &QAction::triggered, this, &Analyzer::gotoFunctionStart); diff --git a/plugins/Analyzer/Analyzer.h b/plugins/Analyzer/Analyzer.h index 5120f2508..6705c867e 100644 --- a/plugins/Analyzer/Analyzer.h +++ b/plugins/Analyzer/Analyzer.h @@ -53,7 +53,7 @@ class Analyzer final : public QObject, public IAnalyzer, public IPlugin { public: QMenu *menu(QWidget *parent = nullptr) override; - QList cpuContextMenu() override; + QList cpuContextMenu(QMenu *parent) override; private: void privateInit() override; diff --git a/plugins/Assembler/Assembler.cpp b/plugins/Assembler/Assembler.cpp index a5347df0f..705b03618 100644 --- a/plugins/Assembler/Assembler.cpp +++ b/plugins/Assembler/Assembler.cpp @@ -61,7 +61,7 @@ QList Assembler::globalShortcuts() { * @brief Assembler::cpuContextMenu * @return */ -QList Assembler::cpuContextMenu() { +QList Assembler::cpuContextMenu(QMenu *parent) { QList ret; ret << &action_assemble_; diff --git a/plugins/Assembler/Assembler.h b/plugins/Assembler/Assembler.h index 31fd083f0..b9d3c9c98 100644 --- a/plugins/Assembler/Assembler.h +++ b/plugins/Assembler/Assembler.h @@ -41,7 +41,7 @@ class Assembler : public QObject, public IPlugin { public: QMenu *menu(QWidget *parent = nullptr) override; QList globalShortcuts() override; - QList cpuContextMenu() override; + QList cpuContextMenu(QMenu *parent) override; QWidget *optionsPage() override; private: diff --git a/plugins/BinarySearcher/BinarySearcher.cpp b/plugins/BinarySearcher/BinarySearcher.cpp index e5cafcc38..fbaa83632 100644 --- a/plugins/BinarySearcher/BinarySearcher.cpp +++ b/plugins/BinarySearcher/BinarySearcher.cpp @@ -53,11 +53,11 @@ QMenu *BinarySearcher::menu(QWidget *parent) { * @brief BinarySearcher::stackContextMenu * @return */ -QList BinarySearcher::stackContextMenu() { +QList BinarySearcher::stackContextMenu(QMenu *parent) { QList ret; - auto action_find = new QAction(tr("&Find ASCII String"), this); + auto action_find = new QAction(tr("&Find ASCII String"), parent); connect(action_find, &QAction::triggered, this, &BinarySearcher::mnuStackFindAscii); ret << action_find; diff --git a/plugins/BinarySearcher/BinarySearcher.h b/plugins/BinarySearcher/BinarySearcher.h index ecf348aed..2bb375ae8 100644 --- a/plugins/BinarySearcher/BinarySearcher.h +++ b/plugins/BinarySearcher/BinarySearcher.h @@ -39,7 +39,7 @@ class BinarySearcher : public QObject, public IPlugin { public: QMenu *menu(QWidget *parent = nullptr) override; - QList stackContextMenu() override; + QList stackContextMenu(QMenu *parent) override; public Q_SLOTS: void showMenu(); diff --git a/plugins/Bookmarks/Bookmarks.cpp b/plugins/Bookmarks/Bookmarks.cpp index 303779fc8..f0d824c19 100644 --- a/plugins/Bookmarks/Bookmarks.cpp +++ b/plugins/Bookmarks/Bookmarks.cpp @@ -95,11 +95,11 @@ QMenu *Bookmarks::menu(QWidget *parent) { * @brief Bookmarks::cpuContextMenu * @return */ -QList Bookmarks::cpuContextMenu() { +QList Bookmarks::cpuContextMenu(QMenu *parent) { QList ret; - auto action_bookmark = new QAction(tr("Add &Bookmark"), this); + auto action_bookmark = new QAction(tr("Add &Bookmark"), parent); connect(action_bookmark, &QAction::triggered, this, &Bookmarks::addBookmarkMenu); ret << action_bookmark; diff --git a/plugins/Bookmarks/Bookmarks.h b/plugins/Bookmarks/Bookmarks.h index 2582ee080..dde035ba9 100644 --- a/plugins/Bookmarks/Bookmarks.h +++ b/plugins/Bookmarks/Bookmarks.h @@ -40,7 +40,7 @@ class Bookmarks : public QObject, public IPlugin { public: QMenu *menu(QWidget *parent = nullptr) override; - QList cpuContextMenu() override; + QList cpuContextMenu(QMenu *parent) override; public: QVariantMap saveState() const override; diff --git a/plugins/HardwareBreakpoints/HardwareBreakpoints.cpp b/plugins/HardwareBreakpoints/HardwareBreakpoints.cpp index 5c97c76bd..6e0dc0ec7 100644 --- a/plugins/HardwareBreakpoints/HardwareBreakpoints.cpp +++ b/plugins/HardwareBreakpoints/HardwareBreakpoints.cpp @@ -249,8 +249,8 @@ edb::EventStatus HardwareBreakpoints::handleEvent(const std::shared_ptr HardwareBreakpoints::stackContextMenu() { - auto menu = new QMenu(tr("Hardware Breakpoints")); +QList HardwareBreakpoints::stackContextMenu(QMenu *parent) { + auto menu = new QMenu(tr("Hardware Breakpoints"), parent); auto rw1 = menu->addAction(tr("Hardware, On Read/Write #1"), this, SLOT(setAccess1())); auto rw2 = menu->addAction(tr("Hardware, On Read/Write #2"), this, SLOT(setAccess2())); @@ -274,7 +274,7 @@ QList HardwareBreakpoints::stackContextMenu() { QList ret; - auto action = new QAction(tr("Hardware Breakpoints"), this); + auto action = new QAction(tr("Hardware Breakpoints"), parent); action->setMenu(menu); ret << action; return ret; @@ -284,8 +284,8 @@ QList HardwareBreakpoints::stackContextMenu() { * @brief HardwareBreakpoints::dataContextMenu * @return */ -QList HardwareBreakpoints::dataContextMenu() { - auto menu = new QMenu(tr("Hardware Breakpoints")); +QList HardwareBreakpoints::dataContextMenu(QMenu *parent) { + auto menu = new QMenu(tr("Hardware Breakpoints"), parent); auto rw1 = menu->addAction(tr("Hardware, On Read/Write #1"), this, SLOT(setAccess1())); auto rw2 = menu->addAction(tr("Hardware, On Read/Write #2"), this, SLOT(setAccess2())); @@ -309,7 +309,7 @@ QList HardwareBreakpoints::dataContextMenu() { QList ret; - auto action = new QAction(tr("Hardware Breakpoints"), this); + auto action = new QAction(tr("Hardware Breakpoints"), parent); action->setMenu(menu); ret << action; return ret; @@ -319,9 +319,9 @@ QList HardwareBreakpoints::dataContextMenu() { * @brief HardwareBreakpoints::cpuContextMenu * @return */ -QList HardwareBreakpoints::cpuContextMenu() { +QList HardwareBreakpoints::cpuContextMenu(QMenu *parent) { - auto menu = new QMenu(tr("Hardware Breakpoints")); + auto menu = new QMenu(tr("Hardware Breakpoints"), parent); auto ex1 = menu->addAction(tr("Hardware, On Execute #1"), this, SLOT(setExec1())); auto ex2 = menu->addAction(tr("Hardware, On Execute #2"), this, SLOT(setExec2())); auto ex3 = menu->addAction(tr("Hardware, On Execute #3"), this, SLOT(setExec3())); @@ -354,7 +354,7 @@ QList HardwareBreakpoints::cpuContextMenu() { QList ret; - auto action = new QAction(tr("Hardware Breakpoints"), this); + auto action = new QAction(tr("Hardware Breakpoints"), parent); action->setMenu(menu); ret << action; return ret; diff --git a/plugins/HardwareBreakpoints/HardwareBreakpoints.h b/plugins/HardwareBreakpoints/HardwareBreakpoints.h index 70356a04e..6c7a04fd3 100644 --- a/plugins/HardwareBreakpoints/HardwareBreakpoints.h +++ b/plugins/HardwareBreakpoints/HardwareBreakpoints.h @@ -49,9 +49,9 @@ class HardwareBreakpoints : public QObject, public IPlugin, public IDebugEventHa public: QMenu *menu(QWidget *parent = nullptr) override; edb::EventStatus handleEvent(const std::shared_ptr &event) override; - QList cpuContextMenu() override; - QList stackContextMenu() override; - QList dataContextMenu() override; + QList cpuContextMenu(QMenu *parent) override; + QList stackContextMenu(QMenu *parent) override; + QList dataContextMenu(QMenu *parent) override; public Q_SLOTS: void showMenu(); diff --git a/plugins/InstructionInspector/Plugin.cpp b/plugins/InstructionInspector/Plugin.cpp index 84d60df69..f1186cd9a 100644 --- a/plugins/InstructionInspector/Plugin.cpp +++ b/plugins/InstructionInspector/Plugin.cpp @@ -1319,7 +1319,7 @@ QMenu *Plugin::menu(QWidget *) { * @brief Plugin::cpuContextMenu * @return */ -QList Plugin::cpuContextMenu() { +QList Plugin::cpuContextMenu(QMenu *) { return {menuAction_}; } diff --git a/plugins/InstructionInspector/Plugin.h b/plugins/InstructionInspector/Plugin.h index f964a8724..5e57f4e18 100644 --- a/plugins/InstructionInspector/Plugin.h +++ b/plugins/InstructionInspector/Plugin.h @@ -67,7 +67,7 @@ class Plugin : public QObject, public IPlugin { public: explicit Plugin(QObject *parent = nullptr); QMenu *menu(QWidget *parent = nullptr) override; - QList cpuContextMenu() override; + QList cpuContextMenu(QMenu *parent) override; private: void showDialog() const; diff --git a/plugins/ODbgRegisterView/RegisterView.cpp b/plugins/ODbgRegisterView/RegisterView.cpp index d7a8906b1..f3cd27c61 100644 --- a/plugins/ODbgRegisterView/RegisterView.cpp +++ b/plugins/ODbgRegisterView/RegisterView.cpp @@ -170,7 +170,7 @@ void ODBRegView::showMenu(const QPoint &position, const QList &additi if (model_->activeIndex().isValid()) { QList debuggerActions; - QMetaObject::invokeMethod(edb::v1::debugger_ui, "currentRegisterContextMenuItems", Qt::DirectConnection, Q_RETURN_ARG(QList, debuggerActions)); + QMetaObject::invokeMethod(edb::v1::debugger_ui, "currentRegisterContextMenuItems", Qt::DirectConnection, Q_RETURN_ARG(QList, debuggerActions), Q_ARG(QMenu *, &menu)); items.push_back(nullptr); items.append(debuggerActions); } diff --git a/src/Debugger.cpp b/src/Debugger.cpp index ead308ed7..0acb725e2 100644 --- a/src/Debugger.cpp +++ b/src/Debugger.cpp @@ -1227,7 +1227,7 @@ Register Debugger::activeRegister() const { // Name: on_registerList_customContextMenuRequested // Desc: context menu handler for register view //------------------------------------------------------------------------------ -QList Debugger::currentRegisterContextMenuItems() const { +QList Debugger::currentRegisterContextMenuItems(QMenu *parent) const { QList allActions; const auto reg = activeRegister(); if (reg.type() & (Register::TYPE_GPR | Register::TYPE_IP)) { @@ -1239,7 +1239,7 @@ QList Debugger::currentRegisterContextMenuItems() const { allActions.append(actions); } - allActions.append(getPluginContextMenuItems(&IPlugin::registerContextMenu)); + allActions.append(getPluginContextMenuItems(parent, &IPlugin::registerContextMenu)); return allActions; } @@ -3212,12 +3212,12 @@ void Debugger::mnuDumpDeleteTab() { // NULL pointer items mean "create separator here". //------------------------------------------------------------------------------ template -QList Debugger::getPluginContextMenuItems(const F &f) const { +QList Debugger::getPluginContextMenuItems(QMenu *parent, const F &f) const { QList actions; for (QObject *plugin : edb::v1::plugin_list()) { if (auto p = qobject_cast(plugin)) { - const QList acts = (p->*f)(); + const QList acts = (p->*f)(parent); if (!acts.isEmpty()) { actions.push_back(nullptr); actions.append(acts); @@ -3235,7 +3235,7 @@ template void Debugger::addPluginContextMenu(const T &menu, const F &f) { for (QObject *plugin : edb::v1::plugin_list()) { if (auto p = qobject_cast(plugin)) { - const QList acts = (p->*f)(); + const QList acts = (p->*f)(menu); if (!acts.isEmpty()) { menu->addSeparator(); menu->addActions(acts); diff --git a/src/Debugger.h b/src/Debugger.h index b125a027d..557400a18 100644 --- a/src/Debugger.h +++ b/src/Debugger.h @@ -170,7 +170,7 @@ private Q_SLOTS: private Q_SLOTS: // the manually connected Register slots - QList currentRegisterContextMenuItems() const; + QList currentRegisterContextMenuItems(QMenu *parent) const; void mnuRegisterFollowInDump() { followRegisterInDump(false); } void mnuRegisterFollowInDumpNewTab() { followRegisterInDump(true); } void mnuRegisterFollowInStack(); @@ -269,7 +269,7 @@ private Q_SLOTS: Result getFollowAddress(const Ptr &hexview); template - QList getPluginContextMenuItems(const F &f) const; + QList getPluginContextMenuItems(QMenu *parent, const F &f) const; template void addPluginContextMenu(const T &menu, const F &f);