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 <ogoffart@woboq.com>

1 files 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 <algorithm>
+
 QT_BEGIN_NAMESPACE
 
 
@@ -275,12 +277,13 @@ bool QAnimationGroup::event(QEvent *event)
         }
     } else if (event->type() == QEvent::ChildRemoved) {
         QChildEvent *childEvent = static_cast<QChildEvent *>(event);
-        QAbstractAnimation *a = static_cast<QAbstractAnimation *>(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<QAbstractAnimation *>::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);
 }