Skip to content

Commit 42a65c5

Browse files
rebnridgwaydanmar
rebnridgway
authored and
danmar
committed
Fix crash bug #8579 (danmar#1238)
* Added declaration for deletePrevious function * Added definition for deletePrevious function * Fixed crash from deleteThis invalidating pointers The crash was caused by deleteThis() invalidating the pointer to a constant variable usage. This happened when a usage followed an assignment. This fixes bug #8579. * Added tokensFront to match tokensBack This means deletePrevious can set the list's front if necessary. * Initialised tokensFront in appropriate places * Switched to using default Token constructor * Switched to using Token default constructor * Switched to using default constructor for Token * Added missing argument to Token constructor * Changed to use default constructor for Tokens * Switched to using default constructor for Tokens * Switched to using default constructor for Token * Added new test for deleting front Token Also made sure to use the correct constructor for Token in other tests. * Syntax error * Replaced tokensFront and tokensBack with a struct This decreases the size of the Token class for performance purposes. * Replaced tokensFront and tokensBack with a struct * Added tokensFrontBack to destructor * Reworked to use TokensBackFront struct Also ran astyle. * Reworked to use TokenList's TokensFrontBack member * Reworked to use TokensFrontBack struct * Reworked to use TokensFrontBack struct * Reworked to work with TokensFrontBack struct * Removed unnecessary scope operator * Added missing parentheses * Fixed syntax error * Removed unnecessary constructor * Default constructor now 0-initialises everything This is safer for not using a temporary TokensFrontBack object, and doesn't use delegating constructors which aren't supported yet. * Fixed unsafe null check * Added missing explicit keyword * Fixing stylistic nits Removed default constructor as it has been superseded by the single-argument constructor with a default argument value. Renamed listEnds to tokensFrontBack. Fixed if statement that was supposed to be adding safety but would actually cause a crash if tokensFrontBack was null. * Fixing stylistic nits Removed default constructor and replaced it with a single-argument constructor with a default value. * Fixing stylistic nits Renamed _listEnds to _tokensFrontBack. * Fixing stylistic nits Renamed _listEnds to _tokensFrontBack.
1 parent 1af983d commit 42a65c5

File tree

10 files changed

+172
-117
lines changed

10 files changed

+172
-117
lines changed

‎lib/checkio.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,7 +1329,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings *settings,
13291329
top = top->astParent();
13301330
const ValueType *valuetype = top->argumentType();
13311331
if (valuetype && valuetype->type >= ValueType::Type::BOOL) {
1332-
typeToken = tempToken = new Token(nullptr);
1332+
typeToken = tempToken = new Token();
13331333
if (valuetype->pointer && valuetype->constness & 1) {
13341334
tempToken->str("const");
13351335
tempToken->insertToken("a");
@@ -1406,7 +1406,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings *settings,
14061406
if (function->retType->classScope->enumType)
14071407
typeToken = function->retType->classScope->enumType;
14081408
else {
1409-
tempToken = new Token(nullptr);
1409+
tempToken = new Token();
14101410
tempToken->fileIndex(tok1->fileIndex());
14111411
tempToken->linenr(tok1->linenr());
14121412
tempToken->str("int");
@@ -1427,7 +1427,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings *settings,
14271427
if (function->retType->classScope->enumType)
14281428
typeToken = function->retType->classScope->enumType;
14291429
else {
1430-
tempToken = new Token(nullptr);
1430+
tempToken = new Token();
14311431
tempToken->fileIndex(tok1->fileIndex());
14321432
tempToken->linenr(tok1->linenr());
14331433
tempToken->str("int");
@@ -1452,7 +1452,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings *settings,
14521452
// check for some common well known functions
14531453
else if (isCPP && ((Token::Match(tok1->previous(), "%var% . size|empty|c_str ( ) [,)]") && isStdContainer(tok1->previous())) ||
14541454
(Token::Match(tok1->previous(), "] . size|empty|c_str ( ) [,)]") && isStdContainer(tok1->previous()->link()->previous())))) {
1455-
tempToken = new Token(nullptr);
1455+
tempToken = new Token();
14561456
tempToken->fileIndex(tok1->fileIndex());
14571457
tempToken->linenr(tok1->linenr());
14581458
if (tok1->next()->str() == "size") {
@@ -1513,7 +1513,7 @@ CheckIO::ArgumentInfo::ArgumentInfo(const Token * arg, const Settings *settings,
15131513
if (variableInfo->type() && variableInfo->type()->classScope && variableInfo->type()->classScope->enumType)
15141514
typeToken = variableInfo->type()->classScope->enumType;
15151515
else {
1516-
tempToken = new Token(nullptr);
1516+
tempToken = new Token();
15171517
tempToken->fileIndex(tok1->fileIndex());
15181518
tempToken->linenr(tok1->linenr());
15191519
tempToken->str("int");
@@ -1552,7 +1552,7 @@ bool CheckIO::ArgumentInfo::isStdVectorOrString()
15521552
_template = true;
15531553
return true;
15541554
} else if (variableInfo->isStlType(stl_string)) {
1555-
tempToken = new Token(nullptr);
1555+
tempToken = new Token();
15561556
tempToken->fileIndex(variableInfo->typeStartToken()->fileIndex());
15571557
tempToken->linenr(variableInfo->typeStartToken()->linenr());
15581558
if (variableInfo->typeStartToken()->strAt(2) == "string")
@@ -1570,7 +1570,7 @@ bool CheckIO::ArgumentInfo::isStdVectorOrString()
15701570
_template = true;
15711571
return true;
15721572
} else if (Token::Match(nameTok, "std :: string|wstring")) {
1573-
tempToken = new Token(nullptr);
1573+
tempToken = new Token();
15741574
tempToken->fileIndex(variableInfo->typeStartToken()->fileIndex());
15751575
tempToken->linenr(variableInfo->typeStartToken()->linenr());
15761576
if (nameTok->strAt(2) == "string")

‎lib/checkmemoryleak.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::list<const Toke
688688
std::set<unsigned int> extravar;
689689

690690
// The first token should be ";"
691-
Token* rethead = new Token(nullptr);
691+
Token* rethead = new Token();
692692
rethead->str(";");
693693
rethead->linenr(tok->linenr());
694694
rethead->fileIndex(tok->fileIndex());

‎lib/token.cpp‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@
3535

3636
const std::list<ValueFlow::Value> Token::emptyValueList;
3737

38-
Token::Token(Token **tokens) :
39-
tokensBack(tokens),
38+
Token::Token(TokensFrontBack *tokensFrontBack) :
39+
tokensFrontBack(tokensFrontBack),
4040
_next(nullptr),
4141
_previous(nullptr),
4242
_link(nullptr),
@@ -219,8 +219,28 @@ void Token::deleteNext(unsigned long index)
219219

220220
if (_next)
221221
_next->previous(this);
222-
else if (tokensBack)
223-
*tokensBack = this;
222+
else if (tokensFrontBack)
223+
tokensFrontBack->back = this;
224+
}
225+
226+
void Token::deletePrevious(unsigned long index)
227+
{
228+
while (_previous && index) {
229+
Token *p = _previous;
230+
231+
// #8154 we are about to be unknown -> destroy the link to us
232+
if (p->_link && p->_link->_link == p)
233+
p->_link->link(nullptr);
234+
235+
_previous = p->previous();
236+
delete p;
237+
--index;
238+
}
239+
240+
if (_previous)
241+
_previous->next(this);
242+
else if (tokensFrontBack)
243+
tokensFrontBack->front = this;
224244
}
225245

226246
void Token::swapWithNext()
@@ -312,10 +332,10 @@ void Token::replace(Token *replaceThis, Token *start, Token *end)
312332
start->previous(replaceThis->previous());
313333
end->next(replaceThis->next());
314334

315-
if (end->tokensBack && *(end->tokensBack) == end) {
335+
if (end->tokensFrontBack && end->tokensFrontBack->back == end) {
316336
while (end->next())
317337
end = end->next();
318-
*(end->tokensBack) = end;
338+
end->tokensFrontBack->back = end;
319339
}
320340

321341
// Update _progressValue, fileIndex and linenr
@@ -906,7 +926,7 @@ void Token::insertToken(const std::string &tokenStr, const std::string &original
906926
if (_str.empty())
907927
newToken = this;
908928
else
909-
newToken = new Token(tokensBack);
929+
newToken = new Token(tokensFrontBack);
910930
newToken->str(tokenStr);
911931
if (!originalNameStr.empty())
912932
newToken->originalName(originalNameStr);
@@ -921,16 +941,16 @@ void Token::insertToken(const std::string &tokenStr, const std::string &original
921941
newToken->previous(this->previous());
922942
newToken->previous()->next(newToken);
923943
} /*else if (tokensFront?) {
924-
*tokensFront? = newToken;
925-
}*/
944+
*tokensFront? = newToken;
945+
}*/
926946
this->previous(newToken);
927947
newToken->next(this);
928948
} else {
929949
if (this->next()) {
930950
newToken->next(this->next());
931951
newToken->next()->previous(newToken);
932-
} else if (tokensBack) {
933-
*tokensBack = newToken;
952+
} else if (tokensFrontBack) {
953+
tokensFrontBack->back = newToken;
934954
}
935955
this->next(newToken);
936956
newToken->previous(this);

‎lib/token.h‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,14 @@ class Type;
3939
class ValueType;
4040
class Variable;
4141

42+
/**
43+
* @brief This struct stores pointers to the front and back tokens of the list this token is in.
44+
*/
45+
struct TokensFrontBack {
46+
Token *front;
47+
Token *back;
48+
};
49+
4250
/// @addtogroup Core
4351
/// @{
4452

@@ -54,10 +62,9 @@ class Variable;
5462
*/
5563
class CPPCHECKLIB Token {
5664
private:
57-
Token **tokensBack;
65+
TokensFrontBack* tokensFrontBack;
5866

5967
// Not implemented..
60-
Token();
6168
Token(const Token &);
6269
Token operator=(const Token &);
6370

@@ -71,7 +78,7 @@ class CPPCHECKLIB Token {
7178
eNone
7279
};
7380

74-
explicit Token(Token **tokens);
81+
explicit Token(TokensFrontBack *tokensFrontBack = nullptr);
7582
~Token();
7683

7784
template<typename T>
@@ -97,6 +104,11 @@ class CPPCHECKLIB Token {
97104
*/
98105
void deleteNext(unsigned long index = 1);
99106

107+
/**
108+
* Unlink and delete the previous 'index' tokens.
109+
*/
110+
void deletePrevious(unsigned long index = 1);
111+
100112
/**
101113
* Swap the contents of this token with the next token.
102114
*/

‎lib/tokenize.cpp‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3372,7 +3372,7 @@ bool Tokenizer::simplifySizeof()
33723372

33733373
else if (Token::Match(tok->previous(), "%type% %name% [ %num% ] [,)]") ||
33743374
Token::Match(tok->tokAt(-2), "%type% * %name% [ %num% ] [,)]")) {
3375-
Token tempTok(nullptr);
3375+
Token tempTok;
33763376
tempTok.str("*");
33773377
sizeOfVar[varId] = sizeOfType(&tempTok);
33783378
declTokOfVar[varId] = tok;
@@ -6445,7 +6445,16 @@ bool Tokenizer::simplifyKnownVariables()
64456445
while (startTok->next()->str() != ";")
64466446
startTok->deleteNext();
64476447
startTok->deleteNext();
6448-
startTok->deleteThis();
6448+
6449+
// #8579 if we can we want another token to delete startTok. if we can't it doesn't matter
6450+
if (startTok->previous()) {
6451+
startTok->previous()->deleteNext();
6452+
} else if (startTok->next()) {
6453+
startTok->next()->deletePrevious();
6454+
} else {
6455+
startTok->deleteThis();
6456+
}
6457+
startTok = nullptr;
64496458

64506459
constantVar->second = nullptr;
64516460
ret = true;

‎lib/tokenlist.cpp‎

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ static const unsigned int AST_MAX_DEPTH = 50U;
3838

3939

4040
TokenList::TokenList(const Settings* settings) :
41-
_front(nullptr),
42-
_back(nullptr),
41+
_tokensFrontBack(),
4342
_settings(settings),
4443
_isC(false),
4544
_isCPP(false)
@@ -66,9 +65,9 @@ const std::string& TokenList::getSourceFilePath() const
6665
// Deallocate lists..
6766
void TokenList::deallocateTokens()
6867
{
69-
deleteTokens(_front);
70-
_front = nullptr;
71-
_back = nullptr;
68+
deleteTokens(_tokensFrontBack.front);
69+
_tokensFrontBack.front = nullptr;
70+
_tokensFrontBack.back = nullptr;
7271
_files.clear();
7372
}
7473

@@ -142,38 +141,38 @@ void TokenList::addtoken(std::string str, const unsigned int lineno, const unsig
142141
str = MathLib::value(str).str() + suffix;
143142
}
144143

145-
if (_back) {
146-
_back->insertToken(str);
144+
if (_tokensFrontBack.back) {
145+
_tokensFrontBack.back->insertToken(str);
147146
} else {
148-
_front = new Token(&_back);
149-
_back = _front;
150-
_back->str(str);
147+
_tokensFrontBack.front = new Token(&_tokensFrontBack);
148+
_tokensFrontBack.back = _tokensFrontBack.front;
149+
_tokensFrontBack.back->str(str);
151150
}
152151

153152
if (isCPP() && str == "delete")
154-
_back->isKeyword(true);
155-
_back->linenr(lineno);
156-
_back->fileIndex(fileno);
153+
_tokensFrontBack.back->isKeyword(true);
154+
_tokensFrontBack.back->linenr(lineno);
155+
_tokensFrontBack.back->fileIndex(fileno);
157156
}
158157

159158
void TokenList::addtoken(const Token * tok, const unsigned int lineno, const unsigned int fileno)
160159
{
161160
if (tok == nullptr)
162161
return;
163162

164-
if (_back) {
165-
_back->insertToken(tok->str(), tok->originalName());
163+
if (_tokensFrontBack.back) {
164+
_tokensFrontBack.back->insertToken(tok->str(), tok->originalName());
166165
} else {
167-
_front = new Token(&_back);
168-
_back = _front;
169-
_back->str(tok->str());
166+
_tokensFrontBack.front = new Token(&_tokensFrontBack);
167+
_tokensFrontBack.back = _tokensFrontBack.front;
168+
_tokensFrontBack.back->str(tok->str());
170169
if (!tok->originalName().empty())
171-
_back->originalName(tok->originalName());
170+
_tokensFrontBack.back->originalName(tok->originalName());
172171
}
173172

174-
_back->linenr(lineno);
175-
_back->fileIndex(fileno);
176-
_back->flags(tok->flags());
173+
_tokensFrontBack.back->linenr(lineno);
174+
_tokensFrontBack.back->fileIndex(fileno);
175+
_tokensFrontBack.back->flags(tok->flags());
177176
}
178177

179178

@@ -305,28 +304,28 @@ void TokenList::createTokens(const simplecpp::TokenList *tokenList)
305304
if (str.size() > 1 && str[0] == '.' && std::isdigit(str[1]))
306305
str = '0' + str;
307306

308-
if (_back) {
309-
_back->insertToken(str);
307+
if (_tokensFrontBack.back) {
308+
_tokensFrontBack.back->insertToken(str);
310309
} else {
311-
_front = new Token(&_back);
312-
_back = _front;
313-
_back->str(str);
310+
_tokensFrontBack.front = new Token(&_tokensFrontBack);
311+
_tokensFrontBack.back = _tokensFrontBack.front;
312+
_tokensFrontBack.back->str(str);
314313
}
315314

316-
if (isCPP() && _back->str() == "delete")
317-
_back->isKeyword(true);
318-
_back->fileIndex(tok->location.fileIndex);
319-
_back->linenr(tok->location.line);
320-
_back->col(tok->location.col);
321-
_back->isExpandedMacro(!tok->macro.empty());
315+
if (isCPP() && _tokensFrontBack.back->str() == "delete")
316+
_tokensFrontBack.back->isKeyword(true);
317+
_tokensFrontBack.back->fileIndex(tok->location.fileIndex);
318+
_tokensFrontBack.back->linenr(tok->location.line);
319+
_tokensFrontBack.back->col(tok->location.col);
320+
_tokensFrontBack.back->isExpandedMacro(!tok->macro.empty());
322321
}
323322

324323
if (_settings && _settings->relativePaths) {
325324
for (std::size_t i = 0; i < _files.size(); i++)
326325
_files[i] = Path::getRelativePath(_files[i], _settings->basePaths);
327326
}
328327

329-
Token::assignProgressValues(_front);
328+
Token::assignProgressValues(_tokensFrontBack.front);
330329
}
331330

332331
//---------------------------------------------------------------------------
@@ -1164,7 +1163,7 @@ static Token * createAstAtToken(Token *tok, bool cpp)
11641163

11651164
void TokenList::createAst()
11661165
{
1167-
for (Token *tok = _front; tok; tok = tok ? tok->next() : nullptr) {
1166+
for (Token *tok = _tokensFrontBack.front; tok; tok = tok ? tok->next() : nullptr) {
11681167
tok = createAstAtToken(tok, isCPP());
11691168
}
11701169
}
@@ -1173,7 +1172,7 @@ void TokenList::validateAst() const
11731172
{
11741173
// Check for some known issues in AST to avoid crash/hang later on
11751174
std::set < const Token* > safeAstTokens; // list of "safe" AST tokens without endless recursion
1176-
for (const Token *tok = _front; tok; tok = tok->next()) {
1175+
for (const Token *tok = _tokensFrontBack.front; tok; tok = tok->next()) {
11771176
// Syntax error if binary operator only has 1 operand
11781177
if ((tok->isAssignmentOp() || tok->isComparisonOp() || Token::Match(tok,"[|^/%]")) && tok->astOperand1() && !tok->astOperand2())
11791178
throw InternalError(tok, "Syntax Error: AST broken, binary operator has only one operand.", InternalError::AST);
@@ -1217,7 +1216,7 @@ bool TokenList::validateToken(const Token* tok) const
12171216
{
12181217
if (!tok)
12191218
return true;
1220-
for (const Token *t = _front; t; t = t->next()) {
1219+
for (const Token *t = _tokensFrontBack.front; t; t = t->next()) {
12211220
if (tok==t)
12221221
return true;
12231222
}

0 commit comments

Comments
 (0)