Fix invalid reads with strings

It may happen that a dynamically created compilation unit disappears before any
QV4::Strings it created. Those strings would still have a reference to the
QString data in the compilation unit. I don't see a choice other than making a
copy of the string data ;(. But this patch adds a flag that would allow for
avoiding it if we happen to know that the compilation unit data is static.

Change-Id: Ib35a4d2a566b301a25ffe56e392809e44e7b4ae8
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp
index 7f02509..0bf7657 100644 (file)
--- a/src/qml/compiler/qv4compileddata.cpp
+++ b/src/qml/compiler/qv4compileddata.cpp
@@ -78,7 +78,7 @@ QV4::Function *CompilationUnit::linkToEngine(ExecutionEngine *engine)
     assert(data);
     runtimeStrings = (QV4::String**)malloc(data->stringTableSize * sizeof(QV4::String*));
     for (int i = 0; i < data->stringTableSize; ++i)
-        runtimeStrings[i] = engine->newIdentifier(data->stringAt(i)->qString());
+        runtimeStrings[i] = engine->newIdentifier(data->stringAt(i));
 
     runtimeRegularExpressions = new QV4::Value[data->regexpTableSize];
     for (int i = 0; i < data->regexpTableSize; ++i) {
@@ -90,7 +90,7 @@ QV4::Function *CompilationUnit::linkToEngine(ExecutionEngine *engine)
             flags |= QQmlJS::V4IR::RegExp::RegExp_IgnoreCase;
         if (re->flags & CompiledData::RegExp::RegExp_Multiline)
             flags |= QQmlJS::V4IR::RegExp::RegExp_Multiline;
-        QV4::RegExpObject *obj = engine->newRegExpObject(data->stringAt(re->stringIndex)->qString(), flags);
+        QV4::RegExpObject *obj = engine->newRegExpObject(data->stringAt(re->stringIndex), flags);
         runtimeRegularExpressions[i] = QV4::Value::fromObject(obj);
     }
 

diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h
index 379766f..fa139d3 100644 (file)
--- a/src/qml/compiler/qv4compileddata_p.h
+++ b/src/qml/compiler/qv4compileddata_p.h
@@ -108,6 +108,18 @@ struct JSClass
     static int calculateSize(int nMembers) { return (sizeof(JSClass) + nMembers * sizeof(JSClassMember) + 7) & ~7; }
 };
 
+struct String
+{
+    quint32 hash;
+    quint32 flags; // isArrayIndex
+    QArrayData str;
+    // uint16 strdata[]
+
+    static int calculateSize(const QString &str) {
+        return (sizeof(String) + (str.length() + 1) * sizeof(quint16) + 7) & ~0x7;
+    }
+};
+
 static const char magic_str[] = "qv4cdata";
 
 struct Unit
@@ -118,7 +130,8 @@ struct Unit
 
     enum {
         IsJavascript = 0x1,
-        IsQml = 0x2
+        IsQml = 0x2,
+        StaticData = 0x4 // Unit data persistent in memory?
     };
     quint32 flags;
     uint stringTableSize;
@@ -134,10 +147,15 @@ struct Unit
     uint indexOfRootFunction;
     quint32 sourceFileIndex;
 
-    const String *stringAt(int idx) const {
+    QString stringAt(int idx) const {
         const uint *offsetTable = reinterpret_cast<const uint*>((reinterpret_cast<const char *>(this)) + offsetToStringTable);
         const uint offset = offsetTable[idx];
-        return reinterpret_cast<const String*>(reinterpret_cast<const char *>(this) + offset);
+        const String *str = reinterpret_cast<const String*>(reinterpret_cast<const char *>(this) + offset);
+        QStringDataPtr holder = { const_cast<QStringData *>(static_cast<const QStringData*>(&str->str)) };
+        QString qstr(holder);
+        if (flags & StaticData)
+            return qstr;
+        return QString(qstr.constData(), qstr.length());
     }
 
     const Function *functionAt(int idx) const {
@@ -203,23 +221,6 @@ struct Function
     }
 };
 
-struct String
-{
-    quint32 hash;
-    quint32 flags; // isArrayIndex
-    QArrayData str;
-    // uint16 strdata[]
-
-    QString qString() const {
-        QStringDataPtr holder { const_cast<QStringData *>(static_cast<const QStringData*>(&str)) };
-        return QString(holder);
-    }
-
-    static int calculateSize(const QString &str) {
-        return (sizeof(String) + (str.length() + 1) * sizeof(quint16) + 7) & ~0x7;
-    }
-};
-
 // Qml data structures
 
 struct Value
@@ -322,7 +323,7 @@ struct CompilationUnit
     ExecutionEngine *engine;
     Unit *data;
 
-    QString fileName() const { return data->stringAt(data->sourceFileIndex)->qString(); }
+    QString fileName() const { return data->stringAt(data->sourceFileIndex); }
 
     QV4::String **runtimeStrings; // Array
     QV4::Lookup *runtimeLookups;

diff --git a/src/qml/jsruntime/qv4function.cpp b/src/qml/jsruntime/qv4function.cpp
index 607d067..e989d31 100644 (file)
--- a/src/qml/jsruntime/qv4function.cpp
+++ b/src/qml/jsruntime/qv4function.cpp
@@ -72,13 +72,13 @@ Function::Function(ExecutionEngine *engine, CompiledData::CompilationUnit *unit,
     formals.resize(compiledFunction->nFormals);
     const quint32 *formalsIndices = compiledFunction->formalsTable();
     for (int i = 0; i < compiledFunction->nFormals; ++i)
-        formals[i] = engine->newString(unit->data->stringAt(formalsIndices[i])->qString());
+        formals[i] = engine->newString(unit->data->stringAt(formalsIndices[i]));
 
 
     locals.resize(compiledFunction->nLocals);
     const quint32 *localsIndices = compiledFunction->localsTable();
     for (int i = 0; i < compiledFunction->nLocals; ++i)
-        locals[i] = engine->newString(unit->data->stringAt(localsIndices[i])->qString());
+        locals[i] = engine->newString(unit->data->stringAt(localsIndices[i]));
 }
 
 Function::~Function()