From e4e4a7912b03499a20f25e261e1c515aab17e5a8 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Tue, 4 Mar 2014 16:15:26 +0100 Subject: [PATCH] [new compiler] Fix invalid memory reads when JS closures outlive QML types If QQmlCompiledData gets destroyed while somebody still has refcount on the QV4::CompiledData::CompilationUnit, then unit's _data_ would be freed already by ~QQmlCompiledData. Given that compilationUnit->data is pointing to the same malloc'ed address as QQmlCompiledData::qmlUnit, we can just let the CompilationUnit always own the data. Fixes tst_qquickloader and makes it possible to run the qquickcomponent tests. Change-Id: Ie3f3e5335139236d7c2524a327665bda0a9cc847 Reviewed-by: Lars Knoll --- src/qml/compiler/qqmltypecompiler.cpp | 3 ++- src/qml/compiler/qv4compileddata.cpp | 3 +-- src/qml/compiler/qv4compileddata_p.h | 2 -- src/qml/compiler/qv4isel_p.cpp | 4 +--- src/qml/qml/qqmlcompileddata.cpp | 3 ++- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/qml/compiler/qqmltypecompiler.cpp b/src/qml/compiler/qqmltypecompiler.cpp index dfea506c8f..0cab2c4397 100644 --- a/src/qml/compiler/qqmltypecompiler.cpp +++ b/src/qml/compiler/qqmltypecompiler.cpp @@ -215,7 +215,8 @@ bool QQmlTypeCompiler::compile() if (jsUnit) { Q_ASSERT(!jsUnit->data); - jsUnit->ownsData = false; + Q_ASSERT((void*)qmlUnit == (void*)&qmlUnit->header); + // The js unit owns the data and will free the qml unit. jsUnit->data = &qmlUnit->header; } diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp index eae94decd3..0ddc75488c 100644 --- a/src/qml/compiler/qv4compileddata.cpp +++ b/src/qml/compiler/qv4compileddata.cpp @@ -150,8 +150,7 @@ void CompilationUnit::unlink() if (engine) engine->compilationUnits.erase(engine->compilationUnits.find(this)); engine = 0; - if (ownsData) - free(data); + free(data); data = 0; free(runtimeStrings); runtimeStrings = 0; diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index 026ce630d2..dec2bb8137 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -549,7 +549,6 @@ struct Q_QML_EXPORT CompilationUnit : refCount(0) , engine(0) , data(0) - , ownsData(false) , runtimeStrings(0) , runtimeLookups(0) , runtimeRegularExpressions(0) @@ -563,7 +562,6 @@ struct Q_QML_EXPORT CompilationUnit int refCount; ExecutionEngine *engine; Unit *data; - bool ownsData; QString fileName() const { return data->stringAt(data->sourceFileIndex); } diff --git a/src/qml/compiler/qv4isel_p.cpp b/src/qml/compiler/qv4isel_p.cpp index 7d05b9c28a..0e429423ab 100644 --- a/src/qml/compiler/qv4isel_p.cpp +++ b/src/qml/compiler/qv4isel_p.cpp @@ -86,10 +86,8 @@ QV4::CompiledData::CompilationUnit *EvalInstructionSelection::compile(bool gener run(i); QV4::CompiledData::CompilationUnit *unit = backendCompileStep(); - if (generateUnitData) { + if (generateUnitData) unit->data = jsGenerator->generateUnit(); - unit->ownsData = true; - } return unit; } diff --git a/src/qml/qml/qqmlcompileddata.cpp b/src/qml/qml/qqmlcompileddata.cpp index aec4553d5d..d20215c78a 100644 --- a/src/qml/qml/qqmlcompileddata.cpp +++ b/src/qml/qml/qqmlcompileddata.cpp @@ -140,9 +140,10 @@ QQmlCompiledData::~QQmlCompiledData() if (rootPropertyCache) rootPropertyCache->release(); + qmlUnit = 0; + if (compilationUnit) compilationUnit->deref(); - free(qmlUnit); } void QQmlCompiledData::clear()