From 94c953931d9d7cb0987b0cf2d967ee12b3ba1553 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Thu, 31 Jan 2013 20:08:48 +0100 Subject: [PATCH] Simplify checks by caching symbol database Variable pointer in Token --- lib/check64bit.cpp | 6 ++-- lib/checkassignif.cpp | 13 ++------ lib/checkautovariables.cpp | 61 ++++++++++++++++++-------------------- lib/checkautovariables.h | 8 ++--- lib/checkbufferoverrun.cpp | 4 +-- 5 files changed, 41 insertions(+), 51 deletions(-) diff --git a/lib/check64bit.cpp b/lib/check64bit.cpp index 71874524f..ec2897c8d 100644 --- a/lib/check64bit.cpp +++ b/lib/check64bit.cpp @@ -66,7 +66,7 @@ void Check64BitPortability::pointerassignment() for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { if (Token::Match(tok, "return %var% [;+]")) { - const Variable *var = symbolDatabase->getVariableFromVarId(tok->next()->varId()); + const Variable *var = tok->next()->variable(); if (retPointer && isint(var)) returnIntegerError(tok); else if (!retPointer && isaddr(var)) @@ -81,8 +81,8 @@ void Check64BitPortability::pointerassignment() for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) { if (Token::Match(tok, "[;{}] %var% = %var% [;+]")) { - const Variable *var1(symbolDatabase->getVariableFromVarId(tok->next()->varId())); - const Variable *var2(symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId())); + const Variable *var1(tok->next()->variable()); + const Variable *var2(tok->tokAt(3)->variable()); if (isaddr(var1) && isint(var2) && tok->strAt(4) != "+") assignmentIntegerToAddressError(tok->next()); diff --git a/lib/checkassignif.cpp b/lib/checkassignif.cpp index d18896dca..dea375b6e 100644 --- a/lib/checkassignif.cpp +++ b/lib/checkassignif.cpp @@ -36,15 +36,13 @@ void CheckAssignIf::assignIf() if (!_settings->isEnabled("style")) return; - const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (tok->str() != "=") continue; if (Token::Match(tok->tokAt(-2), "[;{}] %var% =")) { - const unsigned int varid(tok->previous()->varId()); - if (varid == 0) + const Variable *var = tok->previous()->variable(); + if (var == 0) continue; char bitop = '\0'; @@ -67,12 +65,7 @@ void CheckAssignIf::assignIf() if (num < 0 && bitop == '|') continue; - bool islocal = false; - const Variable *var = symbolDatabase->getVariableFromVarId(varid); - if (var && var->isLocal()) - islocal = true; - - assignIfParseScope(tok, tok->tokAt(4), varid, islocal, bitop, num); + assignIfParseScope(tok, tok->tokAt(4), var->varId(), var->isLocal(), bitop, num); } } } diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 3856e6b25..eee465c51 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -35,23 +35,23 @@ namespace { } -bool CheckAutoVariables::isRefPtrArg(unsigned int varId) +bool CheckAutoVariables::isRefPtrArg(const Token *tok) { - const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId); + const Variable *var = tok->variable(); return(var && var->isArgument() && var->isReference() && var->isPointer()); } -bool CheckAutoVariables::isPtrArg(unsigned int varId) +bool CheckAutoVariables::isPtrArg(const Token *tok) { - const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId); + const Variable *var = tok->variable(); return(var && var->isArgument() && var->isPointer()); } -bool CheckAutoVariables::isAutoVar(unsigned int varId) +bool CheckAutoVariables::isAutoVar(const Token *tok) { - const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId); + const Variable *var = tok->variable(); if (!var || !var->isLocal() || var->isStatic()) return false; @@ -66,9 +66,9 @@ bool CheckAutoVariables::isAutoVar(unsigned int varId) return true; } -bool CheckAutoVariables::isAutoVarArray(unsigned int varId) +bool CheckAutoVariables::isAutoVarArray(const Token *tok) { - const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varId); + const Variable *var = tok->variable(); return (var && var->isLocal() && !var->isStatic() && var->isArray()); } @@ -88,21 +88,21 @@ void CheckAutoVariables::autoVariables() const Scope * scope = symbolDatabase->functionScopes[i]; for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) { // Critical assignment - if (Token::Match(tok, "[;{}] %var% = & %var%") && isRefPtrArg(tok->next()->varId()) && isAutoVar(tok->tokAt(4)->varId())) { - const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(4)->varId()); + if (Token::Match(tok, "[;{}] %var% = & %var%") && isRefPtrArg(tok->next()) && isAutoVar(tok->tokAt(4))) { + const Variable * var = tok->tokAt(4)->variable(); if (checkRvalueExpression(var, tok->tokAt(5))) errorAutoVariableAssignment(tok->next(), false); - } else if (Token::Match(tok, "[;{}] * %var% = & %var%") && isPtrArg(tok->tokAt(2)->varId()) && isAutoVar(tok->tokAt(5)->varId())) { - const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(5)->varId()); + } else if (Token::Match(tok, "[;{}] * %var% = & %var%") && isPtrArg(tok->tokAt(2)) && isAutoVar(tok->tokAt(5))) { + const Variable * var = tok->tokAt(5)->variable(); if (checkRvalueExpression(var, tok->tokAt(6))) errorAutoVariableAssignment(tok->next(), false); } else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) { // TODO: check if the parameter is only changed temporarily (#2969) if (_settings->inconclusive) { - const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->next()->varId()); + const Variable * var1 = tok->next()->variable(); if (var1 && var1->isArgument() && var1->isPointer()) { - const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(6)->varId()); - if (isAutoVar(tok->tokAt(6)->varId()) && checkRvalueExpression(var2, tok->tokAt(7))) + const Variable * var2 = tok->tokAt(6)->variable(); + if (isAutoVar(tok->tokAt(6)) && checkRvalueExpression(var2, tok->tokAt(7))) errorAutoVariableAssignment(tok->next(), true); } } @@ -110,42 +110,42 @@ void CheckAutoVariables::autoVariables() } else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) { // TODO: check if the parameter is only changed temporarily (#2969) if (_settings->inconclusive) { - const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->next()->varId()); + const Variable * var1 = tok->next()->variable(); if (var1 && var1->isArgument() && var1->isPointer()) { - if (isAutoVarArray(tok->tokAt(5)->varId())) + if (isAutoVarArray(tok->tokAt(5))) errorAutoVariableAssignment(tok->next(), true); } } tok = tok->tokAt(5); } else if (Token::Match(tok, "[;{}] * %var% = %var% ;")) { - const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId()); + const Variable * var1 = tok->tokAt(2)->variable(); if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-3), "%type% * *")) { - if (isAutoVarArray(tok->tokAt(4)->varId())) + if (isAutoVarArray(tok->tokAt(4))) errorAutoVariableAssignment(tok->next(), false); } tok = tok->tokAt(4); - } else if (Token::Match(tok, "[;{}] %var% [") && Token::Match(tok->linkAt(2), "] = & %var%") && isPtrArg(tok->next()->varId()) && isAutoVar(tok->linkAt(2)->tokAt(3)->varId())) { + } else if (Token::Match(tok, "[;{}] %var% [") && Token::Match(tok->linkAt(2), "] = & %var%") && isPtrArg(tok->next()) && isAutoVar(tok->linkAt(2)->tokAt(3))) { const Token* const varTok = tok->linkAt(2)->tokAt(3); - const Variable * var = symbolDatabase->getVariableFromVarId(varTok->varId()); + const Variable * var = varTok->variable(); if (checkRvalueExpression(var, varTok->next())) errorAutoVariableAssignment(tok->next(), false); } // Critical return - else if (Token::Match(tok, "return & %var% ;") && isAutoVar(tok->tokAt(2)->varId())) { + else if (Token::Match(tok, "return & %var% ;") && isAutoVar(tok->tokAt(2))) { errorReturnAddressToAutoVariable(tok); } else if (Token::Match(tok, "return & %var% [") && Token::simpleMatch(tok->linkAt(3), "] ;") && - isAutoVarArray(tok->tokAt(2)->varId())) { + isAutoVarArray(tok->tokAt(2))) { errorReturnAddressToAutoVariable(tok); } else if (Token::Match(tok, "return & %var% ;") && tok->tokAt(2)->varId()) { - const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId()); + const Variable * var1 = tok->tokAt(2)->variable(); if (var1 && var1->isArgument() && var1->typeEndToken()->str() != "&") errorReturnAddressOfFunctionParameter(tok, tok->strAt(2)); } // Invalid pointer deallocation else if (Token::Match(tok, "free ( %var% ) ;") || Token::Match(tok, "delete [| ]| (| %var% !![")) { tok = Token::findmatch(tok->next(), "%var%"); - if (isAutoVarArray(tok->varId())) + if (isAutoVarArray(tok)) errorInvalidDeallocation(tok); } } @@ -171,8 +171,7 @@ void CheckAutoVariables::returnPointerToLocalArray() for (const Token *tok2 = scope->classStart->next(); tok2 && tok2 != scope->classEnd; tok2 = tok2->next()) { // Return pointer to local array variable.. if (Token::Match(tok2, "return %var% ;")) { - const unsigned int varid = tok2->next()->varId(); - if (isAutoVarArray(varid)) { + if (isAutoVarArray(tok2->next())) { errorReturnPointerToLocalArray(tok2); } } @@ -285,10 +284,8 @@ void CheckAutoVariables::returnReference() // return.. if (Token::Match(tok2, "return %var% ;")) { // is the returned variable a local variable? - const unsigned int varid1 = tok2->next()->varId(); - const Variable *var1 = symbolDatabase->getVariableFromVarId(varid1); - - if (isAutoVar(varid1)) { + if (isAutoVar(tok2->next())) { + const Variable *var1 = tok2->next()->variable(); // If reference variable is used, check what it references if (Token::Match(var1->nameToken(), "%var% [=(]")) { const Token *tok3 = var1->nameToken()->tokAt(2); @@ -297,7 +294,7 @@ void CheckAutoVariables::returnReference() // Only report error if variable that is referenced is // a auto variable - if (!isAutoVar(tok3->varId())) + if (!isAutoVar(tok3)) continue; } diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index 6aa8ffddb..be50a88ae 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -63,10 +63,10 @@ public: void returnReference(); private: - bool isRefPtrArg(unsigned int varId); - bool isPtrArg(unsigned int varId); - bool isAutoVar(unsigned int varId); - bool isAutoVarArray(unsigned int varId); + bool isRefPtrArg(const Token *tok); + bool isPtrArg(const Token *tok); + bool isAutoVar(const Token *tok); + bool isAutoVarArray(const Token *tok); /** * Returning a temporary object? diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index dd72f24ef..d721e7ac6 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1046,7 +1046,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector 0 && Token::Match(tok, "strcpy|strcat ( %varid% , %var% )", varid)) || (varid == 0 && Token::Match(tok, ("strcpy|strcat ( " + varnames + " , %var% )").c_str()))) { - const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->tokAt(4)->varId()); + const Variable *var = tok->tokAt(4)->variable(); if (var && var->isArray() && var->dimensions().size() == 1) { const std::size_t len = (std::size_t)var->dimension(0); if (total_size > 0 && len > (unsigned int)total_size) { @@ -1915,7 +1915,7 @@ void CheckBufferOverrun::negativeIndex() tok2 = tok2->previous()->link(); if (tok2->previous() && tok2->previous()->varId()) { - const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok2->previous()->varId()); + const Variable *var = tok2->previous()->variable(); if (var && var->isArray()) negativeIndexError(tok, index); }