From d5c7229339a916b2f1f004ec4ea9de89995c003d Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 16 Mar 2018 17:27:38 +0100 Subject: Restore the QV4_WRITE_PERF_MAP feature We want to be able to generate perf map files for JITed code. Task-number: QTBUG-67056 Change-Id: I56899e1dbf184083d94efe926d21fca4f9ea1e18 Reviewed-by: Simon Hausmann --- src/qml/jit/qv4assembler.cpp | 47 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 6 deletions(-) (limited to 'src/qml') diff --git a/src/qml/jit/qv4assembler.cpp b/src/qml/jit/qv4assembler.cpp index 186e5952da..72b057b2bc 100644 --- a/src/qml/jit/qv4assembler.cpp +++ b/src/qml/jit/qv4assembler.cpp @@ -38,6 +38,7 @@ ****************************************************************************/ #include +#include #include "qv4engine_p.h" #include "qv4assembler_p.h" @@ -1367,6 +1368,17 @@ static void printDisassembledOutputWithCalls(QByteArray processedOutput, qDebug("%s", processedOutput.constData()); } +static QByteArray functionName(Function *function) +{ + QByteArray name = function->name()->toQString().toUtf8(); + if (name.isEmpty()) { + name = QByteArray::number(reinterpret_cast(function), 16); + name.prepend("QV4::Function(0x"); + name.append(')'); + } + return name; +} + void Assembler::link(Function *function) { for (const auto &jumpTarget : pasm()->patches) @@ -1388,12 +1400,7 @@ void Assembler::link(Function *function) buf.open(QIODevice::WriteOnly); WTF::setDataFile(new QIODevicePrintStream(&buf)); - QByteArray name = function->name()->toQString().toUtf8(); - if (name.isEmpty()) { - name = QByteArray::number(quintptr(function), 16); - name.prepend("QV4::Function(0x"); - name.append(')'); - } + QByteArray name = functionName(function); codeRef = linkBuffer.finalizeCodeWithDisassembly("%s", name.data()); WTF::setDataFile(stderr); @@ -1404,6 +1411,34 @@ void Assembler::link(Function *function) function->codeRef = new JSC::MacroAssemblerCodeRef(codeRef); function->jittedCode = reinterpret_cast(function->codeRef->code().executableAddress()); + +#if defined(Q_OS_LINUX) + // This implements writing of JIT'd addresses so that perf can find the + // symbol names. + // + // Perf expects the mapping to be in a certain place and have certain + // content, for more information, see: + // https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jit-interface.txt + static bool doProfile = !qEnvironmentVariableIsEmpty("QV4_PROFILE_WRITE_PERF_MAP"); + if (doProfile) { + static QFile perfMapFile(QString::fromLatin1("/tmp/perf-%1.map") + .arg(QCoreApplication::applicationPid())); + static const bool isOpen = perfMapFile.open(QIODevice::WriteOnly); + if (!isOpen) { + qWarning("QV4::JIT::Assembler: Cannot write perf map file."); + doProfile = false; + } else { + perfMapFile.write(QByteArray::number(reinterpret_cast( + codeRef.code().executableAddress()), 16)); + perfMapFile.putChar(' '); + perfMapFile.write(QByteArray::number(static_cast(codeRef.size()), 16)); + perfMapFile.putChar(' '); + perfMapFile.write(functionName(function)); + perfMapFile.putChar('\n'); + perfMapFile.flush(); + } + } +#endif } void Assembler::addLabel(int offset) -- cgit v1.2.3 From a3ad52526f79c1528f170c8affe5af00b68ca61d Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Thu, 15 Mar 2018 17:08:21 +0100 Subject: Fix crash when calling QQmlEngine::clearComponentCache() We must protect various resources in the type loader with our existing lock. The QQmlTypeLoaderQmldirContent is now value based, so that we can release the lock on the shared cache early. Copying it involves adjusting the refcount of the QHash and QString instances in the QQmlDirParser. The safety of this was verified with a TSAN build and the example supplied in the task. It crashed reliably with TASN errors first and with this patch it runs without errors. Task-number: QTBUG-41465 Change-Id: I616843c4b8bdfd65d1277d4faa8cb884d8e77df8 Reviewed-by: Lars Knoll --- src/qml/qml/qqmldirparser_p.h | 2 -- src/qml/qml/qqmlengine.cpp | 2 ++ src/qml/qml/qqmlimport.cpp | 62 +++++++++++++++++++++--------------------- src/qml/qml/qqmlimport_p.h | 2 +- src/qml/qml/qqmltypeloader.cpp | 55 ++++++++++++++++++++----------------- src/qml/qml/qqmltypeloader_p.h | 9 ++++-- 6 files changed, 71 insertions(+), 61 deletions(-) (limited to 'src/qml') diff --git a/src/qml/qml/qqmldirparser_p.h b/src/qml/qml/qqmldirparser_p.h index 95370398ad..820c40238d 100644 --- a/src/qml/qml/qqmldirparser_p.h +++ b/src/qml/qml/qqmldirparser_p.h @@ -63,8 +63,6 @@ class QQmlError; class QQmlEngine; class Q_QML_PRIVATE_EXPORT QQmlDirParser { - Q_DISABLE_COPY(QQmlDirParser) - public: QQmlDirParser(); ~QQmlDirParser(); diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index 4054d2f0be..49f25e89fe 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -1104,7 +1104,9 @@ QQmlEngine::~QQmlEngine() void QQmlEngine::clearComponentCache() { Q_D(QQmlEngine); + d->typeLoader.lock(); d->typeLoader.clearCache(); + d->typeLoader.unlock(); } /*! diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp index 8e6cbcbd7e..005db4248e 100644 --- a/src/qml/qml/qqmlimport.cpp +++ b/src/qml/qml/qqmlimport.cpp @@ -318,17 +318,17 @@ public: QQmlImportDatabase *database, QString *outQmldirFilePath, QString *outUrl); - static bool validateQmldirVersion(const QQmlTypeLoaderQmldirContent *qmldir, const QString &uri, int vmaj, int vmin, + static bool validateQmldirVersion(const QQmlTypeLoaderQmldirContent &qmldir, const QString &uri, int vmaj, int vmin, QList *errors); bool importExtension(const QString &absoluteFilePath, const QString &uri, int vmaj, int vmin, QQmlImportDatabase *database, - const QQmlTypeLoaderQmldirContent *qmldir, + const QQmlTypeLoaderQmldirContent &qmldir, QList *errors); bool getQmldirContent(const QString &qmldirIdentifier, const QString &uri, - const QQmlTypeLoaderQmldirContent **qmldir, QList *errors); + QQmlTypeLoaderQmldirContent *qmldir, QList *errors); QString resolvedUri(const QString &dir_arg, QQmlImportDatabase *database); @@ -668,14 +668,14 @@ bool QQmlImports::resolveType(const QHashedStringRef &type, return false; } -bool QQmlImportInstance::setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent *qmldir, QQmlImportNamespace *nameSpace, QList *errors) +bool QQmlImportInstance::setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent &qmldir, QQmlImportNamespace *nameSpace, QList *errors) { Q_ASSERT(resolvedUrl.endsWith(Slash)); url = resolvedUrl; - qmlDirComponents = qmldir->components(); + qmlDirComponents = qmldir.components(); - const QQmlDirScripts &scripts = qmldir->scripts(); + const QQmlDirScripts &scripts = qmldir.scripts(); if (!scripts.isEmpty()) { // Verify that we haven't imported these scripts already for (QList::const_iterator it = nameSpace->imports.constBegin(); @@ -1068,26 +1068,26 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, const QString &uri, int vmaj, int vmin, QQmlImportDatabase *database, - const QQmlTypeLoaderQmldirContent *qmldir, + const QQmlTypeLoaderQmldirContent &qmldir, QList *errors) { - Q_ASSERT(qmldir); + Q_ASSERT(qmldir.hasContent()); if (qmlImportTrace()) qDebug().nospace() << "QQmlImports(" << qPrintable(base) << ")::importExtension: " << "loaded " << qmldirFilePath; - if (designerSupportRequired && !qmldir->designerSupported()) { + if (designerSupportRequired && !qmldir.designerSupported()) { if (errors) { QQmlError error; - error.setDescription(QQmlImportDatabase::tr("module does not support the designer \"%1\"").arg(qmldir->typeNamespace())); + error.setDescription(QQmlImportDatabase::tr("module does not support the designer \"%1\"").arg(qmldir.typeNamespace())); error.setUrl(QUrl::fromLocalFile(qmldirFilePath)); errors->prepend(error); } return false; } - int qmldirPluginCount = qmldir->plugins().count(); + int qmldirPluginCount = qmldir.plugins().count(); if (qmldirPluginCount == 0) return true; @@ -1098,7 +1098,7 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, // listed plugin inside qmldir. And for this reason, mixing dynamic and static plugins inside a // single module is not recommended. - QString typeNamespace = qmldir->typeNamespace(); + QString typeNamespace = qmldir.typeNamespace(); QString qmldirPath = qmldirFilePath; int slash = qmldirPath.lastIndexOf(Slash); if (slash > 0) @@ -1108,7 +1108,7 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, int staticPluginsFound = 0; #if defined(QT_SHARED) - const auto qmldirPlugins = qmldir->plugins(); + const auto qmldirPlugins = qmldir.plugins(); for (const QQmlDirParser::Plugin &plugin : qmldirPlugins) { QString resolvedFilePath = database->resolvePlugin(typeLoader, qmldirPath, plugin.path, plugin.name); if (!resolvedFilePath.isEmpty()) { @@ -1174,7 +1174,7 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, if (qmldirPluginCount > 1 && staticPluginsFound > 0) error.setDescription(QQmlImportDatabase::tr("could not resolve all plugins for module \"%1\"").arg(uri)); else - error.setDescription(QQmlImportDatabase::tr("module \"%1\" plugin \"%2\" not found").arg(uri).arg(qmldir->plugins()[dynamicPluginsFound].name)); + error.setDescription(QQmlImportDatabase::tr("module \"%1\" plugin \"%2\" not found").arg(uri).arg(qmldir.plugins()[dynamicPluginsFound].name)); error.setUrl(QUrl::fromLocalFile(qmldirFilePath)); errors->prepend(error); } @@ -1187,17 +1187,17 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, } bool QQmlImportsPrivate::getQmldirContent(const QString &qmldirIdentifier, const QString &uri, - const QQmlTypeLoaderQmldirContent **qmldir, QList *errors) + QQmlTypeLoaderQmldirContent *qmldir, QList *errors) { Q_ASSERT(errors); Q_ASSERT(qmldir); *qmldir = typeLoader->qmldirContent(qmldirIdentifier); - if (*qmldir) { + if ((*qmldir).hasContent()) { // Ensure that parsing was successful - if ((*qmldir)->hasError()) { + if ((*qmldir).hasError()) { QUrl url = QUrl::fromLocalFile(qmldirIdentifier); - const QList qmldirErrors = (*qmldir)->errors(uri); + const QList qmldirErrors = (*qmldir).errors(uri); for (int i = 0; i < qmldirErrors.size(); ++i) { QQmlError error = qmldirErrors.at(i); error.setUrl(url); @@ -1323,14 +1323,14 @@ bool QQmlImportsPrivate::locateQmldir(const QString &uri, int vmaj, int vmin, QQ return false; } -bool QQmlImportsPrivate::validateQmldirVersion(const QQmlTypeLoaderQmldirContent *qmldir, const QString &uri, int vmaj, int vmin, +bool QQmlImportsPrivate::validateQmldirVersion(const QQmlTypeLoaderQmldirContent &qmldir, const QString &uri, int vmaj, int vmin, QList *errors) { int lowest_min = INT_MAX; int highest_min = INT_MIN; typedef QQmlDirComponents::const_iterator ConstIterator; - const QQmlDirComponents &components = qmldir->components(); + const QQmlDirComponents &components = qmldir.components(); ConstIterator cend = components.constEnd(); for (ConstIterator cit = components.constBegin(); cit != cend; ++cit) { @@ -1354,7 +1354,7 @@ bool QQmlImportsPrivate::validateQmldirVersion(const QQmlTypeLoaderQmldirContent } typedef QList::const_iterator SConstIterator; - const QQmlDirScripts &scripts = qmldir->scripts(); + const QQmlDirScripts &scripts = qmldir.scripts(); SConstIterator send = scripts.constEnd(); for (SConstIterator sit = scripts.constBegin(); sit != send; ++sit) { @@ -1446,14 +1446,14 @@ bool QQmlImportsPrivate::addLibraryImport(const QString& uri, const QString &pre Q_ASSERT(inserted); if (!incomplete) { - const QQmlTypeLoaderQmldirContent *qmldir = nullptr; + QQmlTypeLoaderQmldirContent qmldir; if (!qmldirIdentifier.isEmpty()) { if (!getQmldirContent(qmldirIdentifier, uri, &qmldir, errors)) return false; - if (qmldir) { - if (!importExtension(qmldir->pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) + if (qmldir.hasContent()) { + if (!importExtension(qmldir.pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) return false; if (!inserted->setQmldirContent(qmldirUrl, qmldir, nameSpace, errors)) @@ -1471,7 +1471,7 @@ bool QQmlImportsPrivate::addLibraryImport(const QString& uri, const QString &pre error.setDescription(QQmlImportDatabase::tr("module \"%1\" is not installed").arg(uri)); errors->prepend(error); return false; - } else if ((vmaj >= 0) && (vmin >= 0) && qmldir) { + } else if ((vmaj >= 0) && (vmin >= 0) && qmldir.hasContent()) { // Verify that the qmldir content is valid for this version if (!validateQmldirVersion(qmldir, uri, vmaj, vmin, errors)) return false; @@ -1565,12 +1565,12 @@ bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix Q_ASSERT(inserted); if (!incomplete && !qmldirIdentifier.isEmpty()) { - const QQmlTypeLoaderQmldirContent *qmldir = nullptr; + QQmlTypeLoaderQmldirContent qmldir; if (!getQmldirContent(qmldirIdentifier, importUri, &qmldir, errors)) return false; - if (qmldir) { - if (!importExtension(qmldir->pluginLocation(), importUri, vmaj, vmin, database, qmldir, errors)) + if (qmldir.hasContent()) { + if (!importExtension(qmldir.pluginLocation(), importUri, vmaj, vmin, database, qmldir, errors)) return false; if (!inserted->setQmldirContent(url, qmldir, nameSpace, errors)) @@ -1589,14 +1589,14 @@ bool QQmlImportsPrivate::updateQmldirContent(const QString &uri, const QString & Q_ASSERT(nameSpace); if (QQmlImportInstance *import = nameSpace->findImport(uri)) { - const QQmlTypeLoaderQmldirContent *qmldir = nullptr; + QQmlTypeLoaderQmldirContent qmldir; if (!getQmldirContent(qmldirIdentifier, uri, &qmldir, errors)) return false; - if (qmldir) { + if (qmldir.hasContent()) { int vmaj = import->majversion; int vmin = import->minversion; - if (!importExtension(qmldir->pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) + if (!importExtension(qmldir.pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) return false; if (import->setQmldirContent(qmldirUrl, qmldir, nameSpace, errors)) { diff --git a/src/qml/qml/qqmlimport_p.h b/src/qml/qml/qqmlimport_p.h index b70bb5253c..2437979ef8 100644 --- a/src/qml/qml/qqmlimport_p.h +++ b/src/qml/qml/qqmlimport_p.h @@ -85,7 +85,7 @@ struct QQmlImportInstance QQmlDirComponents qmlDirComponents; // a copy of the components listed in the qmldir QQmlDirScripts qmlDirScripts; // a copy of the scripts in the qmldir - bool setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent *qmldir, + bool setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent &qmldir, QQmlImportNamespace *nameSpace, QList *errors); static QQmlDirScripts getVersionedScripts(const QQmlDirScripts &qmldirscripts, int vmaj, int vmin); diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index 5b954605e0..2b778b0b63 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -1370,8 +1370,8 @@ bool QQmlTypeLoader::Blob::updateQmldir(QQmlQmldirData *data, const QV4::Compile if (!importQualifier.isEmpty()) { // Does this library contain any qualified scripts? QUrl libraryUrl(qmldirUrl); - const QQmlTypeLoaderQmldirContent *qmldir = typeLoader()->qmldirContent(qmldirIdentifier); - const auto qmldirScripts = qmldir->scripts(); + const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirIdentifier); + const auto qmldirScripts = qmldir.scripts(); for (const QQmlDirParser::Script &script : qmldirScripts) { QUrl scriptUrl = libraryUrl.resolved(QUrl(script.fileName)); QQmlScriptBlob *blob = typeLoader()->getScript(scriptUrl); @@ -1418,8 +1418,8 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL if (!importQualifier.isEmpty()) { // Does this library contain any qualified scripts? QUrl libraryUrl(qmldirUrl); - const QQmlTypeLoaderQmldirContent *qmldir = typeLoader()->qmldirContent(qmldirFilePath); - const auto qmldirScripts = qmldir->scripts(); + const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirFilePath); + const auto qmldirScripts = qmldir.scripts(); for (const QQmlDirParser::Script &script : qmldirScripts) { QUrl scriptUrl = libraryUrl.resolved(QUrl(script.fileName)); QQmlScriptBlob *blob = typeLoader()->getScript(scriptUrl); @@ -1584,6 +1584,7 @@ QString QQmlTypeLoaderQmldirContent::typeNamespace() const void QQmlTypeLoaderQmldirContent::setContent(const QString &location, const QString &content) { + m_hasContent = true; m_location = location; m_parser.parse(content); } @@ -1808,6 +1809,7 @@ QString QQmlTypeLoader::absoluteFilePath(const QString &path) int lastSlash = path.lastIndexOf(QLatin1Char('/')); QString dirPath(path.left(lastSlash)); + LockHolder holder(this); if (!m_importDirCache.contains(dirPath)) { bool exists = QDir(dirPath).exists(); QCache *entry = exists ? new QCache : nullptr; @@ -1871,6 +1873,7 @@ bool QQmlTypeLoader::directoryExists(const QString &path) --length; QString dirPath(path.left(length)); + LockHolder holder(this); if (!m_importDirCache.contains(dirPath)) { bool exists = QDir(dirPath).exists(); QCache *files = exists ? new QCache : nullptr; @@ -1889,8 +1892,10 @@ Return a QQmlTypeLoaderQmldirContent for absoluteFilePath. The QQmlTypeLoaderQm It can also be a remote path for a remote directory import, but it will have been cached by now in this case. */ -const QQmlTypeLoaderQmldirContent *QQmlTypeLoader::qmldirContent(const QString &filePathIn) +const QQmlTypeLoaderQmldirContent QQmlTypeLoader::qmldirContent(const QString &filePathIn) { + LockHolder holder(this); + QString filePath; // Try to guess if filePathIn is already a URL. This is necessarily fragile, because @@ -1904,39 +1909,39 @@ const QQmlTypeLoaderQmldirContent *QQmlTypeLoader::qmldirContent(const QString & filePath = filePathIn; } else { filePath = QQmlFile::urlToLocalFileOrQrc(url); - if (filePath.isEmpty()) // Can't load the remote here, but should be cached - return *(m_importQmlDirCache.value(filePathIn)); + if (filePath.isEmpty()) { // Can't load the remote here, but should be cached + if (auto entry = m_importQmlDirCache.value(filePathIn)) + return **entry; + else + return QQmlTypeLoaderQmldirContent(); + } } - QQmlTypeLoaderQmldirContent *qmldir; QQmlTypeLoaderQmldirContent **val = m_importQmlDirCache.value(filePath); - if (!val) { - qmldir = new QQmlTypeLoaderQmldirContent; + if (val) + return **val; + QQmlTypeLoaderQmldirContent *qmldir = new QQmlTypeLoaderQmldirContent; #define ERROR(description) { QQmlError e; e.setDescription(description); qmldir->setError(e); } #define NOT_READABLE_ERROR QString(QLatin1String("module \"$$URI$$\" definition \"%1\" not readable")) #define CASE_MISMATCH_ERROR QString(QLatin1String("cannot load module \"$$URI$$\": File name case mismatch for \"%1\"")) - QFile file(filePath); - if (!QQml_isFileCaseCorrect(filePath)) { - ERROR(CASE_MISMATCH_ERROR.arg(filePath)); - } else if (file.open(QFile::ReadOnly)) { - QByteArray data = file.readAll(); - qmldir->setContent(filePath, QString::fromUtf8(data)); - } else { - ERROR(NOT_READABLE_ERROR.arg(filePath)); - } + QFile file(filePath); + if (!QQml_isFileCaseCorrect(filePath)) { + ERROR(CASE_MISMATCH_ERROR.arg(filePath)); + } else if (file.open(QFile::ReadOnly)) { + QByteArray data = file.readAll(); + qmldir->setContent(filePath, QString::fromUtf8(data)); + } else { + ERROR(NOT_READABLE_ERROR.arg(filePath)); + } #undef ERROR #undef NOT_READABLE_ERROR #undef CASE_MISMATCH_ERROR - m_importQmlDirCache.insert(filePath, qmldir); - } else { - qmldir = *val; - } - - return qmldir; + m_importQmlDirCache.insert(filePath, qmldir); + return *qmldir; } void QQmlTypeLoader::setQmldirContent(const QString &url, const QString &content) diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h index df79d13f1c..4665d342c1 100644 --- a/src/qml/qml/qqmltypeloader_p.h +++ b/src/qml/qml/qqmltypeloader_p.h @@ -228,12 +228,16 @@ class QQmlTypeLoaderQmldirContent { private: friend class QQmlTypeLoader; - QQmlTypeLoaderQmldirContent(); void setContent(const QString &location, const QString &content); void setError(const QQmlError &); public: + QQmlTypeLoaderQmldirContent(); + QQmlTypeLoaderQmldirContent(const QQmlTypeLoaderQmldirContent &) = default; + QQmlTypeLoaderQmldirContent &operator=(const QQmlTypeLoaderQmldirContent &) = default; + + bool hasContent() const { return m_hasContent; } bool hasError() const; QList errors(const QString &uri) const; @@ -250,6 +254,7 @@ public: private: QQmlDirParser m_parser; QString m_location; + bool m_hasContent = false; }; class Q_QML_PRIVATE_EXPORT QQmlTypeLoader @@ -304,7 +309,7 @@ public: QString absoluteFilePath(const QString &path); bool directoryExists(const QString &path); - const QQmlTypeLoaderQmldirContent *qmldirContent(const QString &filePath); + const QQmlTypeLoaderQmldirContent qmldirContent(const QString &filePath); void setQmldirContent(const QString &filePath, const QString &content); void clearCache(); -- cgit v1.2.3 From f495d4b660107536d0a67ba48e88550278f13893 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Mon, 19 Mar 2018 15:07:28 +0100 Subject: Fix out of bounds reads in Array.concat In some cases, when our simple array data had an offset and data would wrap around, ArrayData::append would write out of bounds data into the new array, leading to crashes. Task-number: QTBUG-51581 Change-Id: I55172542ef0b94d263cfc9a17d7ca49ec6c3a565 Reviewed-by: Simon Hausmann --- src/qml/jsruntime/qv4arraydata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/qml') diff --git a/src/qml/jsruntime/qv4arraydata.cpp b/src/qml/jsruntime/qv4arraydata.cpp index 30c8527f21..b9c0e12305 100644 --- a/src/qml/jsruntime/qv4arraydata.cpp +++ b/src/qml/jsruntime/qv4arraydata.cpp @@ -617,7 +617,7 @@ uint ArrayData::append(Object *obj, ArrayObject *otherObj, uint n) uint toCopy = n; uint chunk = toCopy; if (chunk > os->values.alloc - os->offset) - chunk -= os->values.alloc - os->offset; + chunk = os->values.alloc - os->offset; obj->arrayPut(oldSize, os->values.data() + os->offset, chunk); toCopy -= chunk; if (toCopy) -- cgit v1.2.3 From 1de4d413c9dd5c8ad5859f944d9ded7d8778f777 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 19 Mar 2018 17:01:21 +0100 Subject: Fix assigning objects to QJSValue properties We support simple object bindings such as someProperty: Rectangle { ... } when the type of "someProperty" is QVariant, but we produce an error when it's QJSValue. There is no good reason for that, and the fix for QTBUG-67118 requires this. Change-Id: Ia5dc88749bcba0b5c781a6ab2b4a9fb92299e0ac Reviewed-by: Mitch Curtis Reviewed-by: Lars Knoll --- src/qml/compiler/qqmlpropertyvalidator.cpp | 2 +- src/qml/qml/qqmlobjectcreator.cpp | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) (limited to 'src/qml') diff --git a/src/qml/compiler/qqmlpropertyvalidator.cpp b/src/qml/compiler/qqmlpropertyvalidator.cpp index 00bb694ef4..ffd3b5975a 100644 --- a/src/qml/compiler/qqmlpropertyvalidator.cpp +++ b/src/qml/compiler/qqmlpropertyvalidator.cpp @@ -653,7 +653,7 @@ QQmlCompileError QQmlPropertyValidator::validateObjectBinding(QQmlPropertyData * // Can only check at instantiation time if the created sub-object successfully casts to the // target interface. return noError; - } else if (property->propType() == QMetaType::QVariant) { + } else if (property->propType() == QMetaType::QVariant || property->propType() == qMetaTypeId()) { // We can convert everything to QVariant :) return noError; } else if (property->isQList()) { diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 90f3beb40b..7051fb51da 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -55,6 +55,7 @@ #include #include #include +#include QT_USE_NAMESPACE @@ -1039,6 +1040,17 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper argv[0] = &value; QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv); } + } else if (bindingProperty->propType() == qMetaTypeId()) { + QV4::Scope scope(v4); + QV4::ScopedValue wrappedObject(scope, QV4::QObjectWrapper::wrap(engine->handle(), createdSubObject)); + if (bindingProperty->isVarProperty()) { + _vmeMetaObject->setVMEProperty(bindingProperty->coreIndex(), wrappedObject); + } else { + QJSValue value; + QJSValuePrivate::setValue(&value, v4, wrappedObject); + argv[0] = &value; + QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv); + } } else if (bindingProperty->isQList()) { Q_ASSERT(_currentList.object); -- cgit v1.2.3 From 22b13921f8067f8a93164875a4ad59bed85b0400 Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Mon, 19 Mar 2018 13:14:13 +0100 Subject: Handle function expressions as signal handlers There are two ways to use function expressions on the right-hand side of bindings: property var somethingPressed somethingPressed: function() { /* ..press something else.. */ } signal buttonPressed onButtonPressed: function() { /* ..handle buttonPress.. */ } In the former case, it declares a property that holds a function. So on initialization, the right-hand side of the binding returns a closure that gets assigned to the property 'somethingPressed'. In the latter case, the signal handler is explicitly marked as a function for clarity. So, the handler should not be returning the closure, but the handler should *be* the closure. In general, it is not possible to detect if the left-hand side is a property or a signal handler when generating QML cache files ahead of time. So for this case, we mark the function as only returning a closure. Then when instantiating the object, we check if it is a signal handler, and if the handler is marked as only returning a closure. If so, we set that closure to be the signal handler. Task-number: QTBUG-57043 Task-number: QTBUG-50328 Change-Id: I3008ddd847e30b7d0adef07344a326f84d85f1ba Reviewed-by: Simon Hausmann --- src/qml/compiler/qv4codegen.cpp | 8 ++++++++ src/qml/compiler/qv4compileddata_p.h | 2 +- src/qml/compiler/qv4compiler.cpp | 4 +++- src/qml/compiler/qv4compilercontext_p.h | 1 + src/qml/jsruntime/qv4function_p.h | 7 +++++++ src/qml/qml/qqmlobjectcreator.cpp | 8 ++++++++ src/qml/types/qqmlconnections.cpp | 14 +++++++++++--- 7 files changed, 39 insertions(+), 5 deletions(-) (limited to 'src/qml') diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index bc4ca5d6f4..a262908960 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -2075,6 +2075,14 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast, _context->hasDirectEval |= (_context->compilationMode == EvalCode || _context->compilationMode == GlobalCode || _module->debugMode); // Conditional breakpoints are like eval in the function + // When a user writes the following QML signal binding: + // onSignal: function() { doSomethingUsefull } + // we will generate a binding function that just returns the closure. However, that's not useful + // at all, because if the onSignal is a signal handler, the user is actually making it explicit + // that the binding is a function, so we should execute that. However, we don't know that during + // AOT compilation, so mark the surrounding function as only-returning-a-closure. + _context->returnsClosure = cast(ast) && cast(cast(ast)->expression); + BytecodeGenerator bytecode(_context->line, _module->debugMode); BytecodeGenerator *savedBytecodeGenerator; savedBytecodeGenerator = bytecodeGenerator; diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index f1776f5772..e6c4225bbd 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -225,7 +225,7 @@ struct Function quint32_le localsOffset; quint32_le nLineNumbers; quint32_le lineNumberOffset; - quint32_le nInnerFunctions; + quint32_le nestedFunctionIndex; // for functions that only return a single closure, used in signal handlers quint32_le nRegisters; Location location; diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index f2e1f4a0de..ccc909c199 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -305,6 +305,9 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::Compiler::Conte function->flags |= CompiledData::Function::IsStrict; if (irFunction->hasTry || irFunction->hasWith) function->flags |= CompiledData::Function::HasCatchOrWith; + function->nestedFunctionIndex = + irFunction->returnsClosure ? quint32(module->functions.indexOf(irFunction->nestedContexts.first())) + : std::numeric_limits::max(); function->nFormals = irFunction->arguments.size(); function->formalsOffset = currentOffset; currentOffset += function->nFormals * sizeof(quint32); @@ -317,7 +320,6 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::Compiler::Conte function->lineNumberOffset = currentOffset; currentOffset += function->nLineNumbers * sizeof(CompiledData::CodeOffsetToLine); - function->nInnerFunctions = irFunction->nestedContexts.size(); function->nRegisters = irFunction->registerCount; diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h index a78a66db52..8fabf41c40 100644 --- a/src/qml/compiler/qv4compilercontext_p.h +++ b/src/qml/compiler/qv4compilercontext_p.h @@ -144,6 +144,7 @@ struct Context { bool usesThis = false; bool hasTry = false; bool hasWith = false; + bool returnsClosure = false; mutable bool argumentsCanEscape = false; enum UsesArgumentsObject { diff --git a/src/qml/jsruntime/qv4function_p.h b/src/qml/jsruntime/qv4function_p.h index 4c8c790ca7..59a94e5dde 100644 --- a/src/qml/jsruntime/qv4function_p.h +++ b/src/qml/jsruntime/qv4function_p.h @@ -105,6 +105,13 @@ struct Q_QML_EXPORT Function { { return QQmlSourceLocation(sourceFile(), compiledFunction->location.line, compiledFunction->location.column); } + + Function *nestedFunction() const + { + if (compiledFunction->nestedFunctionIndex == std::numeric_limits::max()) + return nullptr; + return compilationUnit->runtimeFunctions[compiledFunction->nestedFunctionIndex]; + } }; } diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 7051fb51da..7c36f59035 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -879,6 +879,14 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper if (binding->type == QV4::CompiledData::Binding::Type_Script || binding->containsTranslations()) { if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression) { QV4::Function *runtimeFunction = compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + + // When a user writes the following: + // onSignal: function() { doSomethingUsefull } + // then do not run the binding that returns the closure, but run the closure + // instead. + if (auto closure = runtimeFunction->nestedFunction()) + runtimeFunction = closure; + int signalIndex = _propertyCache->methodIndexToSignalIndex(bindingProperty->coreIndex()); QQmlBoundSignal *bs = new QQmlBoundSignal(_bindingTarget, signalIndex, _scopeObject, engine); QQmlBoundSignalExpression *expr = new QQmlBoundSignalExpression(_bindingTarget, signalIndex, diff --git a/src/qml/types/qqmlconnections.cpp b/src/qml/types/qqmlconnections.cpp index a43562a7b8..d1a7aa9b6f 100644 --- a/src/qml/types/qqmlconnections.cpp +++ b/src/qml/types/qqmlconnections.cpp @@ -288,9 +288,17 @@ void QQmlConnections::connectSignals() new QQmlBoundSignal(target, signalIndex, this, qmlEngine(this)); signal->setEnabled(d->enabled); - QQmlBoundSignalExpression *expression = ctxtdata ? - new QQmlBoundSignalExpression(target, signalIndex, - ctxtdata, this, d->compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]) : nullptr; + auto f = d->compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + + // If the function is marked as having a nested function, then the user wrote: + // onSomeSignal: function() { /*....*/ } + // So take that nested function: + if (auto closure = f->nestedFunction()) + f = closure; + + QQmlBoundSignalExpression *expression = + ctxtdata ? new QQmlBoundSignalExpression(target, signalIndex, ctxtdata, this, f) + : nullptr; signal->takeExpression(expression); d->boundsignals += signal; } else { -- cgit v1.2.3 From 64e90f393146dadb382d154e7d67a9109ab2492a Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Fri, 9 Mar 2018 16:40:38 +0100 Subject: Fix QML data structure version checking for ahead-of-time generated files We must also do version checking for QML and JS files that were compiled ahead of time and are embedded in resources. If the lookup for the original source code fails, then we must generate an appropriate error message. As an upside we get better error reporting when trying to load an empty file and Qt.include() now reports the error message in the statusText field. The error reporting for imported scripts was not changed as importing an empty script is (oddly) allowed. Task-number: QTBUG-66986 Change-Id: Ie0ef81af371a51ecf8c66ae7954d43f5cc6c12de Reviewed-by: Erik Verbruggen --- src/qml/compiler/qv4compilationunitmapper.cpp | 32 ------------------ src/qml/compiler/qv4compilationunitmapper_p.h | 2 -- src/qml/compiler/qv4compilationunitmapper_unix.cpp | 2 +- src/qml/compiler/qv4compilationunitmapper_win.cpp | 2 +- src/qml/compiler/qv4compileddata.cpp | 38 ++++++++++++++++++++++ src/qml/compiler/qv4compileddata_p.h | 2 ++ src/qml/jsruntime/qv4include.cpp | 10 ++++-- src/qml/jsruntime/qv4include_p.h | 3 +- src/qml/jsruntime/qv4script.cpp | 17 ++++++++-- src/qml/jsruntime/qv4script_p.h | 2 +- src/qml/qml/qqmlmetatype.cpp | 35 ++++++++++++++++++-- src/qml/qml/qqmlmetatype_p.h | 12 ++++++- src/qml/qml/qqmltypeloader.cpp | 38 ++++++++++++++++++---- src/qml/qml/qqmltypeloader_p.h | 5 +++ src/qml/types/qquickworkerscript.cpp | 6 ++-- 15 files changed, 150 insertions(+), 56 deletions(-) (limited to 'src/qml') diff --git a/src/qml/compiler/qv4compilationunitmapper.cpp b/src/qml/compiler/qv4compilationunitmapper.cpp index d94f7ac238..350f6f9485 100644 --- a/src/qml/compiler/qv4compilationunitmapper.cpp +++ b/src/qml/compiler/qv4compilationunitmapper.cpp @@ -59,36 +59,4 @@ CompilationUnitMapper::~CompilationUnitMapper() close(); } -bool CompilationUnitMapper::verifyHeader(const CompiledData::Unit *header, QDateTime sourceTimeStamp, QString *errorString) -{ - if (strncmp(header->magic, CompiledData::magic_str, sizeof(header->magic))) { - *errorString = QStringLiteral("Magic bytes in the header do not match"); - return false; - } - - if (header->version != quint32(QV4_DATA_STRUCTURE_VERSION)) { - *errorString = QString::fromUtf8("V4 data structure version mismatch. Found %1 expected %2").arg(header->version, 0, 16).arg(QV4_DATA_STRUCTURE_VERSION, 0, 16); - return false; - } - - if (header->qtVersion != quint32(QT_VERSION)) { - *errorString = QString::fromUtf8("Qt version mismatch. Found %1 expected %2").arg(header->qtVersion, 0, 16).arg(QT_VERSION, 0, 16); - return false; - } - - if (header->sourceTimeStamp) { - // Files from the resource system do not have any time stamps, so fall back to the application - // executable. - if (!sourceTimeStamp.isValid()) - sourceTimeStamp = QFileInfo(QCoreApplication::applicationFilePath()).lastModified(); - - if (sourceTimeStamp.isValid() && sourceTimeStamp.toMSecsSinceEpoch() != header->sourceTimeStamp) { - *errorString = QStringLiteral("QML source file has a different time stamp than cached file."); - return false; - } - } - - return true; -} - QT_END_NAMESPACE diff --git a/src/qml/compiler/qv4compilationunitmapper_p.h b/src/qml/compiler/qv4compilationunitmapper_p.h index b24f98df7c..80f914c141 100644 --- a/src/qml/compiler/qv4compilationunitmapper_p.h +++ b/src/qml/compiler/qv4compilationunitmapper_p.h @@ -72,8 +72,6 @@ public: void close(); private: - static bool verifyHeader(const QV4::CompiledData::Unit *header, QDateTime sourceTimeStamp, QString *errorString); - #if defined(Q_OS_UNIX) size_t length; #endif diff --git a/src/qml/compiler/qv4compilationunitmapper_unix.cpp b/src/qml/compiler/qv4compilationunitmapper_unix.cpp index 38dabc41cf..8348613888 100644 --- a/src/qml/compiler/qv4compilationunitmapper_unix.cpp +++ b/src/qml/compiler/qv4compilationunitmapper_unix.cpp @@ -73,7 +73,7 @@ CompiledData::Unit *CompilationUnitMapper::open(const QString &cacheFileName, co return nullptr; } - if (!verifyHeader(&header, sourceTimeStamp, errorString)) + if (!header.verifyHeader(sourceTimeStamp, errorString)) return nullptr; // Data structure and qt version matched, so now we can access the rest of the file safely. diff --git a/src/qml/compiler/qv4compilationunitmapper_win.cpp b/src/qml/compiler/qv4compilationunitmapper_win.cpp index d7a93ae233..8b000021f8 100644 --- a/src/qml/compiler/qv4compilationunitmapper_win.cpp +++ b/src/qml/compiler/qv4compilationunitmapper_win.cpp @@ -87,7 +87,7 @@ CompiledData::Unit *CompilationUnitMapper::open(const QString &cacheFileName, co return nullptr; } - if (!verifyHeader(&header, sourceTimeStamp, errorString)) + if (!header.verifyHeader(sourceTimeStamp, errorString)) return nullptr; const uint mappingFlags = header.flags & QV4::CompiledData::Unit::ContainsMachineCode diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp index cc11b250f3..6220c1bc11 100644 --- a/src/qml/compiler/qv4compileddata.cpp +++ b/src/qml/compiler/qv4compileddata.cpp @@ -755,6 +755,44 @@ void Unit::generateChecksum() #endif } +bool Unit::verifyHeader(QDateTime expectedSourceTimeStamp, QString *errorString) const +{ +#ifndef V4_BOOTSTRAP + if (strncmp(magic, CompiledData::magic_str, sizeof(magic))) { + *errorString = QStringLiteral("Magic bytes in the header do not match"); + return false; + } + + if (version != quint32(QV4_DATA_STRUCTURE_VERSION)) { + *errorString = QString::fromUtf8("V4 data structure version mismatch. Found %1 expected %2").arg(version, 0, 16).arg(QV4_DATA_STRUCTURE_VERSION, 0, 16); + return false; + } + + if (qtVersion != quint32(QT_VERSION)) { + *errorString = QString::fromUtf8("Qt version mismatch. Found %1 expected %2").arg(qtVersion, 0, 16).arg(QT_VERSION, 0, 16); + return false; + } + + if (sourceTimeStamp) { + // Files from the resource system do not have any time stamps, so fall back to the application + // executable. + if (!expectedSourceTimeStamp.isValid()) + expectedSourceTimeStamp = QFileInfo(QCoreApplication::applicationFilePath()).lastModified(); + + if (expectedSourceTimeStamp.isValid() && expectedSourceTimeStamp.toMSecsSinceEpoch() != sourceTimeStamp) { + *errorString = QStringLiteral("QML source file has a different time stamp than cached file."); + return false; + } + } + + return true; +#else + Q_UNUSED(expectedSourceTimeStamp) + Q_UNUSED(errorString) + return false; +#endif +} + } } diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index e6c4225bbd..49360bc3f1 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -727,6 +727,8 @@ struct Unit quint32_le padding; + bool verifyHeader(QDateTime expectedSourceTimeStamp, QString *errorString) const; + const Import *importAt(int idx) const { return reinterpret_cast((reinterpret_cast(this)) + offsetToImports + idx * sizeof(Import)); } diff --git a/src/qml/jsruntime/qv4include.cpp b/src/qml/jsruntime/qv4include.cpp index aaf5e3b857..d0d66c9b9a 100644 --- a/src/qml/jsruntime/qv4include.cpp +++ b/src/qml/jsruntime/qv4include.cpp @@ -92,7 +92,8 @@ QV4Include::~QV4Include() #endif } -QV4::ReturnedValue QV4Include::resultValue(QV4::ExecutionEngine *v4, Status status) +QV4::ReturnedValue QV4Include::resultValue(QV4::ExecutionEngine *v4, Status status, + const QString &statusText) { QV4::Scope scope(v4); @@ -105,6 +106,8 @@ QV4::ReturnedValue QV4Include::resultValue(QV4::ExecutionEngine *v4, Status stat o->put((s = v4->newString(QStringLiteral("NETWORK_ERROR"))), (v = QV4::Primitive::fromInt32(NetworkError))); o->put((s = v4->newString(QStringLiteral("EXCEPTION"))), (v = QV4::Primitive::fromInt32(Exception))); o->put((s = v4->newString(QStringLiteral("status"))), (v = QV4::Primitive::fromInt32(status))); + if (!statusText.isEmpty()) + o->put((s = v4->newString(QStringLiteral("statusText"))), (v = v4->newString(statusText))); return o.asReturnedValue(); } @@ -227,7 +230,8 @@ QV4::ReturnedValue QV4Include::method_include(const QV4::FunctionObject *b, cons } else { QScopedPointer script; - script.reset(QV4::Script::createFromFileOrCache(scope.engine, qmlcontext, localFile, url)); + QString error; + script.reset(QV4::Script::createFromFileOrCache(scope.engine, qmlcontext, localFile, url, &error)); if (!script.isNull()) { script->parse(); @@ -242,7 +246,7 @@ QV4::ReturnedValue QV4Include::method_include(const QV4::FunctionObject *b, cons result = resultValue(scope.engine, Ok); } } else { - result = resultValue(scope.engine, NetworkError); + result = resultValue(scope.engine, NetworkError, error); } callback(callbackFunction, result); diff --git a/src/qml/jsruntime/qv4include_p.h b/src/qml/jsruntime/qv4include_p.h index 8015722afc..70ccfbf223 100644 --- a/src/qml/jsruntime/qv4include_p.h +++ b/src/qml/jsruntime/qv4include_p.h @@ -88,7 +88,8 @@ private: QV4::ReturnedValue result(); - static QV4::ReturnedValue resultValue(QV4::ExecutionEngine *v4, Status status = Loading); + static QV4::ReturnedValue resultValue(QV4::ExecutionEngine *v4, Status status = Loading, + const QString &statusText = QString()); static void callback(const QV4::Value &callback, const QV4::Value &status); QV4::ExecutionEngine *v4; diff --git a/src/qml/jsruntime/qv4script.cpp b/src/qml/jsruntime/qv4script.cpp index 267c93952d..b4d9e11716 100644 --- a/src/qml/jsruntime/qv4script.cpp +++ b/src/qml/jsruntime/qv4script.cpp @@ -223,17 +223,28 @@ QQmlRefPointer Script::precompile(QV4::Compi return cg.generateCompilationUnit(/*generate unit data*/false); } -Script *Script::createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl) +Script *Script::createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl, QString *error) { - if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(originalUrl)) { + if (error) + error->clear(); + + QQmlMetaType::CachedUnitLookupError cacheError = QQmlMetaType::CachedUnitLookupError::NoError; + if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(originalUrl, &cacheError)) { QQmlRefPointer jsUnit; jsUnit.adopt(new QV4::CompiledData::CompilationUnit(cachedUnit)); return new QV4::Script(engine, qmlContext, jsUnit); } QFile f(fileName); - if (!f.open(QIODevice::ReadOnly)) + if (!f.open(QIODevice::ReadOnly)) { + if (error) { + if (cacheError == QQmlMetaType::CachedUnitLookupError::VersionMismatch) + *error = originalUrl.toString() + QString::fromUtf8(" was compiled ahead of time with an incompatible version of Qt and the original source code cannot be found. Please recompile"); + else + *error = QString::fromUtf8("Error opening source file %1: %2").arg(originalUrl.toString()).arg(f.errorString()); + } return nullptr; + } QByteArray data = f.readAll(); QString sourceCode = QString::fromUtf8(data); diff --git a/src/qml/jsruntime/qv4script_p.h b/src/qml/jsruntime/qv4script_p.h index cb03c6b064..b4ac150044 100644 --- a/src/qml/jsruntime/qv4script_p.h +++ b/src/qml/jsruntime/qv4script_p.h @@ -101,7 +101,7 @@ struct Q_QML_EXPORT Script { QV4::Compiler::Module *module, Compiler::JSUnitGenerator *unitGenerator, const QString &fileName, const QString &finalUrl, const QString &source, QList *reportedErrors = nullptr, QQmlJS::Directives *directivesCollector = nullptr); - static Script *createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl); + static Script *createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl, QString *error); static ReturnedValue evaluate(ExecutionEngine *engine, const QString &script, QmlContext *qmlContext); }; diff --git a/src/qml/qml/qqmlmetatype.cpp b/src/qml/qml/qqmlmetatype.cpp index 7754f0fddc..8fda7f6f77 100644 --- a/src/qml/qml/qqmlmetatype.cpp +++ b/src/qml/qml/qqmlmetatype.cpp @@ -51,6 +51,7 @@ #include #include #include +#include #include #include @@ -63,6 +64,8 @@ #include #include "qqmlcomponent.h" +Q_DECLARE_LOGGING_CATEGORY(DBG_DISK_CACHE) + QT_BEGIN_NAMESPACE struct QQmlMetaTypeData @@ -2539,18 +2542,46 @@ QList QQmlMetaType::qmlSingletonTypes() return retn; } -const QV4::CompiledData::Unit *QQmlMetaType::findCachedCompilationUnit(const QUrl &uri) +const QV4::CompiledData::Unit *QQmlMetaType::findCachedCompilationUnit(const QUrl &uri, CachedUnitLookupError *status) { QMutexLocker lock(metaTypeDataLock()); QQmlMetaTypeData *data = metaTypeData(); for (const auto lookup : qAsConst(data->lookupCachedQmlUnit)) { - if (const QQmlPrivate::CachedQmlUnit *unit = lookup(uri)) + if (const QQmlPrivate::CachedQmlUnit *unit = lookup(uri)) { + QString error; + if (!unit->qmlData->verifyHeader(QDateTime(), &error)) { + qCDebug(DBG_DISK_CACHE) << "Error loading pre-compiled file " << uri << ":" << error; + if (status) + *status = CachedUnitLookupError::VersionMismatch; + return nullptr; + } + if (status) + *status = CachedUnitLookupError::NoError; return unit->qmlData; + } } + + if (status) + *status = CachedUnitLookupError::NoUnitFound; + return nullptr; } +void QQmlMetaType::prependCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler) +{ + QMutexLocker lock(metaTypeDataLock()); + QQmlMetaTypeData *data = metaTypeData(); + data->lookupCachedQmlUnit.prepend(handler); +} + +void QQmlMetaType::removeCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler) +{ + QMutexLocker lock(metaTypeDataLock()); + QQmlMetaTypeData *data = metaTypeData(); + data->lookupCachedQmlUnit.removeAll(handler); +} + /*! Returns the pretty QML type name (e.g. 'Item' instead of 'QtQuickItem') for the given object. */ diff --git a/src/qml/qml/qqmlmetatype_p.h b/src/qml/qml/qqmlmetatype_p.h index 07bef526ba..cd7afc8a01 100644 --- a/src/qml/qml/qqmlmetatype_p.h +++ b/src/qml/qml/qqmlmetatype_p.h @@ -133,7 +133,17 @@ public: static QList parentFunctions(); - static const QV4::CompiledData::Unit *findCachedCompilationUnit(const QUrl &uri); + enum class CachedUnitLookupError { + NoError, + NoUnitFound, + VersionMismatch + }; + + static const QV4::CompiledData::Unit *findCachedCompilationUnit(const QUrl &uri, CachedUnitLookupError *status); + + // used by tst_qqmlcachegen.cpp + static void prependCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler); + static void removeCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler); static bool namespaceContainsRegistrations(const QString &, int majorVersion); diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index 2b778b0b63..5572fdad44 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -1259,6 +1259,7 @@ void QQmlTypeLoader::setData(QQmlDataBlob *blob, const QByteArray &data) QML_MEMORY_SCOPE_URL(blob->url()); QQmlDataBlob::SourceCodeData d; d.inlineSourceCode = QString::fromUtf8(data); + d.hasInlineSourceCode = true; setData(blob, d); } @@ -1670,9 +1671,11 @@ QQmlTypeData *QQmlTypeLoader::getType(const QUrl &url, Mode mode) typeData = new QQmlTypeData(url, this); // TODO: if (compiledData == 0), is it safe to omit this insertion? m_typeCache.insert(url, typeData); - if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(typeData->url())) { + QQmlMetaType::CachedUnitLookupError error = QQmlMetaType::CachedUnitLookupError::NoError; + if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(typeData->url(), &error)) { QQmlTypeLoader::loadWithCachedUnit(typeData, cachedUnit, mode); } else { + typeData->setCachedUnitStatus(error); QQmlTypeLoader::load(typeData, mode); } } else if ((mode == PreferSynchronous || mode == Synchronous) && QQmlFile::isSynchronous(url)) { @@ -1727,9 +1730,11 @@ QQmlScriptBlob *QQmlTypeLoader::getScript(const QUrl &url) scriptBlob = new QQmlScriptBlob(url, this); m_scriptCache.insert(url, scriptBlob); - if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(scriptBlob->url())) { + QQmlMetaType::CachedUnitLookupError error; + if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(scriptBlob->url(), &error)) { QQmlTypeLoader::loadWithCachedUnit(scriptBlob, cachedUnit); } else { + scriptBlob->setCachedUnitStatus(error); QQmlTypeLoader::load(scriptBlob); } } @@ -2428,8 +2433,13 @@ void QQmlTypeData::dataReceived(const SourceCodeData &data) if (isError()) return; - if (!m_backupSourceCode.exists()) { - setError(QQmlTypeLoader::tr("No such file or directory")); + if (!m_backupSourceCode.exists() || m_backupSourceCode.isEmpty()) { + if (m_cachedUnitStatus == QQmlMetaType::CachedUnitLookupError::VersionMismatch) + setError(QQmlTypeLoader::tr("File was compiled ahead of time with an incompatible version of Qt and the original file cannot be found. Please recompile")); + else if (!m_backupSourceCode.exists()) + setError(QQmlTypeLoader::tr("No such file or directory")); + else + setError(QQmlTypeLoader::tr("File is empty")); return; } @@ -2981,6 +2991,13 @@ void QQmlScriptBlob::dataReceived(const SourceCodeData &data) } } + if (!data.exists()) { + if (m_cachedUnitStatus == QQmlMetaType::CachedUnitLookupError::VersionMismatch) + setError(QQmlTypeLoader::tr("File was compiled ahead of time with an incompatible version of Qt and the original file cannot be found. Please recompile")); + else + setError(QQmlTypeLoader::tr("No such file or directory")); + return; + } QmlIR::Document irUnit(isDebugging()); @@ -3178,7 +3195,7 @@ void QQmlQmldirData::initializeFromCachedUnit(const QV4::CompiledData::Unit *) QString QQmlDataBlob::SourceCodeData::readAll(QString *error) const { error->clear(); - if (!inlineSourceCode.isEmpty()) + if (hasInlineSourceCode) return inlineSourceCode; QFile f(fileInfo.absoluteFilePath()); @@ -3205,7 +3222,7 @@ QString QQmlDataBlob::SourceCodeData::readAll(QString *error) const QDateTime QQmlDataBlob::SourceCodeData::sourceTimeStamp() const { - if (!inlineSourceCode.isEmpty()) + if (hasInlineSourceCode) return QDateTime(); QDateTime timeStamp = fileInfo.lastModified(); @@ -3220,11 +3237,18 @@ QDateTime QQmlDataBlob::SourceCodeData::sourceTimeStamp() const bool QQmlDataBlob::SourceCodeData::exists() const { - if (!inlineSourceCode.isEmpty()) + if (hasInlineSourceCode) return true; return fileInfo.exists(); } +bool QQmlDataBlob::SourceCodeData::isEmpty() const +{ + if (hasInlineSourceCode) + return inlineSourceCode.isEmpty(); + return fileInfo.size() == 0; +} + QT_END_NAMESPACE #include "qqmltypeloader.moc" diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h index 4665d342c1..5988632547 100644 --- a/src/qml/qml/qqmltypeloader_p.h +++ b/src/qml/qml/qqmltypeloader_p.h @@ -140,11 +140,13 @@ public: QString readAll(QString *error) const; QDateTime sourceTimeStamp() const; bool exists() const; + bool isEmpty() const; private: friend class QQmlDataBlob; friend class QQmlTypeLoader; QString inlineSourceCode; QFileInfo fileInfo; + bool hasInlineSourceCode = false; }; protected: @@ -271,6 +273,8 @@ public: const QQmlImports &imports() const { return m_importCache; } + void setCachedUnitStatus(QQmlMetaType::CachedUnitLookupError status) { m_cachedUnitStatus = status; } + protected: bool addImport(const QV4::CompiledData::Import *import, QList *errors); @@ -293,6 +297,7 @@ public: QQmlImports m_importCache; QHash m_unresolvedImports; QList m_qmldirs; + QQmlMetaType::CachedUnitLookupError m_cachedUnitStatus = QQmlMetaType::CachedUnitLookupError::NoError; }; QQmlTypeLoader(QQmlEngine *); diff --git a/src/qml/types/qquickworkerscript.cpp b/src/qml/types/qquickworkerscript.cpp index 80c2d3a4bc..ef92e4108d 100644 --- a/src/qml/types/qquickworkerscript.cpp +++ b/src/qml/types/qquickworkerscript.cpp @@ -397,9 +397,11 @@ void QQuickWorkerScriptEnginePrivate::processLoad(int id, const QUrl &url) QV4::Scoped qmlContext(scope, getWorker(script)); Q_ASSERT(!!qmlContext); - program.reset(QV4::Script::createFromFileOrCache(v4, qmlContext, fileName, url)); + QString error; + program.reset(QV4::Script::createFromFileOrCache(v4, qmlContext, fileName, url, &error)); if (program.isNull()) { - qWarning().nospace() << "WorkerScript: Cannot find source file " << url.toString(); + if (!error.isEmpty()) + qWarning().nospace() << error; return; } -- cgit v1.2.3 From 80592dcf0df3bce6177e8e051e88af7b9e6b3f6e Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 12 Mar 2018 16:33:01 +0100 Subject: Tighten QML cache version checking Don't just include the "compile hash" of QtQml in the dependencies hash of QML files but use a dedicated field in the data structure, that we will also fill in when generating cache files ahead of time. This ensures that AOT generated cache files are considered invalid even when switching between different sha1s of declarative. Task-number: QTBUG-66986 Change-Id: I3d8ee103fd1a33a5b4c4576b3a2703fcd09712dd Reviewed-by: Erik Verbruggen --- src/qml/compiler/compiler.pri | 2 -- src/qml/compiler/qv4compileddata.cpp | 45 +++++++++--------------------------- src/qml/compiler/qv4compileddata_p.h | 6 +++-- src/qml/compiler/qv4compiler.cpp | 4 ++++ src/qml/qml.pro | 3 ++- 5 files changed, 21 insertions(+), 39 deletions(-) (limited to 'src/qml') diff --git a/src/qml/compiler/compiler.pri b/src/qml/compiler/compiler.pri index 2ca0c39acc..95096db51d 100644 --- a/src/qml/compiler/compiler.pri +++ b/src/qml/compiler/compiler.pri @@ -40,8 +40,6 @@ SOURCES += \ unix: SOURCES += $$PWD/qv4compilationunitmapper_unix.cpp else: SOURCES += $$PWD/qv4compilationunitmapper_win.cpp - -qtConfig(private_tests):qtConfig(dlopen): QMAKE_USE_PRIVATE += libdl } gcc { diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp index 6220c1bc11..8dcc068a06 100644 --- a/src/qml/compiler/qv4compileddata.cpp +++ b/src/qml/compiler/qv4compileddata.cpp @@ -70,18 +70,14 @@ #include -#if defined(QT_BUILD_INTERNAL) -#if defined(Q_OS_UNIX) && !defined(QT_NO_DYNAMIC_CAST) -#include -#endif -#endif - QT_BEGIN_NAMESPACE namespace QV4 { namespace CompiledData { +static_assert(sizeof(Unit::libraryVersionHash) >= QML_COMPILE_HASH_LENGTH + 1, "Compile hash length exceeds reserved size in data structure. Please adjust and bump the format version"); + #if !defined(V4_BOOTSTRAP) static QString cacheFilePath(const QUrl &url) { @@ -686,32 +682,6 @@ void ResolvedTypeReference::doDynamicTypeCheck() isFullyDynamicType = qtTypeInherits(mo); } -static QByteArray ownLibraryChecksum() -{ - static QByteArray libraryChecksum; - static bool checksumInitialized = false; - if (checksumInitialized) - return libraryChecksum; - checksumInitialized = true; -#if defined(QT_BUILD_INTERNAL) && !defined(QT_NO_DYNAMIC_CAST) && QT_CONFIG(dlopen) - // This is a bit of a hack to make development easier. When hacking on the code generator - // the cache files may end up being re-used. To avoid that we also add the checksum of - // the QtQml library. - Dl_info libInfo; - if (dladdr(reinterpret_cast(&ownLibraryChecksum), &libInfo) != 0) { - QFile library(QFile::decodeName(libInfo.dli_fname)); - if (library.open(QIODevice::ReadOnly)) { - QCryptographicHash hash(QCryptographicHash::Md5); - hash.addData(&library); - libraryChecksum = hash.result(); - } - } -#else - libraryChecksum = QByteArray(QML_COMPILE_HASH); -#endif - return libraryChecksum; -} - bool ResolvedTypeReferenceMap::addToHash(QCryptographicHash *hash, QQmlEngine *engine) const { for (auto it = constBegin(), end = constEnd(); it != end; ++it) { @@ -719,8 +689,6 @@ bool ResolvedTypeReferenceMap::addToHash(QCryptographicHash *hash, QQmlEngine *e return false; } - hash->addData(ownLibraryChecksum()); - return true; } @@ -785,6 +753,15 @@ bool Unit::verifyHeader(QDateTime expectedSourceTimeStamp, QString *errorString) } } +#if defined(QML_COMPILE_HASH) + if (qstrcmp(QML_COMPILE_HASH, libraryVersionHash) != 0) { + *errorString = QStringLiteral("QML library version mismatch. Expected compile hash does not match"); + return false; + } +#else +#error "QML_COMPILE_HASH must be defined for the build of QtDeclarative to ensure version checking for cache files" +#endif + return true; #else Q_UNUSED(expectedSourceTimeStamp) diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index 49360bc3f1..1df9d6794f 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -72,7 +72,7 @@ QT_BEGIN_NAMESPACE // Bump this whenever the compiler data structures change in an incompatible way. -#define QV4_DATA_STRUCTURE_VERSION 0x18 +#define QV4_DATA_STRUCTURE_VERSION 0x19 class QIODevice; class QQmlPropertyCache; @@ -688,6 +688,8 @@ struct Unit quint32_le unitSize; // Size of the Unit and any depending data. // END DO NOT CHANGE THESE FIELDS EVER + char libraryVersionHash[48]; + char md5Checksum[16]; // checksum of all bytes following this field. void generateChecksum(); @@ -793,7 +795,7 @@ struct Unit } }; -static_assert(sizeof(Unit) == 144, "Unit structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target"); +static_assert(sizeof(Unit) == 192, "Unit structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target"); struct TypeReference { diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index ccc909c199..c9e535c93f 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -48,6 +48,9 @@ #include #include +// generated by qmake: +#include "qml_compile_hash_p.h" + QV4::Compiler::StringTableGenerator::StringTableGenerator() { clear(); @@ -396,6 +399,7 @@ QV4::CompiledData::Unit QV4::Compiler::JSUnitGenerator::generateHeader(QV4::Comp unit.flags |= module->unitFlags; unit.version = QV4_DATA_STRUCTURE_VERSION; unit.qtVersion = QT_VERSION; + qstrcpy(unit.libraryVersionHash, QML_COMPILE_HASH); memset(unit.md5Checksum, 0, sizeof(unit.md5Checksum)); memset(unit.dependencyMD5Checksum, 0, sizeof(unit.dependencyMD5Checksum)); diff --git a/src/qml/qml.pro b/src/qml/qml.pro index f75bfa0313..2137877427 100644 --- a/src/qml/qml.pro +++ b/src/qml/qml.pro @@ -36,7 +36,8 @@ DEFINES += QT_NO_FOREACH } compile_hash_contents = \ "// Generated file, DO NOT EDIT" \ - "$${LITERAL_HASH}define QML_COMPILE_HASH \"$$QML_COMPILE_HASH\"" + "$${LITERAL_HASH}define QML_COMPILE_HASH \"$$QML_COMPILE_HASH\"" \ + "$${LITERAL_HASH}define QML_COMPILE_HASH_LENGTH $$str_size($$QML_COMPILE_HASH)" write_file("$$OUT_PWD/qml_compile_hash_p.h", compile_hash_contents)|error() } -- cgit v1.2.3 From cc62a35bbebc9d069c9dbaeb703dfd6afea548e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Vr=C3=A1til?= Date: Tue, 20 Mar 2018 18:14:20 +0100 Subject: Fix QQmlListModel crash when appending an empty array in debug mode Calling QQmlListModel::append() with an empty JS array triggers an assert in QAbstractItemModel::beginInsertRows() because it's called with negative "last" parameter. Change-Id: I202da260d79f2e6677c663c5785ff754c715fef8 Reviewed-by: Simon Hausmann --- src/qml/types/qqmllistmodel.cpp | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'src/qml') diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp index d4fc02cd3e..c4e33d572d 100644 --- a/src/qml/types/qqmllistmodel.cpp +++ b/src/qml/types/qqmllistmodel.cpp @@ -2351,21 +2351,22 @@ void QQmlListModel::append(QQmlV4Function *args) QV4::ScopedObject argObject(scope); int objectArrayLength = objectArray->getLength(); + if (objectArrayLength > 0) { + int index = count(); + emitItemsAboutToBeInserted(index, objectArrayLength); - int index = count(); - emitItemsAboutToBeInserted(index, objectArrayLength); + for (int i=0 ; i < objectArrayLength ; ++i) { + argObject = objectArray->getIndexed(i); - for (int i=0 ; i < objectArrayLength ; ++i) { - argObject = objectArray->getIndexed(i); - - if (m_dynamicRoles) { - m_modelObjects.append(DynamicRoleModelNode::create(scope.engine->variantMapFromJS(argObject), this)); - } else { - m_listModel->append(argObject); + if (m_dynamicRoles) { + m_modelObjects.append(DynamicRoleModelNode::create(scope.engine->variantMapFromJS(argObject), this)); + } else { + m_listModel->append(argObject); + } } - } - emitItemsInserted(); + emitItemsInserted(); + } } else if (argObject) { int index; -- cgit v1.2.3