Opened 13 years ago

Closed 13 years ago

Last modified 11 years ago

#2725 closed defect (invalid)

dynamic_cast can return NULL, must test

Reported by: Nigel Owned by: Nigel
Priority: minor Milestone: unknown
Component: mythtv Version: 0.20
Severity: medium Keywords:
Cc: Ticket locked: yes

Description (last modified by Nigel)

There are a few places in the code where the result of dynamic_cast isn't tested before it is used. Sadly, in some cases, it can return NULL, which causes a bus error or SEGV.
One particuar problem is in the keypress event processing. Causes a crash on OS X binaries built on 10.4 (gcc4), but running on 10.3. It is an old problem, usually caused by incorrectly compiling with -fno-rtti: http://www.gossamer-threads.com/lists/mythtv/dev/72988#72988
Some documents (e.g. http://www.cplusplus.com/doc/tutorial/typecasting.html) say that dynamic_cast ing a base class to a subclass is illegal. QKeyEvent is a base class of QEvent, so if I believe this, it is the problem here.

Change History (4)

comment:1 Changed 13 years ago by Isaac Richards

Resolution: invalid
Status: newclosed

Those casts in the event handler are absolutely guaranteed to succeed there, unless something is _seriously_ wrong with the compiler or Qt. The type has already been checked by the case statement, and those are valid uses of dynamic_cast (though not really necessary).

comment:2 Changed 13 years ago by Nigel

Description: modified (diff)

comment:3 Changed 13 years ago by Nigel

I suspect there is something seriously wrong with the GCC vtable parsing, but I haven't found it. The following:

Index: mythmainwindow.cpp
===================================================================
--- mythmainwindow.cpp  (revision 11820)
+++ mythmainwindow.cpp  (working copy)
@@ -1089,9 +1089,32 @@
     {
         case QEvent::KeyPress:
         {
+            QKeyEvent *ke = dynamic_cast<QKeyEvent*>(e);
+
+            if (!ke)
+            {
+                //puts("Dynamic_cast of a QtEvent failed!!!");
+
+                // In some strange situations, e is already a QKeyEvent
+                ke = new QKeyEvent(QEvent::KeyPress, 0, 0, 0);
+
+                if (strcmp(typeid(*e).name(), typeid(*ke).name()) == 0)
+                {
+                    delete ke;
+
+                    // Same typeid? Safe to do a plain-old cast:
+                    ke = static_cast<QKeyEvent*>(e);
+                }
+                else
+                {
+                    delete ke;
+                    //puts("Did you use -fno-rtti to build?");
+                    return false;
+                }
+            }
+

works. Some debuging reveals typeid(e).name() == "P9QKeyEvent" (QKeyEvent*), and typeid(*e).name() == "9QKeyEvent" (QKeyEvent). Freaking weird.
A few other random dynamic_cast values that aren't checked: libmyth/settings.cpp line 611, 671, 718, 765, 796, 1402, libmythtv/darwinfirewirerecorder.cpp line 17, libmythtv/diseqcsettings.cpp line 1362-3, 1375-6, 1388-89, 1400, 1411, 1422-23, 1434-35, 1447-48, 1473-74

comment:5 Changed 11 years ago by stuartm

Ticket locked: set
Note: See TracTickets for help on using tickets.