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 <simon.hausmann@qt.io>
Change-Id: I395153afaf7c835d0119690ee7f4b915e6f90d4a
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
This commit is contained in:
Oswald Buddenhagen 2017-08-14 18:30:29 +02:00 committed by Simon Hausmann
parent e6fad35155
commit 190aa94be7
7 changed files with 53 additions and 28 deletions

View File

@ -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;
}

View File

@ -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;
};

View File

@ -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)

View File

@ -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();

View File

@ -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); }

View File

@ -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)

View File

@ -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<QString, int> fileIdMap;
#ifdef PROEVALUATOR_THREAD_SAFE
QMutex fileIdMutex;
#endif
int fileIdCounter = 0;
ProFileCache *m_cache;
QMakeParserHandler *m_handler;
QMakeVfs *m_vfs;