Skip to content
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.

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

Prevent segmentation fault and construct DlgAddWatchEntry dialogs in the stack. #143

Merged

Conversation

cristian64
Copy link
Collaborator

Previously, these dialogs were constructed dynamically in the heap, and were never deleted explicitly; only when their parent QObject was destroyed at a much later time would they be deleted.

This deferred destruction was in fact masking a segmentation fault caused by a double-free: the entry in the dialog was not extracted via stealEntry(); instead the reference was taken from the entryCopy variable (same pointer), resulting in the entry getting deleted twice, eventually.

Reproduction steps:

  • Double-click on an watch entry to bring up the Edit Watch dialog.
  • Make any edit and press OK.
  • Quit the application gracefully to force the destruction of the dialog.
  • A segmentation fault would be produced:
(gdb) bt
#0  QArrayDataPointer<char16_t>::deref() (this=<optimised out>, this=0x558b5d1a35c0) at /usr/include/c++/11/bits/atomic_base.h:392
#1  QArrayDataPointer<char16_t>::~QArrayDataPointer() (this=0x558b5d1a35c0, __in_chrg=<optimised out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qarraydatapointer.h:129
#2  QString::~QString() (this=0x558b5d1a35c0, __in_chrg=<optimised out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qstring.h:1302
#3  MemWatchEntry::~MemWatchEntry() (this=0x558b5d1a35c0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/MemoryWatch/MemWatchEntry.cpp:48
#4  0x0000558b5c517787 in DlgAddWatchEntry::~DlgAddWatchEntry() (this=0x558b5d1be8f0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MemWatcher/Dialogs/DlgAddWatchEntry.cpp:31
#5  0x0000558b5c51782d in DlgAddWatchEntry::~DlgAddWatchEntry() (this=0x558b5d1be8f0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MemWatcher/Dialogs/DlgAddWatchEntry.cpp:32
#6  0x00007fde5556599b in QObjectPrivate::deleteChildren() () at /lib/x86_64-linux-gnu/libQt6Core.so.6
#7  0x00007fde5624cab8 in QWidget::~QWidget() () at /lib/x86_64-linux-gnu/libQt6Widgets.so.6
#8  0x0000558b5c51d0cd in MemWatchWidget::~MemWatchWidget() (this=0x558b5cd068e0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MemWatcher/MemWatchWidget.cpp:41
#9  0x0000558b5c5367cd in MainWindow::~MainWindow() (this=0x7ffc70ef10f0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MainWindow.cpp:56
#10 0x0000558b5c4f1ad7 in main(int, char**) (argc=<optimised out>, argv=<optimised out>) at /w/dolphin-memory-engine/Source/main.cpp:56

…n the stack.

Previously, these dialogs were constructed dynamically in the heap, and
were never deleted explicitly; only when their parent QObject was
destroyed at a much later time would they be deleted.

This deferred destruction was in fact masking a segmentation fault
caused by a double-free: the entry in the dialog was not extracted
via `stealEntry()`; instead the reference was taken from the
`entryCopy` variable (same pointer), resulting in the entry getting
deleted twice, eventually.

Reproduction steps:
- Double-click on an watch entry to bring up the **Edit Watch** dialog.
- Make any edit and press **OK**.
- Quit the application gracefully to force the destruction of the
  dialog.
- A segmentation fault would be produced:

```
(gdb) bt
#0  QArrayDataPointer<char16_t>::deref() (this=<optimised out>, this=0x558b5d1a35c0) at /usr/include/c++/11/bits/atomic_base.h:392
aldelaro5#1  QArrayDataPointer<char16_t>::~QArrayDataPointer() (this=0x558b5d1a35c0, __in_chrg=<optimised out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qarraydatapointer.h:129
aldelaro5#2  QString::~QString() (this=0x558b5d1a35c0, __in_chrg=<optimised out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qstring.h:1302
aldelaro5#3  MemWatchEntry::~MemWatchEntry() (this=0x558b5d1a35c0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/MemoryWatch/MemWatchEntry.cpp:48
aldelaro5#4  0x0000558b5c517787 in DlgAddWatchEntry::~DlgAddWatchEntry() (this=0x558b5d1be8f0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MemWatcher/Dialogs/DlgAddWatchEntry.cpp:31
aldelaro5#5  0x0000558b5c51782d in DlgAddWatchEntry::~DlgAddWatchEntry() (this=0x558b5d1be8f0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MemWatcher/Dialogs/DlgAddWatchEntry.cpp:32
aldelaro5#6  0x00007fde5556599b in QObjectPrivate::deleteChildren() () at /lib/x86_64-linux-gnu/libQt6Core.so.6
aldelaro5#7  0x00007fde5624cab8 in QWidget::~QWidget() () at /lib/x86_64-linux-gnu/libQt6Widgets.so.6
aldelaro5#8  0x0000558b5c51d0cd in MemWatchWidget::~MemWatchWidget() (this=0x558b5cd068e0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MemWatcher/MemWatchWidget.cpp:41
aldelaro5#9  0x0000558b5c5367cd in MainWindow::~MainWindow() (this=0x7ffc70ef10f0, __in_chrg=<optimised out>) at /w/dolphin-memory-engine/Source/GUI/MainWindow.cpp:56
aldelaro5#10 0x0000558b5c4f1ad7 in main(int, char**) (argc=<optimised out>, argv=<optimised out>) at /w/dolphin-memory-engine/Source/main.cpp:56
```
@dreamsyntax dreamsyntax merged commit 75fe588 into aldelaro5:master May 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants