Skip to content

Commit

Permalink
QGraphicsScene: Fix UB (invalid cast) in Private::ungrabMouse()
Browse files Browse the repository at this point in the history
Found by UBSan:

  qgraphicsscene.cpp:1000:40: runtime error: downcast of address 0x2af0d4072b00 which does not point to an object of type 'QGraphicsWidget'
  0x2af0d4072b00: note: object is of type 'QGraphicsObject'
   00 00 00 00  30 f5 26 bd f0 2a 00 00  90 e1 05 d4 f0 2a 00 00  a8 e3 26 bd f0 2a 00 00  d0 33 0f d4
                ^~~~~~~~~~~~~~~~~~~~~~~
                vptr for 'QGraphicsObject'
    #0 0x2af0badf1305 in QGraphicsScenePrivate::ungrabMouse(QGraphicsItem*, bool) qgraphicsscene.cpp:1000
    #1 0x2af0bae0fc24 in QGraphicsScenePrivate::removeItemHelper(QGraphicsItem*) qgraphicsscene.cpp:692
    #2 0x2af0bacd21f6 in QGraphicsItem::~QGraphicsItem() qgraphicsitem.cpp:1555
    #3 0x2af0bacd4c48 in QGraphicsObject::~QGraphicsObject() qgraphicsitem.cpp:7766
    #4 0x2af0baf7e99c in QGraphicsWidget::~QGraphicsWidget() qgraphicswidget.cpp:231
    #5 0x2af0baf7f8c0 in QGraphicsWidget::~QGraphicsWidget() qgraphicswidget.cpp:282
    qt#6 0x2af0badcee34 in QGraphicsScene::clear() qgraphicsscene.cpp:2388
    qt#7 0x2af0badcf3fc in QGraphicsScene::~QGraphicsScene() qgraphicsscene.cpp:1682
    qt#8 0x4b26f0 in tst_QGraphicsWidget::popupMouseGrabber() tst_qgraphicswidget.cpp:47

Fix by using the existing graphics widget pointer,
determined a line above to be equivalent to 'item',
for the removePopup() function call instead of
casting 'item' itself.

The rest of removePopup() appears to be well-behaved
and doesn't trigger any more UBSan errors, so it was
indeed just the cast which was undefined, no member
calls.

Change-Id: Ia54da90262a7a02f527914a90b0208be0ffc0f0b
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
  • Loading branch information
marc-kdab committed Sep 22, 2016
1 parent f6cb8b1 commit 622681e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/widgets/graphicsview/qgraphicsscene.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ void QGraphicsScenePrivate::ungrabMouse(QGraphicsItem *item, bool itemIsDying)
// If the item is a popup, go via removePopup to ensure state
// consistency and that it gets hidden correctly - beware that
// removePopup() reenters this function to continue removing the grab.
removePopup((QGraphicsWidget *)item, itemIsDying);
removePopup(popupWidgets.constLast(), itemIsDying);
return;
}

Expand Down

0 comments on commit 622681e

Please sign in to comment.