From 2c2c92d999ae8584840e44785fbece9bf2cfac62 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Sun, 16 Oct 2016 17:54:59 +0200 Subject: Fix excessive invalidation of QML disk caches We use an MD5 checksum over the meta-object data to verify that the types a QML file depends on haven't changed since the cache was generated. However when the dependent types are QML types, then the meta-object data contains dynamically generated type names such as QMLTYPE_1234, which is non-deterministic. To address this, we resort to the checksum over the meta-object data only for C++ types (if those change it's likely an incompatible change) and for QML types use the fact that all the information about the QML declared types comes from the QML file only, which means we can in that case simply use a checksum over the QV4::CompiledData memory chunk. In addition we need to ensure that the generated CompiledData memory chunk is deterministic by avoiding any uninitialized bytes (memset) and using a map instead of a hash for the mapping of object index to object id. Task-number: QTBUG-55926 Change-Id: I27c840b1960ad36b486198e504b70989c22a3972 Reviewed-by: Erik Verbruggen --- src/qml/compiler/qqmlirbuilder.cpp | 2 ++ src/qml/compiler/qqmltypecompiler_p.h | 3 ++- src/qml/compiler/qv4compileddata.cpp | 30 ++++++++++++++++++++++++++---- src/qml/compiler/qv4compileddata_p.h | 8 ++++++-- src/qml/compiler/qv4compiler.cpp | 6 ++++++ src/qml/qml/qqmlpropertycache.cpp | 6 ++++++ src/qml/qml/qqmltypeloader.cpp | 1 + 7 files changed, 49 insertions(+), 7 deletions(-) (limited to 'src/qml') diff --git a/src/qml/compiler/qqmlirbuilder.cpp b/src/qml/compiler/qqmlirbuilder.cpp index 31b964897f..eb83962630 100644 --- a/src/qml/compiler/qqmlirbuilder.cpp +++ b/src/qml/compiler/qqmlirbuilder.cpp @@ -1531,6 +1531,8 @@ QV4::CompiledData::Unit *QmlUnitGenerator::generate(Document &output, QQmlEngine output.jsGenerator.stringTable.serialize(qmlUnit); + qmlUnit->generateChecksum(); + return qmlUnit; } diff --git a/src/qml/compiler/qqmltypecompiler_p.h b/src/qml/compiler/qqmltypecompiler_p.h index 6ad6ad8557..de6abb4ced 100644 --- a/src/qml/compiler/qqmltypecompiler_p.h +++ b/src/qml/compiler/qqmltypecompiler_p.h @@ -285,7 +285,8 @@ protected: // indices of the objects that are actually Component {} QVector componentRoots; - QHash _idToObjectIndex; + // Deliberate choice of map over hash here to ensure stable generated output. + QMap _idToObjectIndex; QVector _objectsWithAliases; QV4::CompiledData::ResolvedTypeReferenceMap *resolvedTypes; diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp index 35f61b4f69..0ab09f66ec 100644 --- a/src/qml/compiler/qv4compileddata.cpp +++ b/src/qml/compiler/qv4compileddata.cpp @@ -571,6 +571,17 @@ QQmlPropertyCache *ResolvedTypeReference::createPropertyCache(QQmlEngine *engine } } +bool ResolvedTypeReference::addToHash(QCryptographicHash *hash, QQmlEngine *engine) +{ + if (type) { + bool ok = false; + hash->addData(createPropertyCache(engine)->checksum(&ok)); + return ok; + } + hash->addData(compilationUnit->data->md5Checksum, sizeof(compilationUnit->data->md5Checksum)); + return true; +} + template bool qtTypeInherits(const QMetaObject *mo) { while (mo) { @@ -596,15 +607,26 @@ void ResolvedTypeReference::doDynamicTypeCheck() bool ResolvedTypeReferenceMap::addToHash(QCryptographicHash *hash, QQmlEngine *engine) const { for (auto it = constBegin(), end = constEnd(); it != end; ++it) { - QQmlPropertyCache *pc = it.value()->createPropertyCache(engine); - bool ok = false; - hash->addData(pc->checksum(&ok)); - if (!ok) + if (!it.value()->addToHash(hash, engine)) return false; } return true; } +void Unit::generateChecksum() +{ + QCryptographicHash hash(QCryptographicHash::Md5); + + const int checksummableDataOffset = qOffsetOf(QV4::CompiledData::Unit, md5Checksum) + sizeof(md5Checksum); + + const char *dataPtr = reinterpret_cast(this) + checksummableDataOffset; + hash.addData(dataPtr, unitSize - checksummableDataOffset); + + QByteArray checksum = hash.result(); + Q_ASSERT(checksum.size() == sizeof(md5Checksum)); + memcpy(md5Checksum, checksum.constData(), sizeof(md5Checksum)); +} + #endif } diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index fe18024070..8f68131ced 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -71,7 +71,7 @@ QT_BEGIN_NAMESPACE // Bump this whenever the compiler data structures change in an incompatible way. -#define QV4_DATA_STRUCTURE_VERSION 0x05 +#define QV4_DATA_STRUCTURE_VERSION 0x06 class QIODevice; class QQmlPropertyCache; @@ -612,6 +612,9 @@ struct Unit LEUInt32 unitSize; // Size of the Unit and any depending data. // END DO NOT CHANGE THESE FIELDS EVER + char md5Checksum[16]; // checksum of all bytes following this field. + void generateChecksum(); + LEUInt32 architectureIndex; // string index to QSysInfo::buildAbi() LEUInt32 codeGeneratorIndex; char dependencyMD5Checksum[16]; @@ -727,7 +730,7 @@ struct TypeReference bool errorWhenNotFound: 1; }; -// map from name index to location of first use +// Map from name index to location of first use. struct TypeReferenceMap : QHash { TypeReference &add(int nameIndex, const Location &loc) { @@ -791,6 +794,7 @@ struct ResolvedTypeReference QQmlPropertyCache *propertyCache() const; QQmlPropertyCache *createPropertyCache(QQmlEngine *); + bool addToHash(QCryptographicHash *hash, QQmlEngine *engine); void doDynamicTypeCheck(); }; diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index 43347d246a..e1ea3a9b88 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -44,6 +44,7 @@ #include #include #include +#include QV4::Compiler::StringTableGenerator::StringTableGenerator() { @@ -227,6 +228,7 @@ QV4::CompiledData::Unit *QV4::Compiler::JSUnitGenerator::generateUnit(GeneratorO { QV4::CompiledData::Unit tempHeader = generateHeader(option, functionOffsets, &jsClassDataOffset); dataPtr = reinterpret_cast(malloc(tempHeader.unitSize)); + memset(dataPtr, 0, tempHeader.unitSize); memcpy(&unit, &dataPtr, sizeof(CompiledData::Unit*)); memcpy(unit, &tempHeader, sizeof(tempHeader)); } @@ -270,6 +272,8 @@ QV4::CompiledData::Unit *QV4::Compiler::JSUnitGenerator::generateUnit(GeneratorO if (option == GenerateWithStringTable) stringTable.serialize(unit); + unit->generateChecksum(); + return unit; } @@ -363,11 +367,13 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::IR::Function *i QV4::CompiledData::Unit QV4::Compiler::JSUnitGenerator::generateHeader(QV4::Compiler::JSUnitGenerator::GeneratorOption option, QJsonPrivate::q_littleendian *functionOffsets, uint *jsClassDataOffset) { CompiledData::Unit unit; + memset(&unit, 0, sizeof(unit)); memcpy(unit.magic, CompiledData::magic_str, sizeof(unit.magic)); unit.flags = QV4::CompiledData::Unit::IsJavascript; unit.flags |= irModule->unitFlags; unit.version = QV4_DATA_STRUCTURE_VERSION; unit.qtVersion = QT_VERSION; + memset(unit.md5Checksum, 0, sizeof(unit.md5Checksum)); unit.architectureIndex = registerString(QSysInfo::buildAbi()); unit.codeGeneratorIndex = registerString(codeGeneratorName); memset(unit.dependencyMD5Checksum, 0, sizeof(unit.dependencyMD5Checksum)); diff --git a/src/qml/qml/qqmlpropertycache.cpp b/src/qml/qml/qqmlpropertycache.cpp index f75ab9e25b..2610a807b5 100644 --- a/src/qml/qml/qqmlpropertycache.cpp +++ b/src/qml/qml/qqmlpropertycache.cpp @@ -1429,6 +1429,12 @@ QByteArray QQmlPropertyCache::checksum(bool *ok) return _checksum; } + // Generate a checksum on the meta-object data only on C++ types. + if (!_metaObject || _ownMetaObject) { + *ok = false; + return _checksum; + } + QCryptographicHash hash(QCryptographicHash::Md5); if (_parent) { diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index 333ac430d3..09b9dcf452 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -2230,6 +2230,7 @@ void QQmlTypeData::done() // verify if any dependencies changed if we're using a cache if (m_document.isNull() && !m_compiledData->verifyChecksum(engine, resolvedTypeCache)) { + qCDebug(DBG_DISK_CACHE) << "Checksum mismatch for cached version of" << m_compiledData->url().toString(); if (!loadFromSource()) return; m_backupSourceCode.clear(); -- cgit v1.2.3