From 84033f550f5f31f6cb7609c9edacb11f51802649 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sun, 15 Sep 2013 03:18:11 +0200 Subject: [PATCH] QAnimationGroup: avoid undefined behavior The existing code performed a downcast from QObject* to QAbstractAnimation* at a time when the former QAbstractAnimation only is a QObject anymore. The comment indicates that this was understood at the time of writing (or else a little later), but it drew the wrong conclusions. Statically downcasting a type whose dynamic type is (no longer) of the target type is undefined behavior. This change fixes the code to (implicitly) cast _up_ to QObject and compare at that level. Says Clang: src/corelib/animation/qanimationgroup.cpp:278:33: runtime error: downcast of address 0x000002966ab0 which does not point to an object of type 'QAbstractAnimation' 0x000002966ab0: note: object is of type 'QObject' 00 00 00 00 80 d7 e6 d7 88 2b 00 00 70 7b 96 02 00 00 00 00 61 00 6e 00 6e 00 6f 00 21 00 00 00 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QObject' Change-Id: I51d6277020d0ff32cd7b80a8cddcf2cda1a626a6 Reviewed-by: Olivier Goffart --- src/corelib/animation/qanimationgroup.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/corelib/animation/qanimationgroup.cpp b/src/corelib/animation/qanimationgroup.cpp index 07989790e4..23eab52818 100644 --- a/src/corelib/animation/qanimationgroup.cpp +++ b/src/corelib/animation/qanimationgroup.cpp @@ -90,6 +90,8 @@ #ifndef QT_NO_ANIMATION +#include + QT_BEGIN_NAMESPACE @@ -275,12 +277,13 @@ bool QAnimationGroup::event(QEvent *event) } } else if (event->type() == QEvent::ChildRemoved) { QChildEvent *childEvent = static_cast(event); - QAbstractAnimation *a = static_cast(childEvent->child()); // You can only rely on the child being a QObject because in the QEvent::ChildRemoved - // case it might be called from the destructor. - int index = d->animations.indexOf(a); - if (index != -1) - takeAnimation(index); + // case it might be called from the destructor. Casting down to QAbstractAnimation then + // entails undefined behavior, so compare items as QObjects (which std::find does internally): + const QList::const_iterator it + = std::find(d->animations.cbegin(), d->animations.cend(), childEvent->child()); + if (it != d->animations.cend()) + takeAnimation(it - d->animations.cbegin()); } return QAbstractAnimation::event(event); }