From 2c67d47ea15c6dc34cc20d8fbdb406efb19f11d7 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 1 Apr 2025 11:43:00 +0200 Subject: [PATCH] QDockWidget: fix UB (invalid member calls) in QDockAreaLayout::takeAt() The process by which the QDockAreaLayout changes a QDockAreaLayoutInfo from representing a QWidget that's being deleted to representing a QPlaceholderItem involves the construction of the latter from the former. If a QDockWidget is being deleted, however, at the time the QDockAreaLayout notices, the ex-QDockWidget has been demoted to a QObject, causing the calls to QWidget member functions to be UB: Says UBSan: qdockarealayout.cpp:46:25: runtime error: member call on address 0x7ffe74a429d0 which does not point to an object of type 'QWidget' 0x7ffe74a429d0: note: object is of type 'QObject' 33 7f 00 00 c0 ea 73 6e 33 7f 00 00 00 12 00 00 70 61 00 00 40 cd 41 83 33 7f 00 00 00 00 00 00 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QObject' #0 0x7f339546e251 in QPlaceHolderItem::QPlaceHolderItem(QWidget*) qdockarealayout.cpp:46 #1 0x7f33955169a8 in QDockAreaLayoutInfo::takeAt(int*, int) qdockarealayout.cpp:1780 #2 0x7f3395517175 in QDockAreaLayout::takeAt(int*, int) qdockarealayout.cpp:3432 #3 0x7f33959e38a8 in QMainWindowLayoutState::takeAt(int, int*) qmainwindowlayout.cpp:927 #4 0x7f33959e38a8 in QMainWindowLayoutState::takeAt(int, int*) qmainwindowlayout.cpp:919 #5 0x7f3395a42cdd in QMainWindowLayout::takeAt(int) qmainwindowlayout.cpp:2238 #6 0x7f3393fae246 in removeWidgetRecursively qlayout.cpp:485 #7 0x7f3393fb8300 in QLayout::widgetEvent(QEvent*) qlayout.cpp:544 #8 0x7f3393bde28a in QApplicationPrivate::notify_helper(QObject*, QEvent*) qapplication.cpp:3298 #9 0x7f3393c5f74a in QApplication::notify(QObject*, QEvent*) qapplication.cpp:3259 #10 0x7f336b784ada in QCoreApplication::notifyInternal2(QObject*, QEvent*) qcoreapplication.cpp:1111 #11 0x7f336b7874e3 in QCoreApplication::sendEvent(QObject*, QEvent*) qcoreapplication.cpp:1551 #12 0x7f336bcc624a in QObjectPrivate::setParent_helper(QObject*) qobject.cpp:2271 #13 0x7f336bccd76c in QObject::~QObject() qobject.cpp:1146 #14 0x7f339434e126 in QWidget::~QWidget() qwidget.cpp:1584 #15 0x7f33955b5815 in QDockWidget::~QDockWidget() qdockwidget.cpp:1362 [...] qwidget.h:816:25: runtime error: member call on address 0x7ffe74a429d0 which does not point to an object of type 'QWidget' 0x7ffe74a429d0: note: object is of type 'QObject' 33 7f 00 00 c0 ea 73 6e 33 7f 00 00 00 12 00 00 70 61 00 00 40 cd 41 83 33 7f 00 00 00 00 00 00 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QObject' #0 0x7f339546e0bb in QWidget::isWindow() const qwidget.h:816 #1 0x7f339546e0bb in QPlaceHolderItem::QPlaceHolderItem(QWidget*) qdockarealayout.cpp:47 [... rest as above...] Fix by dragging the setParent(nullptr) up into ~QDockWidget(). Ordinarily, that call happens only in ~QObject(). But that's what caused the layout to react to the ChildRemoved element too late. When doing it here, the dock widget is still itself, and all the QDockAreaLayout machinery can still access its QWidget-ness. Amends the start of the public history. After consulting with QtWidgets maintainer, not picking to 5.15, since, even though slim, there's a non-zero chance this might break something, somewhere. Pick-to: 6.9 6.8 6.5 Change-Id: I5472bbb0fcab9fb74272a1da6c2a2896226e12bb Reviewed-by: Richard Moe Gustavsen --- src/widgets/widgets/qdockwidget.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/widgets/widgets/qdockwidget.cpp b/src/widgets/widgets/qdockwidget.cpp index 214d00341ee..abee5f67219 100644 --- a/src/widgets/widgets/qdockwidget.cpp +++ b/src/widgets/widgets/qdockwidget.cpp @@ -1359,7 +1359,13 @@ QDockWidget::QDockWidget(const QString &title, QWidget *parent, Qt::WindowFlags Destroys the dock widget. */ QDockWidget::~QDockWidget() -{ } +{ + // Do all the unregistering while we're still a QDockWidget. Otherwise, it + // would be ~QObject() which does that and then QDockAreaLayout::takeAt(), + // acting on QEvent::ChildRemoved, will try to access our QWidget-ness when + // replacing us with a QPlaceHolderItem, causing UB: + setParent(nullptr); +} /*! Returns the widget for the dock widget. This function returns zero