From 190aa94be7f5e146bef44862b974d733755cec85 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Mon, 14 Aug 2017 18:30:29 +0200 Subject: Change source identifier type in ProString The strings remember in which file they were created/assigned. However, this used a non-counting reference to a ProFile, which could become dangling. If a subsequent ProFile re-used the exact same address, a string's source would be mis-identified, which would be fatal in conjunction with discard_from(). Since we actually need only a unique id for comparison, let's use an integer for that. Task-number: QTBUG-62434 Started-by: Simon Hausmann Change-Id: I395153afaf7c835d0119690ee7f4b915e6f90d4a Reviewed-by: Simon Hausmann --- qmake/library/proitems.cpp | 5 +++-- qmake/library/proitems.h | 10 ++++++---- qmake/library/qmakebuiltins.cpp | 13 ++++++------- qmake/library/qmakeevaluator.cpp | 14 +++++++++++--- qmake/library/qmakeevaluator.h | 3 ++- qmake/library/qmakeparser.cpp | 27 +++++++++++++++++---------- qmake/library/qmakeparser.h | 9 ++++++++- 7 files changed, 53 insertions(+), 28 deletions(-) (limited to 'qmake/library') diff --git a/qmake/library/proitems.cpp b/qmake/library/proitems.cpp index ff1236f64a..1744304c67 100644 --- a/qmake/library/proitems.cpp +++ b/qmake/library/proitems.cpp @@ -477,9 +477,10 @@ bool ProStringList::contains(const char *str, Qt::CaseSensitivity cs) const return false; } -ProFile::ProFile(const QString &fileName) +ProFile::ProFile(int id, const QString &fileName) : m_refCount(1), m_fileName(fileName), + m_id(id), m_ok(true), m_hostBuild(false) { @@ -496,7 +497,7 @@ ProString ProFile::getStr(const ushort *&tPtr) { uint len = *tPtr++; ProString ret(items(), tPtr - tokPtr(), len); - ret.setSource(this); + ret.setSource(m_id); tPtr += len; return ret; } diff --git a/qmake/library/proitems.h b/qmake/library/proitems.h index c81e205699..6ce8c98789 100644 --- a/qmake/library/proitems.h +++ b/qmake/library/proitems.h @@ -73,8 +73,8 @@ public: void setValue(const QString &str); void clear() { m_string.clear(); m_length = 0; } ProString &setSource(const ProString &other) { m_file = other.m_file; return *this; } - ProString &setSource(const ProFile *pro) { m_file = pro; return *this; } - const ProFile *sourceFile() const { return m_file; } + ProString &setSource(int id) { m_file = id; return *this; } + int sourceFile() const { return m_file; } ProString &prepend(const ProString &other); ProString &append(const ProString &other, bool *pending = 0); @@ -163,7 +163,7 @@ private: QString m_string; int m_offset, m_length; - const ProFile *m_file; + int m_file; mutable uint m_hash; QChar *prepareExtend(int extraLen, int thisTarget, int extraTarget); uint updatedHash() const; @@ -340,9 +340,10 @@ enum ProToken { class QMAKE_EXPORT ProFile { public: - explicit ProFile(const QString &fileName); + ProFile(int id, const QString &fileName); ~ProFile(); + int id() const { return m_id; } QString fileName() const { return m_fileName; } QString directoryName() const { return m_directoryName; } const QString &items() const { return m_proitems; } @@ -367,6 +368,7 @@ private: QString m_proitems; QString m_fileName; QString m_directoryName; + int m_id; bool m_ok; bool m_hostBuild; }; diff --git a/qmake/library/qmakebuiltins.cpp b/qmake/library/qmakebuiltins.cpp index 2bb6f2e12d..23867d09ee 100644 --- a/qmake/library/qmakebuiltins.cpp +++ b/qmake/library/qmakebuiltins.cpp @@ -713,9 +713,9 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateBuiltinExpand( after = args[3]; const ProStringList &var = values(map(args.at(0))); if (!var.isEmpty()) { - const ProFile *src = currentProFile(); + int src = currentFileId(); for (const ProString &v : var) - if (const ProFile *s = v.sourceFile()) { + if (int s = v.sourceFile()) { src = s; break; } @@ -1064,7 +1064,7 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateBuiltinExpand( dirs.append(fname + QLatin1Char('/')); } if (regex.exactMatch(qdir[i])) - ret += ProString(fname).setSource(currentProFile()); + ret += ProString(fname).setSource(currentFileId()); } } } @@ -1331,7 +1331,7 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateBuiltinConditional( return ReturnFalse; } QString fn = resolvePath(args.at(0).toQString(m_tmp1)); - ProFile *pro = m_parser->parsedProFile(fn, QMakeParser::ParseOnlyCached); + int pro = m_parser->idForFileName(fn); if (!pro) return ReturnFalse; ProValueMap &vmap = m_valuemapStack.first(); @@ -1351,18 +1351,17 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateBuiltinConditional( ++vit; } for (auto fit = m_functionDefs.testFunctions.begin(); fit != m_functionDefs.testFunctions.end(); ) { - if (fit->pro() == pro) + if (fit->pro()->id() == pro) fit = m_functionDefs.testFunctions.erase(fit); else ++fit; } for (auto fit = m_functionDefs.replaceFunctions.begin(); fit != m_functionDefs.replaceFunctions.end(); ) { - if (fit->pro() == pro) + if (fit->pro()->id() == pro) fit = m_functionDefs.replaceFunctions.erase(fit); else ++fit; } - pro->deref(); ProStringList &iif = m_valuemapStack.first()[ProKey("QMAKE_INTERNAL_INCLUDED_FILES")]; int idx = iif.indexOf(ProString(fn)); if (idx >= 0) diff --git a/qmake/library/qmakeevaluator.cpp b/qmake/library/qmakeevaluator.cpp index b0ed01e3aa..a07f84e242 100644 --- a/qmake/library/qmakeevaluator.cpp +++ b/qmake/library/qmakeevaluator.cpp @@ -271,13 +271,13 @@ void QMakeEvaluator::skipHashStr(const ushort *&tokPtr) // FIXME: this should not build new strings for direct sections. // Note that the E_SPRINTF and E_LIST implementations rely on the deep copy. -ProStringList QMakeEvaluator::split_value_list(const QStringRef &vals, const ProFile *source) +ProStringList QMakeEvaluator::split_value_list(const QStringRef &vals, int source) { QString build; ProStringList ret; if (!source) - source = currentProFile(); + source = currentFileId(); const QChar *vals_data = vals.data(); const int vals_len = vals.length(); @@ -1299,7 +1299,7 @@ void QMakeEvaluator::setupProject() { setTemplate(); ProValueMap &vars = m_valuemapStack.top(); - ProFile *proFile = currentProFile(); + int proFile = currentFileId(); vars[ProKey("TARGET")] << ProString(QFileInfo(currentFileName()).baseName()).setSource(proFile); vars[ProKey("_PRO_FILE_")] << ProString(currentFileName()).setSource(proFile); vars[ProKey("_PRO_FILE_PWD_")] << ProString(currentDirectory()).setSource(proFile); @@ -1593,6 +1593,14 @@ ProFile *QMakeEvaluator::currentProFile() const return 0; } +int QMakeEvaluator::currentFileId() const +{ + ProFile *pro = currentProFile(); + if (pro) + return pro->id(); + return 0; +} + QString QMakeEvaluator::currentFileName() const { ProFile *pro = currentProFile(); diff --git a/qmake/library/qmakeevaluator.h b/qmake/library/qmakeevaluator.h index 5948bd7d14..fcac0388c7 100644 --- a/qmake/library/qmakeevaluator.h +++ b/qmake/library/qmakeevaluator.h @@ -176,12 +176,13 @@ public: void setTemplate(); - ProStringList split_value_list(const QStringRef &vals, const ProFile *source = 0); + ProStringList split_value_list(const QStringRef &vals, int source = 0); VisitReturn expandVariableReferences(const ushort *&tokPtr, int sizeHint, ProStringList *ret, bool joined); QString currentFileName() const; QString currentDirectory() const; ProFile *currentProFile() const; + int currentFileId() const; QString resolvePath(const QString &fileName) const { return QMakeInternal::IoUtils::resolvePath(currentDirectory(), fileName); } diff --git a/qmake/library/qmakeparser.cpp b/qmake/library/qmakeparser.cpp index 78350c76c4..b5d89c1ba6 100644 --- a/qmake/library/qmakeparser.cpp +++ b/qmake/library/qmakeparser.cpp @@ -167,7 +167,7 @@ QMakeParser::QMakeParser(ProFileCache *cache, QMakeVfs *vfs, QMakeParserHandler ProFile *QMakeParser::parsedProFile(const QString &fileName, ParseFlags flags) { ProFile *pro; - if ((flags & (ParseUseCache|ParseOnlyCached)) && m_cache) { + if ((flags & ParseUseCache) && m_cache) { ProFileCache::Entry *ent; #ifdef PROPARSER_THREAD_SAFE QMutexLocker locker(&m_cache->mutex); @@ -189,13 +189,13 @@ ProFile *QMakeParser::parsedProFile(const QString &fileName, ParseFlags flags) #endif if ((pro = ent->pro)) pro->ref(); - } else if (!(flags & ParseOnlyCached)) { + } else { ent = &m_cache->parsed_files[fileName]; #ifdef PROPARSER_THREAD_SAFE ent->locker = new ProFileCache::Entry::Locker; locker.unlock(); #endif - pro = new ProFile(fileName); + pro = new ProFile(idForFileName(fileName), fileName); if (!read(pro, flags)) { delete pro; pro = 0; @@ -214,17 +214,13 @@ ProFile *QMakeParser::parsedProFile(const QString &fileName, ParseFlags flags) ent->locker = 0; } #endif - } else { - pro = 0; } - } else if (!(flags & ParseOnlyCached)) { - pro = new ProFile(fileName); + } else { + pro = new ProFile(idForFileName(fileName), fileName); if (!read(pro, flags)) { delete pro; pro = 0; } - } else { - pro = 0; } return pro; } @@ -232,11 +228,22 @@ ProFile *QMakeParser::parsedProFile(const QString &fileName, ParseFlags flags) ProFile *QMakeParser::parsedProBlock( const QStringRef &contents, const QString &name, int line, SubGrammar grammar) { - ProFile *pro = new ProFile(name); + ProFile *pro = new ProFile(0, name); read(pro, contents, line, grammar); return pro; } +int QMakeParser::idForFileName(const QString &fileName) +{ +#ifdef PROPARSER_THREAD_SAFE + QMutexLocker lck(&fileIdMutex); +#endif + int &place = fileIdMap[fileName]; + if (!place) + place = ++fileIdCounter; + return place; +} + void QMakeParser::discardFileFromCache(const QString &fileName) { if (m_cache) diff --git a/qmake/library/qmakeparser.h b/qmake/library/qmakeparser.h index 6557ad65b5..a29e9c227d 100644 --- a/qmake/library/qmakeparser.h +++ b/qmake/library/qmakeparser.h @@ -78,7 +78,6 @@ public: enum ParseFlag { ParseDefault = 0, ParseUseCache = 1, - ParseOnlyCached = 2, ParseReportMissing = 4 }; Q_DECLARE_FLAGS(ParseFlags, ParseFlag) @@ -91,6 +90,8 @@ public: ProFile *parsedProBlock(const QStringRef &contents, const QString &name, int line = 0, SubGrammar grammar = FullGrammar); + int idForFileName(const QString &fileName); + void discardFileFromCache(const QString &fileName); #ifdef PROPARSER_DEBUG @@ -181,6 +182,12 @@ private: QString m_tmp; // Temporary for efficient toQString + QHash fileIdMap; +#ifdef PROEVALUATOR_THREAD_SAFE + QMutex fileIdMutex; +#endif + int fileIdCounter = 0; + ProFileCache *m_cache; QMakeParserHandler *m_handler; QMakeVfs *m_vfs; -- cgit v1.2.3