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 <erik.verbruggen@qt.io>

7 files changed, 49 insertions, 7 deletions

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<quint32> componentRoots;
 
-    QHash<int, int> _idToObjectIndex;
+    // Deliberate choice of map over hash here to ensure stable generated output.
+    QMap<int, int> _idToObjectIndex;
     QVector<int> _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 <typename T>
 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<const char *>(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<int, TypeReference>
 {
     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 <private/qv4value_p.h>
 #include <private/qv4alloca_p.h>
 #include <wtf/MathExtras.h>
+#include <QCryptographicHash>
 
 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<char *>(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<quint32> *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();