[Bug sanitizer/65769] New: [UBSAN] qt-4.6 and qt-4.7 applications using qobject_cast may crash

[Bug sanitizer/65769] New: [UBSAN] qt-4.6 and qt-4.7 applications using qobject_cast may crash

[sirl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65769

            Bug ID: 65769
           Summary: [UBSAN] qt-4.6 and qt-4.7 applications using
                    qobject_cast may crash
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sirl at gcc dot gnu.org
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org

This is actually a bug in the qt headers, which contains this undefined
construct:

template <class T>
inline T qobject_cast(QObject *object)
{
#if !defined(QT_NO_MEMBER_TEMPLATES) && !defined(QT_NO_QOBJECT_CHECK)
   
reinterpret_cast<T>(0)->qt_check_for_QOBJECT_macro(*reinterpret_cast<T>(object));
#endif
    return
static_cast<T>(reinterpret_cast<T>(0)->staticMetaObject.cast(object));
}

Here UBSAN lets the application crash like this:
/usr/include/QtCore/qobject.h:453:5: runtime error: member access within null
pointer of type 'struct QObject'                                   
Segmentation fault                                                              
What happens is that UBSAN creates 2 checks here (from gimple dump of
demos/undo/mainwindow.cpp):

  UBSAN_NULL (0B, 4B, 8);
  D.90545 = 0B;
  D.90546 = D.90545->_vptr.QObject;
  D.90547 = (long unsigned int) D.90546;
  UBSAN_VPTR (0B, D.90547, 3589049094360264299, &_ZTI8Document, 4B);

Naturally D.90546 = D.90545->_vptr.QObject leads to a NULL dereference.

3 workarounds are possible:
1. compile your qt code with -DQT_NO_QOBJECT_CHECK

2. change the affected header like this (maybe best for LTS distros like SLES11
or RHEL6):

template <class T>
inline T qobject_cast(QObject *object)
{
#if !defined(QT_NO_MEMBER_TEMPLATES) && !defined(QT_NO_QOBJECT_CHECK)
   
reinterpret_cast<T>(object)->qt_check_for_QOBJECT_macro(*reinterpret_cast<T>(object));
#endif
    return
static_cast<T>(reinterpret_cast<T>(object)->staticMetaObject.cast(object));
}

There are more occurrences in the header, but I didn't check them all. Use the
fixed qt-4.8 qobject.h as a reference.

3. Change to qt >= 4.8 which fixed this header.

If someone thinks it might be worth (from a QOI POV that instrumentation
shouldn't influence user code like that) changing UBSAN to skip the UBSAN_VPTR
check in case the UBSAN_NULL already reported the NULL, keep the bug open.
Otherwise it can be closed as INVALID.

[Bug sanitizer/65769] [UBSAN] qt-4.6 and qt-4.7 applications using qobject_cast may crash

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65769

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, guarding the UBSAN_VPTR check for non-NULL is certainly possible, but in
the common case where the pointer isn't a compile time constant nor an obvious
non-NULL value doing so would slow the sanitization down, and won't handle the
case of just using a completely bogus, but still non-NULL pointer, anyway.

[Bug sanitizer/65769] [UBSAN] qt-4.6 and qt-4.7 applications using qobject_cast may crash

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65769

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think this is a missed diagnostic in the C++ frontend if it doesn't warn
about this?