[PATCH 0/2] Capture and display frame thumbnails in qapitrace. (Pass 2)

[Dan McCabe]

On 03/11/2012 07:36 AM, José Fonseca wrote:
> On Tue, Mar 6, 2012 at 1:20 AM, Dan McCabe<zen3d.linux at gmail.com>  wrote:
>> This patch set automatically captures and displays thumbnails for each
>> frame in qapitrace.
>>
>> qapitrace spawns glretrace in a separate process to replay traces and
>> capture relevent information about those traces. In the first patch, a
>> sequence of snapshots is generated and captured thumbnails. In the second
>> patch, those thumbnails are associated with the approproate ApiTraceFrame.
>> When the ApiTraceEvent is processed for each frame, the thumbnail is
>> displayed at the front of the frame's information.
>>
>> Deltas from the previous iteration of the patch set:
>>
>> 1) Replaced readAllStandardOutput with QProcess::setReadChannel() and
>> QProcess::read() while capturing snapshot stream. This eliminates the
>> out-of-memory exceptions I was encountering in my previous patch set.
>>
>> 2) Testing for going past end of QList in bindThumbnailsToFrames. This
>> should eliminate asset failures that Jose encountered (but which I never
>> saw).
> Dan, I'm now seeing a slightly different assertion failure:
>
> ASSERT failure in QList<T>::operator[]: "index out of range", file
> /usr/include/qt4/QtCore/qlist.h, line 460
>
> Program received signal SIGABRT, Aborted.
> 0x00007ffff3830475 in *__GI_raise (sig=<optimized out>) at
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> 64	../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
> (gdb) bt
> #0  0x00007ffff3830475 in *__GI_raise (sig=<optimized out>) at
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> #1  0x00007ffff38336f0 in *__GI_abort () at abort.c:92
> #2  0x00007ffff4392571 in qt_message_output(QtMsgType, char const*) ()
> from /usr/lib/libQtCore.so.4
> #3  0x00007ffff43928ef in ?? () from /usr/lib/libQtCore.so.4
> #4  0x00007ffff4392a94 in qFatal(char const*, ...) () from
> /usr/lib/libQtCore.so.4
> #5  0x000000000043121f in QList<ApiTraceFrame*>::operator[]
> (this=0x85a3b0, i=0) at /usr/include/qt4/QtCore/qlist.h:460
> #6  0x000000000042f62d in ApiTrace::callInFrame (this=0x85a390,
> callIdx=6) at /home/jfonseca/projects/apitrace/gui/apitrace.cpp:454
> #7  0x000000000042f6e7 in ApiTrace::setCallError (this=0x85a390,
> error=...) at /home/jfonseca/projects/apitrace/gui/apitrace.cpp:471
> #8  0x0000000000462bf6 in MainWindow::slotRetraceErrors
> (this=0x7fffffffdd70, errors=...)
>      at /home/jfonseca/projects/apitrace/gui/mainwindow.cpp:1047
> #9  0x0000000000464784 in MainWindow::qt_metacall
> (this=0x7fffffffdd70, _c=QMetaObject::InvokeMetaMethod, _id=35,
> _a=0x7fffffffd400)
>      at /home/jfonseca/projects/apitrace/gui/mainwindow.moc:179
> #10 0x00007ffff4494eba in QMetaObject::activate(QObject*, QMetaObject
> const*, int, void**) () from /usr/lib/libQtCore.so.4
> #11 0x000000000046fcb9 in Retracer::retraceErrors (this=0x81eb80,
> _t1=...) at /home/jfonseca/projects/apitrace/gui/retracer.moc:248
> #12 0x000000000046fb3c in Retracer::qt_metacall (this=0x81eb80,
> _c=QMetaObject::InvokeMetaMethod, _id=4, _a=0x903270)
>      at /home/jfonseca/projects/apitrace/gui/retracer.moc:207
> #13 0x00007ffff4498a6a in QObject::event(QEvent*) () from
> /usr/lib/libQtCore.so.4
> #14 0x00007ffff4cbcc64 in QApplicationPrivate::notify_helper(QObject*,
> QEvent*) () from /usr/lib/libQtGui.so.4
> #15 0x00007ffff4cc1af1 in QApplication::notify(QObject*, QEvent*) ()
> from /usr/lib/libQtGui.so.4
> #16 0x00007ffff448228c in QCoreApplication::notifyInternal(QObject*,
> QEvent*) () from /usr/lib/libQtCore.so.4
> #17 0x00007ffff4485648 in
> QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*)
> () from /usr/lib/libQtCore.so.4
> #18 0x00007ffff44ac9d3 in ?? () from /usr/lib/libQtCore.so.4
> #19 0x00007ffff1e950cf in g_main_context_dispatch () from
> /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #20 0x00007ffff1e958c8 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #21 0x00007ffff1e95a99 in g_main_context_iteration () from
> /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #22 0x00007ffff44ace2f in
> QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>)
> () from /usr/lib/libQtCore.so.4
> #23 0x00007ffff4d60eee in ?? () from /usr/lib/libQtGui.so.4
> #24 0x00007ffff4481492 in
> QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
> from /usr/lib/libQtCore.so.4
> #25 0x00007ffff448168f in
> QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from
> /usr/lib/libQtCore.so.4
> #26 0x00007ffff4485837 in QCoreApplication::exec() () from
> /usr/lib/libQtCore.so.4
> #27 0x000000000046cd7a in main (argc=2, argv=0x7fffffffe198) at
> /home/jfonseca/projects/apitrace/gui/main.cpp:68
> (gdb)
>
> I don't have time to debug this myself right now. But from the looks
> of it, I get the feeling that glretrace is being invoked before the
> trace is fully scanned.
>
> Jose

Hi Jose,


Now that I finally have some time to look at your stack trace on the 
assert failure, it appears to me that the failure to trace exposed a 
latent bug that was there previously. I don't know whether qapitrace 
would have behaved reasonably in the absence of that latent bug or not, 
since I can't reproduce your case.

It appears to me that what is going on is that line 453 in apitrace.cpp 
is incorrect. That line reads:
         for (int frameIdx = 0; frameIdx <= m_frames.size(); ++frameIdx) {
and it allows the index to go beyond the end of the m_frames QList array 
on line 454, which corresponds to stack frame #6 above (I'm assuming 
that Qt is rational and defines array-like index behavior to go from 0 
to n-1; since I'm not that familiar with Qt, this might not be a correct 
assumption).

I believe that line 453 in apitrace.cpp should have been written like:
     for (int frameIdx = 0; frameIdx < m_frames.size(); ++frameIdx) {
replacing "<=" with "<".

For a reasonable input to callInFrame(), this might never be observed. 
But perhaps the failure to trace perhaps callInFrame() its first ever 
invalid input?

Going a step further, it appears that retracing can fail in ways that 
weren't envisioned by the error handler in 
retracer.cpp:RetraceProcess::replayFinished(). When that procedure 
encounters an error, lines 265-276 assume a specific form for the error 
messages and try to parse those messages. However, that code doesn't 
deal correctly with messages that aren't in that format.

Please let me know if changing the for loop in apitrace.cpp addresses 
the assertion failure on your system. And please let me know what the 
new failure mode looks like (since I can't reproduce it :).

I will also make the failure handling code in retracer.cpp more robust.

Thanks.

cheers, danm