Fix QQmlExpression/QQmlScriptString/QQmlBinding crashes

In the QQmlScriptString we store the binding id and it is an index into the
runtimeFunctions array of the compilation unit. However we don't store the
compilation unit and instead in QQmlBinding and QQmlExpression try to retrieve
it from the cache via the context url (we have the context after all). That
turns out to be not a reliable way, as sometimes the URL might slightly differ
from the originally compiled cache (qrc:/// turning to qrc:/ maybe).
Consequently the type is (unnecessarily) compiled again and unfortunately not
_linked_, therefore the runtime functions array is empty. Another option is
that when the component was created from a QByteArray, then no entry exists in
the cache in the first place.

This patch addresses the problem by storing a reference to the compilation unit
in the QQmlContextData. That we can safely retrieve and it'll make sure the
compilation unit also stays alive.

In the process of that the manual reference counting was switched over to
QQmlRefCount and QQmlRefPointer for QV4::CompilationUnit.

Task-number: QTBUG-41193
Change-Id: I9111f9a3b65618e453954abcd789c039e65a94f7
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

9 files changed, 22 insertions, 45 deletions

diff --git a/src/qml/qml/ftw/qqmlrefcount_p.h b/src/qml/qml/ftw/qqmlrefcount_p.h
index 27f609bf39..3f1d252862 100644
--- a/src/qml/qml/ftw/qqmlrefcount_p.h
+++ b/src/qml/qml/ftw/qqmlrefcount_p.h
@@ -47,11 +47,12 @@
 
 #include <QtCore/qglobal.h>
 #include <QtCore/qatomic.h>
+#include <private/qtqmlglobal_p.h>
 
 QT_BEGIN_NAMESPACE
 
 
-class QQmlRefCount
+class Q_QML_PRIVATE_EXPORT QQmlRefCount
 {
 public:
     inline QQmlRefCount();
diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp
index 8b6bc81935..6033395629 100644
--- a/src/qml/qml/qqmlbinding.cpp
+++ b/src/qml/qml/qqmlbinding.cpp
@@ -93,17 +93,10 @@ QQmlBinding::QQmlBinding(const QQmlScriptString &script, QObject *obj, QQmlConte
 
     QQmlContextData *ctxtdata = QQmlContextData::get(scriptPrivate->context);
     QQmlEnginePrivate *engine = QQmlEnginePrivate::get(scriptPrivate->context->engine());
-    if (engine && ctxtdata && !ctxtdata->url.isEmpty()) {
-        QQmlTypeData *typeData = engine->typeLoader.getType(ctxtdata->url);
-        Q_ASSERT(typeData);
-
-        if (QQmlCompiledData *cdata = typeData->compiledData()) {
-            url = cdata->fileName();
-            if (scriptPrivate->bindingId != QQmlBinding::Invalid)
-                runtimeFunction = cdata->compilationUnit->runtimeFunctions.at(scriptPrivate->bindingId);
-        }
-
-        typeData->release();
+    if (engine && ctxtdata && !ctxtdata->url.isEmpty() && ctxtdata->typeCompilationUnit) {
+        url = ctxtdata->url.toString();
+        if (scriptPrivate->bindingId != QQmlBinding::Invalid)
+            runtimeFunction = ctxtdata->typeCompilationUnit->runtimeFunctions.at(scriptPrivate->bindingId);
     }
 
     setNotifyOnValueChanged(true);
diff --git a/src/qml/qml/qqmlcompileddata.cpp b/src/qml/qml/qqmlcompileddata.cpp
index eca8070b3f..22838786b6 100644
--- a/src/qml/qml/qqmlcompileddata.cpp
+++ b/src/qml/qml/qqmlcompileddata.cpp
@@ -50,7 +50,7 @@ QT_BEGIN_NAMESPACE
 
 QQmlCompiledData::QQmlCompiledData(QQmlEngine *engine)
 : engine(engine), importCache(0), metaTypeId(-1), listMetaTypeId(-1), isRegisteredWithEngine(false),
-  rootPropertyCache(0), compilationUnit(0), totalBindingsCount(0), totalParserStatusCount(0)
+  rootPropertyCache(0), totalBindingsCount(0), totalParserStatusCount(0)
 {
     Q_ASSERT(engine);
 }
@@ -92,9 +92,6 @@ QQmlCompiledData::~QQmlCompiledData()
 
     if (rootPropertyCache)
         rootPropertyCache->release();
-
-    if (compilationUnit)
-        compilationUnit->deref();
 }
 
 void QQmlCompiledData::clear()
diff --git a/src/qml/qml/qqmlcompiler_p.h b/src/qml/qml/qqmlcompiler_p.h
index 11646fc6b8..5e76533739 100644
--- a/src/qml/qml/qqmlcompiler_p.h
+++ b/src/qml/qml/qqmlcompiler_p.h
@@ -123,7 +123,7 @@ public:
     QVector<QQmlPropertyCache *> propertyCaches;
     QList<QQmlScriptData *> scripts;
 
-    QV4::CompiledData::CompilationUnit *compilationUnit;
+    QQmlRefPointer<QV4::CompiledData::CompilationUnit> compilationUnit;
     // index in first hash is component index, hash inside maps from object index in that scope to integer id
     QHash<int, QHash<int, int> > objectIndexToIdPerComponent;
     QHash<int, int> objectIndexToIdForRoot;
diff --git a/src/qml/qml/qqmlcontext_p.h b/src/qml/qml/qqmlcontext_p.h
index d3f283357a..233ea4d34a 100644
--- a/src/qml/qml/qqmlcontext_p.h
+++ b/src/qml/qml/qqmlcontext_p.h
@@ -141,6 +141,9 @@ public:
     // VME data that is constructing this context if any
     void *activeVMEData;
 
+    // Compilation unit for contexts that belong to a compiled type.
+    QQmlRefPointer<QV4::CompiledData::CompilationUnit> typeCompilationUnit;
+
     // Property name cache
     QV4::IdentifierHash<int> propertyNames;
 
diff --git a/src/qml/qml/qqmlexpression.cpp b/src/qml/qml/qqmlexpression.cpp
index 22e74135b5..947b55f15d 100644
--- a/src/qml/qml/qqmlexpression.cpp
+++ b/src/qml/qml/qqmlexpression.cpp
@@ -148,20 +148,13 @@ QQmlExpression::QQmlExpression(const QQmlScriptString &script, QQmlContext *ctxt
     if (scriptPrivate->context) {
         QQmlContextData *ctxtdata = QQmlContextData::get(scriptPrivate->context);
         QQmlEnginePrivate *engine = QQmlEnginePrivate::get(scriptPrivate->context->engine());
-        if (engine && ctxtdata && !ctxtdata->url.isEmpty()) {
-            QQmlTypeData *typeData = engine->typeLoader.getType(ctxtdata->url);
-            Q_ASSERT(typeData);
+        if (engine && ctxtdata && !ctxtdata->url.isEmpty() && ctxtdata->typeCompilationUnit) {
+            d->url = ctxtdata->url.toString();
+            d->line = scriptPrivate->lineNumber;
+            d->column = scriptPrivate->columnNumber;
 
-            if (QQmlCompiledData *cdata = typeData->compiledData()) {
-                d->url = cdata->fileName();
-                d->line = scriptPrivate->lineNumber;
-                d->column = scriptPrivate->columnNumber;
-
-                if (scriptPrivate->bindingId != QQmlBinding::Invalid)
-                    runtimeFunction = cdata->compilationUnit->runtimeFunctions.at(scriptPrivate->bindingId);
-            }
-
-            typeData->release();
+            if (scriptPrivate->bindingId != QQmlBinding::Invalid)
+                runtimeFunction = ctxtdata->typeCompilationUnit->runtimeFunctions.at(scriptPrivate->bindingId);
         }
     }
 
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index a827e96ab1..42cac66fbd 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -180,6 +180,7 @@ QObject *QQmlObjectCreator::create(int subComponentIndex, QObject *parent, QQmlI
     context->urlString = compiledData->fileName();
     context->imports = compiledData->importCache;
     context->imports->addref();
+    context->typeCompilationUnit = compiledData->compilationUnit;
     context->setParent(parentContext);
 
     if (!sharedState->rootContext) {
@@ -753,7 +754,7 @@ bool QQmlObjectCreator::setPropertyBinding(QQmlPropertyData *property, const QV4
     // ### resolve this at compile time
     if (property && property->propType == qMetaTypeId<QQmlScriptString>()) {
         QQmlScriptString ss(binding->valueAsScriptString(qmlUnit), context->asQQmlContext(), _scopeObject);
-        ss.d.data()->bindingId = binding->value.compiledScriptIndex;
+        ss.d.data()->bindingId = binding->type == QV4::CompiledData::Binding::Type_Script ? binding->value.compiledScriptIndex : QQmlBinding::Invalid;
         ss.d.data()->lineNumber = binding->location.line;
         ss.d.data()->columnNumber = binding->location.column;
         ss.d.data()->isStringLiteral = binding->type == QV4::CompiledData::Binding::Type_String;
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 0e2d4d027a..1b222fe1a3 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -2541,7 +2541,6 @@ void QQmlTypeData::scriptImported(QQmlScriptBlob *blob, const QV4::CompiledData:
 QQmlScriptData::QQmlScriptData()
     : importCache(0)
     , m_loaded(false)
-    , m_precompiledScript(0)
     , m_program(0)
 {
 }
@@ -2549,10 +2548,6 @@ QQmlScriptData::QQmlScriptData()
 QQmlScriptData::~QQmlScriptData()
 {
     delete m_program;
-    if (m_precompiledScript) {
-        m_precompiledScript->deref();
-        m_precompiledScript = 0;
-    }
 }
 
 void QQmlScriptData::initialize(QQmlEngine *engine)
@@ -2713,13 +2708,10 @@ void QQmlScriptBlob::dataReceived(const Data &data)
     }
 
     QList<QQmlError> errors;
-    QV4::CompiledData::CompilationUnit *unit = QV4::Script::precompile(&irUnit.jsModule, &irUnit.jsGenerator, v4, finalUrl(), source, &errors);
-    if (unit)
-        unit->ref();
+    QQmlRefPointer<QV4::CompiledData::CompilationUnit> unit = QV4::Script::precompile(&irUnit.jsModule, &irUnit.jsGenerator, v4, finalUrl(), source, &errors);
+    // No need to addref on unit, it's initial refcount is 1
     source.clear();
     if (!errors.isEmpty()) {
-        if (unit)
-            unit->deref();
         setError(errors);
         return;
     }
@@ -2732,7 +2724,6 @@ void QQmlScriptBlob::dataReceived(const Data &data)
     unit->data = unitData;
 
     initializeFromCompilationUnit(unit);
-    unit->deref();
 }
 
 void QQmlScriptBlob::initializeFromCachedUnit(const QQmlPrivate::CachedQmlUnit *unit)
@@ -2805,8 +2796,6 @@ void QQmlScriptBlob::initializeFromCompilationUnit(QV4::CompiledData::Compilatio
     m_scriptData->url = finalUrl();
     m_scriptData->urlString = finalUrlString();
     m_scriptData->m_precompiledScript = unit;
-    if (m_scriptData->m_precompiledScript)
-        m_scriptData->m_precompiledScript->ref();
 
     m_importCache.setBaseUrl(finalUrl(), finalUrlString());
 
diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h
index 9f98e4fd40..3b5aa1ec7a 100644
--- a/src/qml/qml/qqmltypeloader_p.h
+++ b/src/qml/qml/qqmltypeloader_p.h
@@ -513,7 +513,7 @@ private:
     void initialize(QQmlEngine *);
 
     bool m_loaded;
-    QV4::CompiledData::CompilationUnit *m_precompiledScript;
+    QQmlRefPointer<QV4::CompiledData::CompilationUnit> m_precompiledScript;
     QV4::Script *m_program;
     QV4::PersistentValue m_value;
 };