Skip to content

Commit

Permalink
an attempt to fix issue eteran#744
Browse files Browse the repository at this point in the history
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)
    eteran#1 0x7f5c7e695267 in HardwareBreakpointsPlugin::HardwareBreakpoints::stackContextMenu() HardwareBreakpoints.cpp:253
    eteran#2 0x694aac in Debugger::Debugger(QWidget*) Debugger.cpp:485
    eteran#3 0x4e683a in start_debugger main.cpp:96
    eteran#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)
    eteran#1 0x7f5c7e69ba38 in HardwareBreakpointsPlugin::HardwareBreakpoints::cpuContextMenu() HardwareBreakpoints.cpp:324
    eteran#2 0x694aac in Debugger::Debugger(QWidget*) Debugger.cpp:485
    eteran#3 0x4e683a in start_debugger main.cpp:96
    eteran#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)
    eteran#1 0x7f5c7e6985d7 in HardwareBreakpointsPlugin::HardwareBreakpoints::dataContextMenu() HardwareBreakpoints.cpp:288
    eteran#2 0x694aac in Debugger::Debugger(QWidget*) Debugger.cpp:485
    eteran#3 0x4e683a in start_debugger main.cpp:96
    eteran#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.
  • Loading branch information
sorokin committed Feb 18, 2020
1 parent 40892ac commit 9427838
Show file tree
Hide file tree
Showing 16 changed files with 41 additions and 41 deletions.
8 changes: 4 additions & 4 deletions include/IPlugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ class IPlugin {

public:
// optional, overload these to have there contents added to a view's context menu
virtual QList<QAction *> cpuContextMenu() { return {}; }
virtual QList<QAction *> registerContextMenu() { return {}; }
virtual QList<QAction *> stackContextMenu() { return {}; }
virtual QList<QAction *> dataContextMenu() { return {}; }
virtual QList<QAction *> cpuContextMenu(QMenu */*parent*/) { return {}; }
virtual QList<QAction *> registerContextMenu(QMenu */*parent*/) { return {}; }
virtual QList<QAction *> stackContextMenu(QMenu */*parent*/) { return {}; }
virtual QList<QAction *> dataContextMenu(QMenu */*parent*/) { return {}; }

// optional, overload this to add a page to the options dialog
virtual QWidget *optionsPage() { return nullptr; }
Expand Down
12 changes: 6 additions & 6 deletions plugins/Analyzer/Analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,15 +316,15 @@ void Analyzer::gotoFunctionEnd() {
* @brief Analyzer::cpuContextMenu
* @return
*/
QList<QAction *> Analyzer::cpuContextMenu() {
QList<QAction *> Analyzer::cpuContextMenu(QMenu *parent) {

QList<QAction *> 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);
Expand Down
2 changes: 1 addition & 1 deletion plugins/Analyzer/Analyzer.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class Analyzer final : public QObject, public IAnalyzer, public IPlugin {

public:
QMenu *menu(QWidget *parent = nullptr) override;
QList<QAction *> cpuContextMenu() override;
QList<QAction *> cpuContextMenu(QMenu *parent) override;

private:
void privateInit() override;
Expand Down
2 changes: 1 addition & 1 deletion plugins/Assembler/Assembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ QList<QAction *> Assembler::globalShortcuts() {
* @brief Assembler::cpuContextMenu
* @return
*/
QList<QAction *> Assembler::cpuContextMenu() {
QList<QAction *> Assembler::cpuContextMenu(QMenu *parent) {

QList<QAction *> ret;
ret << &action_assemble_;
Expand Down
2 changes: 1 addition & 1 deletion plugins/Assembler/Assembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Assembler : public QObject, public IPlugin {
public:
QMenu *menu(QWidget *parent = nullptr) override;
QList<QAction *> globalShortcuts() override;
QList<QAction *> cpuContextMenu() override;
QList<QAction *> cpuContextMenu(QMenu *parent) override;
QWidget *optionsPage() override;

private:
Expand Down
4 changes: 2 additions & 2 deletions plugins/BinarySearcher/BinarySearcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ QMenu *BinarySearcher::menu(QWidget *parent) {
* @brief BinarySearcher::stackContextMenu
* @return
*/
QList<QAction *> BinarySearcher::stackContextMenu() {
QList<QAction *> BinarySearcher::stackContextMenu(QMenu *parent) {

QList<QAction *> 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;

Expand Down
2 changes: 1 addition & 1 deletion plugins/BinarySearcher/BinarySearcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class BinarySearcher : public QObject, public IPlugin {

public:
QMenu *menu(QWidget *parent = nullptr) override;
QList<QAction *> stackContextMenu() override;
QList<QAction *> stackContextMenu(QMenu *parent) override;

public Q_SLOTS:
void showMenu();
Expand Down
4 changes: 2 additions & 2 deletions plugins/Bookmarks/Bookmarks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ QMenu *Bookmarks::menu(QWidget *parent) {
* @brief Bookmarks::cpuContextMenu
* @return
*/
QList<QAction *> Bookmarks::cpuContextMenu() {
QList<QAction *> Bookmarks::cpuContextMenu(QMenu *parent) {

QList<QAction *> 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;

Expand Down
2 changes: 1 addition & 1 deletion plugins/Bookmarks/Bookmarks.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Bookmarks : public QObject, public IPlugin {

public:
QMenu *menu(QWidget *parent = nullptr) override;
QList<QAction *> cpuContextMenu() override;
QList<QAction *> cpuContextMenu(QMenu *parent) override;

public:
QVariantMap saveState() const override;
Expand Down
18 changes: 9 additions & 9 deletions plugins/HardwareBreakpoints/HardwareBreakpoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ edb::EventStatus HardwareBreakpoints::handleEvent(const std::shared_ptr<IDebugEv
* @brief HardwareBreakpoints::stackContextMenu
* @return
*/
QList<QAction *> HardwareBreakpoints::stackContextMenu() {
auto menu = new QMenu(tr("Hardware Breakpoints"));
QList<QAction *> 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()));
Expand All @@ -274,7 +274,7 @@ QList<QAction *> HardwareBreakpoints::stackContextMenu() {

QList<QAction *> ret;

auto action = new QAction(tr("Hardware Breakpoints"), this);
auto action = new QAction(tr("Hardware Breakpoints"), parent);
action->setMenu(menu);
ret << action;
return ret;
Expand All @@ -284,8 +284,8 @@ QList<QAction *> HardwareBreakpoints::stackContextMenu() {
* @brief HardwareBreakpoints::dataContextMenu
* @return
*/
QList<QAction *> HardwareBreakpoints::dataContextMenu() {
auto menu = new QMenu(tr("Hardware Breakpoints"));
QList<QAction *> 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()));
Expand All @@ -309,7 +309,7 @@ QList<QAction *> HardwareBreakpoints::dataContextMenu() {

QList<QAction *> ret;

auto action = new QAction(tr("Hardware Breakpoints"), this);
auto action = new QAction(tr("Hardware Breakpoints"), parent);
action->setMenu(menu);
ret << action;
return ret;
Expand All @@ -319,9 +319,9 @@ QList<QAction *> HardwareBreakpoints::dataContextMenu() {
* @brief HardwareBreakpoints::cpuContextMenu
* @return
*/
QList<QAction *> HardwareBreakpoints::cpuContextMenu() {
QList<QAction *> 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()));
Expand Down Expand Up @@ -354,7 +354,7 @@ QList<QAction *> HardwareBreakpoints::cpuContextMenu() {

QList<QAction *> ret;

auto action = new QAction(tr("Hardware Breakpoints"), this);
auto action = new QAction(tr("Hardware Breakpoints"), parent);
action->setMenu(menu);
ret << action;
return ret;
Expand Down
6 changes: 3 additions & 3 deletions plugins/HardwareBreakpoints/HardwareBreakpoints.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<IDebugEvent> &event) override;
QList<QAction *> cpuContextMenu() override;
QList<QAction *> stackContextMenu() override;
QList<QAction *> dataContextMenu() override;
QList<QAction *> cpuContextMenu(QMenu *parent) override;
QList<QAction *> stackContextMenu(QMenu *parent) override;
QList<QAction *> dataContextMenu(QMenu *parent) override;

public Q_SLOTS:
void showMenu();
Expand Down
2 changes: 1 addition & 1 deletion plugins/InstructionInspector/Plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1319,7 +1319,7 @@ QMenu *Plugin::menu(QWidget *) {
* @brief Plugin::cpuContextMenu
* @return
*/
QList<QAction *> Plugin::cpuContextMenu() {
QList<QAction *> Plugin::cpuContextMenu(QMenu *) {
return {menuAction_};
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/InstructionInspector/Plugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Plugin : public QObject, public IPlugin {
public:
explicit Plugin(QObject *parent = nullptr);
QMenu *menu(QWidget *parent = nullptr) override;
QList<QAction *> cpuContextMenu() override;
QList<QAction *> cpuContextMenu(QMenu *parent) override;

private:
void showDialog() const;
Expand Down
2 changes: 1 addition & 1 deletion plugins/ODbgRegisterView/RegisterView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void ODBRegView::showMenu(const QPoint &position, const QList<QAction *> &additi

if (model_->activeIndex().isValid()) {
QList<QAction *> debuggerActions;
QMetaObject::invokeMethod(edb::v1::debugger_ui, "currentRegisterContextMenuItems", Qt::DirectConnection, Q_RETURN_ARG(QList<QAction *>, debuggerActions));
QMetaObject::invokeMethod(edb::v1::debugger_ui, "currentRegisterContextMenuItems", Qt::DirectConnection, Q_RETURN_ARG(QList<QAction *>, debuggerActions), Q_ARG(QMenu *, &menu));
items.push_back(nullptr);
items.append(debuggerActions);
}
Expand Down
10 changes: 5 additions & 5 deletions src/Debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ Register Debugger::activeRegister() const {
// Name: on_registerList_customContextMenuRequested
// Desc: context menu handler for register view
//------------------------------------------------------------------------------
QList<QAction *> Debugger::currentRegisterContextMenuItems() const {
QList<QAction *> Debugger::currentRegisterContextMenuItems(QMenu *parent) const {
QList<QAction *> allActions;
const auto reg = activeRegister();
if (reg.type() & (Register::TYPE_GPR | Register::TYPE_IP)) {
Expand All @@ -1240,7 +1240,7 @@ QList<QAction *> Debugger::currentRegisterContextMenuItems() const {

allActions.append(actions);
}
allActions.append(getPluginContextMenuItems(&IPlugin::registerContextMenu));
allActions.append(getPluginContextMenuItems(parent, &IPlugin::registerContextMenu));
return allActions;
}

Expand Down Expand Up @@ -3213,12 +3213,12 @@ void Debugger::mnuDumpDeleteTab() {
// NULL pointer items mean "create separator here".
//------------------------------------------------------------------------------
template <class F>
QList<QAction *> Debugger::getPluginContextMenuItems(const F &f) const {
QList<QAction *> Debugger::getPluginContextMenuItems(QMenu *parent, const F &f) const {
QList<QAction *> actions;

for (QObject *plugin : edb::v1::plugin_list()) {
if (auto p = qobject_cast<IPlugin *>(plugin)) {
const QList<QAction *> acts = (p->*f)();
const QList<QAction *> acts = (p->*f)(parent);
if (!acts.isEmpty()) {
actions.push_back(nullptr);
actions.append(acts);
Expand All @@ -3236,7 +3236,7 @@ template <class F, class T>
void Debugger::addPluginContextMenu(const T &menu, const F &f) {
for (QObject *plugin : edb::v1::plugin_list()) {
if (auto p = qobject_cast<IPlugin *>(plugin)) {
const QList<QAction *> acts = (p->*f)();
const QList<QAction *> acts = (p->*f)(menu);
if (!acts.isEmpty()) {
menu->addSeparator();
menu->addActions(acts);
Expand Down
4 changes: 2 additions & 2 deletions src/Debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private Q_SLOTS:

private Q_SLOTS:
// the manually connected Register slots
QList<QAction *> currentRegisterContextMenuItems() const;
QList<QAction *> currentRegisterContextMenuItems(QMenu *parent) const;
void mnuRegisterFollowInDump() { followRegisterInDump(false); }
void mnuRegisterFollowInDumpNewTab() { followRegisterInDump(true); }
void mnuRegisterFollowInStack();
Expand Down Expand Up @@ -269,7 +269,7 @@ private Q_SLOTS:
Result<edb::address_t, QString> getFollowAddress(const Ptr &hexview);

template <class F>
QList<QAction *> getPluginContextMenuItems(const F &f) const;
QList<QAction *> getPluginContextMenuItems(QMenu *parent, const F &f) const;

template <class F, class T>
void addPluginContextMenu(const T &menu, const F &f);
Expand Down

0 comments on commit 9427838

Please sign in to comment.