Fix memory corruption in QML expression compilation

We store QQmlPropertyData pointers in our IR for Qt meta-object property
resolution at compile time. As it turns out however, it is possible that these
pointers change after retrieval from the QQmlPropertyCache, as the cache may
change later in the compilation process. Therefore we must do what also
QQmlCompiler does by storing a copy of the QQmlPropertyData. For the JS IR we
can do that conveniently through the IR memory pool.

A side-effect of this bug was that QQmlPropertyData pointers were re-used
and so the identity check in the isel later such as

    _function->contextObjectDependencies.contains(m->property)

for dependency tracking failed. In the example given in the bug report it was
determined that the window.contentWidth property wouldn't need a property
capture, and therefore the binding was not re-evaluated as window.contentWidth
later in the binding evaluation phase received its correct value.

This patch also fixes the incorrect debug output names assigned to JS binding
expressions, where the index used to look up the name is per compiled object,
not per QML component.

Task-number: QTBUG-35063
Change-Id: I3e5bbfaac11e5c122a2ed15a3e486a93988e1b6e
Reviewed-by: J-P Nurmi <jpnurmi@digia.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

4 files changed, 13 insertions, 5 deletions

diff --git a/src/qml/compiler/qqmlcodegenerator.cpp b/src/qml/compiler/qqmlcodegenerator.cpp
index c16dd5daea..2215551b95 100644
--- a/src/qml/compiler/qqmlcodegenerator.cpp
+++ b/src/qml/compiler/qqmlcodegenerator.cpp
@@ -1292,7 +1292,7 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<AST::N
     return runtimeFunctionIndices;
 }
 
-static QQmlPropertyData *lookupQmlCompliantProperty(QQmlPropertyCache *cache, const QString &name, bool *propertyExistsButForceNameLookup = 0)
+QQmlPropertyData *JSCodeGen::lookupQmlCompliantProperty(QQmlPropertyCache *cache, const QString &name, bool *propertyExistsButForceNameLookup)
 {
     if (propertyExistsButForceNameLookup)
         *propertyExistsButForceNameLookup = false;
@@ -1308,6 +1308,13 @@ static QQmlPropertyData *lookupQmlCompliantProperty(QQmlPropertyCache *cache, co
     if (pd && !cache->isAllowedInRevision(pd))
         pd = 0;
 
+    // Return a copy allocated from our memory pool. Property data pointers can change
+    // otherwise when the QQmlPropertyCache changes later in the QML type compilation process.
+    if (pd) {
+        QQmlPropertyData *copy = pd;
+        pd = _function->New<QQmlPropertyData>();
+        *pd = *copy;
+    }
     return pd;
 }
 
diff --git a/src/qml/compiler/qqmlcodegenerator_p.h b/src/qml/compiler/qqmlcodegenerator_p.h
index 1cc5b1e2f6..a5fec65111 100644
--- a/src/qml/compiler/qqmlcodegenerator_p.h
+++ b/src/qml/compiler/qqmlcodegenerator_p.h
@@ -371,6 +371,8 @@ protected:
     virtual V4IR::Expr *fallbackNameLookup(const QString &name, int line, int col);
 
 private:
+    QQmlPropertyData *lookupQmlCompliantProperty(QQmlPropertyCache *cache, const QString &name, bool *propertyExistsButForceNameLookup = 0);
+
     QQmlEnginePrivate *engine;
     QString sourceCode;
     QQmlJS::Engine *jsEngine; // needed for memory pool
diff --git a/src/qml/qml/qqmlcompiler.cpp b/src/qml/qml/qqmlcompiler.cpp
index 7b33849e67..93ec2516c8 100644
--- a/src/qml/qml/qqmlcompiler.cpp
+++ b/src/qml/qml/qqmlcompiler.cpp
@@ -3630,8 +3630,6 @@ bool QQmlCompiler::completeComponentBuild()
     QQmlJS::Engine *jsEngine = parser.jsEngine();
     QQmlJS::MemoryPool *pool = jsEngine->pool();
 
-    QHash<int, QString> expressionNames;
-
     for (JSBindingReference *b = compileState->bindings.first(); b; b = b->nextReference) {
 
         JSBindingReference &binding = *b;
@@ -3648,7 +3646,7 @@ bool QQmlCompiler::completeComponentBuild()
         ComponentCompileState::PerObjectCompileData *cd = &compileState->jsCompileData[b->bindingContext.object];
         cd->functionsToCompile.append(node);
         binding.compiledIndex = cd->functionsToCompile.count() - 1;
-        expressionNames.insert(binding.compiledIndex, binding.property->name().toString().prepend(QStringLiteral("expression for ")));
+        cd->expressionNames.insert(binding.compiledIndex, binding.property->name().toString().prepend(QStringLiteral("expression for ")));
 
         if (componentStats)
             componentStats->componentStat.scriptBindings.append(b->value->location);
@@ -3681,7 +3679,7 @@ bool QQmlCompiler::completeComponentBuild()
 
             jsCodeGen.beginObjectScope(scopeObject->metatype);
 
-            cd->runtimeFunctionIndices = jsCodeGen.generateJSCodeForFunctionsAndBindings(cd->functionsToCompile, expressionNames);
+            cd->runtimeFunctionIndices = jsCodeGen.generateJSCodeForFunctionsAndBindings(cd->functionsToCompile, cd->expressionNames);
             QList<QQmlError> errors = jsCodeGen.errors();
             if (!errors.isEmpty()) {
                 exceptions << errors;
diff --git a/src/qml/qml/qqmlcompiler_p.h b/src/qml/qml/qqmlcompiler_p.h
index 2e3e6b8f4c..3ca4566e41 100644
--- a/src/qml/qml/qqmlcompiler_p.h
+++ b/src/qml/qml/qqmlcompiler_p.h
@@ -315,6 +315,7 @@ namespace QQmlCompilerTypes {
             QList<QQmlJS::AST::Node*> functionsToCompile;
             QVector<int> runtimeFunctionIndices;
             QVector<CompiledMetaMethod> compiledMetaMethods;
+            QHash<int, QString> expressionNames;
         };
         QHash<QQmlScript::Object *, PerObjectCompileData> jsCompileData;
     };