From 9cdb0ec22ec47dac7a0e440bd93ab7d3d0cb6a9f Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 8 Sep 2017 17:35:28 +0200 Subject: [PATCH] QmlProfiler: Don't trust in externally provided timestamps We might get timestamps that are outside the trace time, negative timestamps, ranges that go backwards, and other insane things. In order to deal with this, we clamp all negative timestamps to 0, and treat the specified trace time as minimum range, that can be overridden by events. Change-Id: Iba661f2a4346077871fc62a46759e169b2aad49d Reviewed-by: Milian Wolff --- .../qmlprofiler/qmlprofilermodelmanager.cpp | 23 ++++++++++++++----- .../qmlprofiler/qmlprofilermodelmanager.h | 2 +- .../qmlprofiler/qmlprofilertraceclient.cpp | 2 +- .../qmlprofiler/qmlprofilertracefile.cpp | 9 +++++--- src/plugins/qmlprofiler/qmltypedevent.cpp | 2 +- .../tests/pixmapcachemodel_test.cpp | 3 ++- 6 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp b/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp index 2b476e1b2f1..7c85817841e 100644 --- a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp +++ b/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp @@ -92,18 +92,23 @@ bool QmlProfilerTraceTime::isRestrictedToRange() const void QmlProfilerTraceTime::clear() { restrictToRange(-1, -1); - setTime(-1, -1); + m_startTime = -1; + m_endTime = -1; } -void QmlProfilerTraceTime::setTime(qint64 startTime, qint64 endTime) +void QmlProfilerTraceTime::update(qint64 time) { - QTC_ASSERT(startTime <= endTime, endTime = startTime); - m_startTime = startTime; - m_endTime = endTime; + QTC_ASSERT(time >= 0, return); + if (m_startTime > time || m_startTime == -1) + m_startTime = time; + if (m_endTime < time || m_endTime == -1) + m_endTime = time; + QTC_ASSERT(m_endTime >= m_startTime, m_startTime = m_endTime); } void QmlProfilerTraceTime::decreaseStartTime(qint64 time) { + QTC_ASSERT(time >= 0, return); if (m_startTime > time || m_startTime == -1) { m_startTime = time; if (m_endTime == -1) @@ -115,6 +120,7 @@ void QmlProfilerTraceTime::decreaseStartTime(qint64 time) void QmlProfilerTraceTime::increaseEndTime(qint64 time) { + QTC_ASSERT(time >= 0, return); if (m_endTime < time || m_endTime == -1) { m_endTime = time; if (m_startTime == -1) @@ -244,6 +250,7 @@ void QmlProfilerModelManager::addEvents(const QVector &events) { for (const QmlEvent &event : events) { d->eventStream << event; + d->traceTime->update(event.timestamp()); d->dispatch(event, d->eventTypes[event.typeIndex()]); } } @@ -251,6 +258,7 @@ void QmlProfilerModelManager::addEvents(const QVector &events) void QmlProfilerModelManager::addEvent(const QmlEvent &event) { d->eventStream << event; + d->traceTime->update(event.timestamp()); QTC_ASSERT(event.typeIndex() < d->eventTypes.size(), d->eventTypes.resize(event.typeIndex() + 1)); d->dispatch(event, d->eventTypes.at(event.typeIndex())); @@ -606,7 +614,10 @@ void QmlProfilerModelManager::load(const QString &filename) this, &QmlProfilerModelManager::addEvents); connect(reader, &QmlProfilerFileReader::success, this, [this, reader]() { - d->traceTime->setTime(reader->traceStart(), reader->traceEnd()); + if (reader->traceStart() >= 0) + d->traceTime->decreaseStartTime(reader->traceStart()); + if (reader->traceEnd() >= 0) + d->traceTime->increaseEndTime(reader->traceEnd()); setRecordedFeatures(reader->loadedFeatures()); delete reader; acquiringDone(); diff --git a/src/plugins/qmlprofiler/qmlprofilermodelmanager.h b/src/plugins/qmlprofiler/qmlprofilermodelmanager.h index 6da39ed93b4..f09537667e5 100644 --- a/src/plugins/qmlprofiler/qmlprofilermodelmanager.h +++ b/src/plugins/qmlprofiler/qmlprofilermodelmanager.h @@ -58,7 +58,7 @@ public: void clear(); - void setTime(qint64 startTime, qint64 endTime); + void update(qint64 time); void decreaseStartTime(qint64 time); void increaseEndTime(qint64 time); void restrictToRange(qint64 startTime, qint64 endTime); diff --git a/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp b/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp index 0ed31a2036b..39cdb96ede0 100644 --- a/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp +++ b/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp @@ -247,7 +247,7 @@ void QmlProfilerTraceClient::setRequestedFeatures(quint64 features) const QmlDebug::QDebugContextInfo &context) { d->updateFeatures(ProfileDebugMessages); - d->currentEvent.event.setTimestamp(context.timestamp); + d->currentEvent.event.setTimestamp(context.timestamp > 0 ? context.timestamp : 0); d->currentEvent.event.setTypeIndex(-1); d->currentEvent.event.setString(text); d->currentEvent.type = QmlEventType(DebugMessage, MaximumRangeType, type, diff --git a/src/plugins/qmlprofiler/qmlprofilertracefile.cpp b/src/plugins/qmlprofiler/qmlprofilertracefile.cpp index b84de415ae7..710461b6565 100644 --- a/src/plugins/qmlprofiler/qmlprofilertracefile.cpp +++ b/src/plugins/qmlprofiler/qmlprofilertracefile.cpp @@ -257,6 +257,8 @@ void QmlProfilerFileReader::loadQzt(QIODevice *device) return; } m_loadedFeatures |= (1ULL << m_eventTypes[event.typeIndex()].feature()); + if (event.timestamp() < 0) + event.setTimestamp(0); } else if (bufferStream.status() == QDataStream::ReadPastEnd) { break; // Apparently EOF is a character so we end up here after the last event. } else if (bufferStream.status() == QDataStream::ReadCorruptData) { @@ -506,15 +508,16 @@ void QmlProfilerFileReader::loadEvents(QXmlStreamReader &stream) continue; } - event.setTimestamp(attributes.value(_("startTime")).toLongLong()); + const qint64 timestamp = attributes.value(_("startTime")).toLongLong(); + event.setTimestamp(timestamp > 0 ? timestamp : 0); event.setTypeIndex(attributes.value(_("eventIndex")).toInt()); if (attributes.hasAttribute(_("duration"))) { event.setRangeStage(RangeStart); QmlEvent rangeEnd(event); rangeEnd.setRangeStage(RangeEnd); - rangeEnd.setTimestamp(event.timestamp() - + attributes.value(_("duration")).toLongLong()); + const qint64 duration = attributes.value(_("duration")).toLongLong(); + rangeEnd.setTimestamp(event.timestamp() + (duration > 0 ? duration : 0)); events.addRange(event, rangeEnd); } else { // attributes for special events diff --git a/src/plugins/qmlprofiler/qmltypedevent.cpp b/src/plugins/qmlprofiler/qmltypedevent.cpp index e5007345b5f..58d7aae8d75 100644 --- a/src/plugins/qmlprofiler/qmltypedevent.cpp +++ b/src/plugins/qmlprofiler/qmltypedevent.cpp @@ -44,7 +44,7 @@ QDataStream &operator>>(QDataStream &stream, QmlTypedEvent &event) subtype = -1; } - event.event.setTimestamp(time); + event.event.setTimestamp(time > 0 ? time : 0); event.event.setTypeIndex(-1); event.serverTypeId = 0; diff --git a/src/plugins/qmlprofiler/tests/pixmapcachemodel_test.cpp b/src/plugins/qmlprofiler/tests/pixmapcachemodel_test.cpp index 0b4d39fdc61..903e2b13320 100644 --- a/src/plugins/qmlprofiler/tests/pixmapcachemodel_test.cpp +++ b/src/plugins/qmlprofiler/tests/pixmapcachemodel_test.cpp @@ -38,7 +38,8 @@ PixmapCacheModelTest::PixmapCacheModelTest(QObject *parent) : QObject(parent), void PixmapCacheModelTest::initTestCase() { manager.startAcquiring(); - manager.traceTime()->setTime(1, 300); + manager.traceTime()->decreaseStartTime(1); + manager.traceTime()->increaseEndTime(300); for (int i = 0; i < MaximumPixmapEventType; ++i) { eventTypeIndices[i] = manager.numLoadedEventTypes();