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>

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;
 }
 

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;

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
 
 }

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();
 };

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));

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) {

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();

tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp

@@ -56,6 +56,7 @@ private slots:
     void fileSelectors();
     void localAliases();
     void cacheResources();
+    void stableOrderOfDependentCompositeTypes();
 };
 
 // A wrapper around QQmlComponent to ensure the temporary reference counts
@@ -566,6 +567,105 @@ void tst_qmldiskcache::cacheResources()
     }
 }
 
+void tst_qmldiskcache::stableOrderOfDependentCompositeTypes()
+{
+    QQmlEngine engine;
+
+    QTemporaryDir tempDir;
+    QVERIFY(tempDir.isValid());
+
+    {
+        const QString depFilePath = tempDir.path() + "/FirstDependentType.qml";
+        QFile f(depFilePath);
+        QVERIFY2(f.open(QIODevice::WriteOnly), qPrintable(f.errorString()));
+        f.write(QByteArrayLiteral("import QtQml 2.0\nQtObject { property int value: 42 }"));
+    }
+
+    {
+        const QString depFilePath = tempDir.path() + "/SecondDependentType.qml";
+        QFile f(depFilePath);
+        QVERIFY2(f.open(QIODevice::WriteOnly), qPrintable(f.errorString()));
+        f.write(QByteArrayLiteral("import QtQml 2.0\nQtObject { property int value: 100 }"));
+    }
+
+    const QString testFilePath = tempDir.path() + "/main.qml";
+    {
+        QFile f(testFilePath);
+        QVERIFY2(f.open(QIODevice::WriteOnly), qPrintable(f.errorString()));
+        f.write(QByteArrayLiteral("import QtQml 2.0\nQtObject {\n"
+                                  "    property QtObject dep1: FirstDependentType{}\n"
+                                  "    property QtObject dep2: SecondDependentType{}\n"
+                                  "    property int value: dep1.value + dep2.value\n"
+                                  "}"));
+    }
+
+    QByteArray firstDependentTypeClassName;
+    QByteArray secondDependentTypeClassName;
+
+    {
+        CleanlyLoadingComponent component(&engine, QUrl::fromLocalFile(testFilePath));
+        QScopedPointer<QObject> obj(component.create());
+        QVERIFY(!obj.isNull());
+        QCOMPARE(obj->property("value").toInt(), 142);
+
+        firstDependentTypeClassName = qvariant_cast<QObject *>(obj->property("dep1"))->metaObject()->className();
+        secondDependentTypeClassName = qvariant_cast<QObject *>(obj->property("dep2"))->metaObject()->className();
+    }
+
+    QVERIFY(firstDependentTypeClassName != secondDependentTypeClassName);
+    QVERIFY2(firstDependentTypeClassName.contains("QMLTYPE"), firstDependentTypeClassName.constData());
+    QVERIFY2(secondDependentTypeClassName.contains("QMLTYPE"), secondDependentTypeClassName.constData());
+
+    const QString testFileCachePath = testFilePath + QLatin1Char('c');
+    QVERIFY(QFile::exists(testFileCachePath));
+    QDateTime initialCacheTimeStamp = QFileInfo(testFileCachePath).lastModified();
+
+    engine.clearComponentCache();
+    waitForFileSystem();
+
+    // Creating the test component a second time should load it from the cache (same time stamp),
+    // despite the class names of the dependent composite types differing.
+    {
+        CleanlyLoadingComponent component(&engine, QUrl::fromLocalFile(testFilePath));
+        QScopedPointer<QObject> obj(component.create());
+        QVERIFY(!obj.isNull());
+        QCOMPARE(obj->property("value").toInt(), 142);
+
+        QVERIFY(qvariant_cast<QObject *>(obj->property("dep1"))->metaObject()->className() != firstDependentTypeClassName);
+        QVERIFY(qvariant_cast<QObject *>(obj->property("dep2"))->metaObject()->className() != secondDependentTypeClassName);
+    }
+
+    {
+        QVERIFY(QFile::exists(testFileCachePath));
+        QDateTime newCacheTimeStamp = QFileInfo(testFileCachePath).lastModified();
+        QCOMPARE(newCacheTimeStamp, initialCacheTimeStamp);
+    }
+
+    // Now change the first dependent QML type and see if we correctly re-generate the
+    // caches.
+    engine.clearComponentCache();
+    waitForFileSystem();
+    {
+        const QString depFilePath = tempDir.path() + "/FirstDependentType.qml";
+        QFile f(depFilePath);
+        QVERIFY2(f.open(QIODevice::WriteOnly), qPrintable(f.errorString()));
+        f.write(QByteArrayLiteral("import QtQml 2.0\nQtObject { property int value: 40 }"));
+    }
+
+    {
+        CleanlyLoadingComponent component(&engine, QUrl::fromLocalFile(testFilePath));
+        QScopedPointer<QObject> obj(component.create());
+        QVERIFY(!obj.isNull());
+        QCOMPARE(obj->property("value").toInt(), 140);
+    }
+
+    {
+        QVERIFY(QFile::exists(testFileCachePath));
+        QDateTime newCacheTimeStamp = QFileInfo(testFileCachePath).lastModified();
+        QVERIFY2(newCacheTimeStamp > initialCacheTimeStamp, qPrintable(newCacheTimeStamp.toString()));
+    }
+}
+
 QTEST_MAIN(tst_qmldiskcache)
 
 #include "tst_qmldiskcache.moc"