From 5c2af0b2e31a853348dfebd177889a31bcb9f0b2 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Thu, 26 Jan 2012 16:50:59 +0100 Subject: [PATCH] - initialising std::string with 0 in initialisation list is partially detected in nullpointer check (#3520) - executionpath checking makes use of symboldatabase - CheckExceptionSafety::checkRethrowCopy makes use of symboldatabase --- lib/checkbufferoverrun.cpp | 2 +- lib/checkexceptionsafety.cpp | 29 ++++++----------- lib/checknullpointer.cpp | 62 +++++++++++++++++++++--------------- lib/checkuninitvar.cpp | 2 +- lib/executionpath.cpp | 31 ++++++++---------- lib/executionpath.h | 3 +- test/testnullpointer.cpp | 13 ++++++++ 7 files changed, 75 insertions(+), 67 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index cbe5d7e4f..0db5adfcf 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -2143,7 +2143,7 @@ void CheckBufferOverrun::executionPaths() // Perform checking - check how the arrayInfo arrays are used ExecutionPathBufferOverrun c(this, arrayInfo); - checkExecutionPaths(_tokenizer->tokens(), &c); + checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c); } diff --git a/lib/checkexceptionsafety.cpp b/lib/checkexceptionsafety.cpp index 465f36259..485e37860 100644 --- a/lib/checkexceptionsafety.cpp +++ b/lib/checkexceptionsafety.cpp @@ -136,28 +136,17 @@ void CheckExceptionSafety::checkRethrowCopy() if (!_settings->isEnabled("style")) return; - const char catchPattern1[] = "catch ("; - const char catchPattern2[] = "%var% ) { %any%"; + const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase(); - const Token* tok = Token::findsimplematch(_tokenizer->tokens(), catchPattern1); - while (tok) { - const Token* endScopeTok = tok->next(); - const Token* endBracketTok = tok->next()->link(); + for (std::list::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { + if (i->type != Scope::eCatch) + continue; - if (endBracketTok && Token::Match(endBracketTok->previous(), catchPattern2)) { - const Token* startScopeTok = endBracketTok->next(); - endScopeTok = startScopeTok->link(); - const unsigned int varid = endBracketTok->previous()->varId(); - - if (varid > 0) { - const Token* rethrowTok = Token::findmatch(startScopeTok->next(), "throw %varid%", endScopeTok->previous(), varid); - if (rethrowTok) { - rethrowCopyError(rethrowTok, endBracketTok->strAt(-1)); - } - } + const unsigned int varid = i->classStart->tokAt(-2)->varId(); + if (varid) { + const Token* rethrowTok = Token::findmatch(i->classStart->next(), "throw %varid% ;", i->classEnd->previous(), varid); + if (rethrowTok) + rethrowCopyError(rethrowTok, rethrowTok->strAt(1)); } - - tok = Token::findsimplematch(endScopeTok->next(), catchPattern1); } } - diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 5f36ce66c..fe8c8a9d9 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -248,8 +248,11 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Sym // std::string dereferences nullpointers if (Token::Match(tok->tokAt(-4), "std :: string ( %var% )")) return true; - if (Token::Match(tok->tokAt(-5), "std :: string %var% ( %var% )")) - return true; + if (Token::Match(tok->tokAt(-2), "%var% ( %var% )")) { + const Variable* var = symbolDatabase->getVariableFromVarId(tok->tokAt(-2)->varId()); + if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::")) + return true; + } unsigned int ovarid = 0; if (Token::Match(tok, "%var% ==|!= %var%")) @@ -687,7 +690,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec() // - logical operators // - while const Token* const tok = i->classDef; - if (i->type == Scope::eIf && tok && !tok->isExpandedMacro() && Token::Match(tok->previous(), "; if ( !| %var% )|%oror%|&&")) { + if (i->type == Scope::eIf && tok && !tok->isExpandedMacro() && Token::Match(tok, "if ( !| %var% )|%oror%|&&")) { const Token * vartok = tok->tokAt(2); if (vartok->str() == "!") vartok = vartok->next(); @@ -824,7 +827,7 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() if (!i->classDef || i->classDef->isExpandedMacro()) continue; - const Token* const tok = i->type != Scope::eElseIf ? i->classDef : i->classDef->next(); + const Token* const tok = i->type != Scope::eElseIf ? i->classDef->next() : i->classDef->tokAt(2); // TODO: investigate false negatives: // - handle "while"? // - if there are logical operators @@ -836,12 +839,12 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() // vartok : token for the variable const Token *vartok = 0; - if (Token::Match(tok, "if ( ! %var% )|&&")) - vartok = tok->tokAt(3); - else if (Token::Match(tok, "if|while ( %var% )|&&")) + if (Token::Match(tok, "( ! %var% )|&&")) vartok = tok->tokAt(2); - else if (Token::Match(tok, "if ( ! ( %var% =")) - vartok = tok->tokAt(4); + else if (Token::Match(tok, "( %var% )|&&")) + vartok = tok->next(); + else if (Token::Match(tok, "( ! ( %var% =")) + vartok = tok->tokAt(3); else continue; @@ -850,8 +853,6 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() if (varid == 0) continue; - const unsigned int linenr = vartok->linenr(); - const Variable* var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(varid); // Check if variable is a pointer if (!var || !var->isPointer()) @@ -866,7 +867,7 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() // start token = inside the if-body const Token *tok1 = i->classStart; - if (Token::Match(tok, "if|while ( %var% )|&&")) { + if (Token::Match(tok, "( %var% )|&&")) { // pointer might be null null = false; @@ -876,10 +877,11 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() continue; } - unsigned int indentlevel = 0; - - // Name of the pointer + // Name and line of the pointer const std::string &pointerName = vartok->str(); + const unsigned int linenr = vartok->linenr(); + + unsigned int indentlevel = 0; // Set to true if we would normally bail out the check. bool inconclusive = false; @@ -929,9 +931,9 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() // parameters to sizeof are not dereferenced if (Token::Match(tok2, "decltype|sizeof")) { if (tok2->strAt(1) != "(") - break; - - tok2 = tok2->next()->link(); + tok2 = tok2->next(); + else + tok2 = tok2->next()->link(); continue; } @@ -948,16 +950,16 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() } // calling unknown function (abort/init).. - if (Token::simpleMatch(tok2, ") ;") && - (Token::Match(tok2->link()->tokAt(-2), "[;{}.] %var% (") || - Token::Match(tok2->link()->tokAt(-5), "[;{}] ( * %var% ) ("))) { + else if (Token::simpleMatch(tok2, ") ;") && + (Token::Match(tok2->link()->tokAt(-2), "[;{}.] %var% (") || + Token::Match(tok2->link()->tokAt(-5), "[;{}] ( * %var% ) ("))) { // noreturn function? bool unknown = false; if (_tokenizer->IsScopeNoReturn(tok2->tokAt(2), &unknown)) { if (!unknown || !_settings->inconclusive) { break; } - inconclusive = true; + inconclusive = _settings->inconclusive; } // init function (global variables) @@ -1006,7 +1008,12 @@ void CheckNullPointer::nullConstantDereference() if (i->type != Scope::eFunction || !i->classStart) continue; - for (const Token *tok = i->classStart; tok != i->classEnd; tok = tok->next()) { + const Token *tok = i->classStart; + + if (i->function && (i->function->type == Function::eConstructor || i->function->type == Function::eCopyConstructor)) + tok = i->function->token; // Check initialization list + + for (; tok != i->classEnd; tok = tok->next()) { if (Token::Match(tok, "sizeof|decltype|typeid (")) tok = tok->next()->link(); @@ -1031,8 +1038,11 @@ void CheckNullPointer::nullConstantDereference() } } else if (Token::Match(tok, "std :: string ( 0 )")) nullPointerError(tok); - if (Token::Match(tok, "std :: string %var% ( 0 )")) - nullPointerError(tok); + else if (Token::Match(tok, "%var% ( 0 )")) { + const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId()); + if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::")) + nullPointerError(tok); + } unsigned int ovarid = 0; if (Token::Match(tok, "0 ==|!= %var%")) @@ -1239,7 +1249,7 @@ void CheckNullPointer::executionPaths() { // Check for null pointer errors.. Nullpointer c(this, _tokenizer->getSymbolDatabase()); - checkExecutionPaths(_tokenizer->tokens(), &c); + checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c); } void CheckNullPointer::nullPointerError(const Token *tok) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index d92620e5e..b64ac975b 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1032,7 +1032,7 @@ void CheckUninitVar::executionPaths() UninitVar::analyseFunctions(_tokenizer->tokens(), UninitVar::uvarFunctions); UninitVar c(this); - checkExecutionPaths(_tokenizer->tokens(), &c); + checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c); } } diff --git a/lib/executionpath.cpp b/lib/executionpath.cpp index 7411d3d3d..eab83ffee 100644 --- a/lib/executionpath.cpp +++ b/lib/executionpath.cpp @@ -19,6 +19,7 @@ #include "executionpath.h" #include "token.h" +#include "symboldatabase.h" #include #include #include @@ -448,29 +449,23 @@ void ExecutionPath::checkScope(const Token *tok, std::list &che } } -void checkExecutionPaths(const Token *tok, ExecutionPath *c) +void checkExecutionPaths(const SymbolDatabase *symbolDatabase, ExecutionPath *c) { - for (; tok; tok = tok->next()) { - if (tok->str() != ")") + for (std::list::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { + if (i->type != Scope::eFunction || !i->classStart) continue; - // Start of implementation.. - if (Token::Match(tok, ") const| {")) { - // goto the "{" - tok = tok->next(); - if (tok->str() == "const") - tok = tok->next(); + // Check function + std::list checks; + checks.push_back(c->copy()); + ExecutionPath::checkScope(i->classStart, checks); - std::list checks; - checks.push_back(c->copy()); - ExecutionPath::checkScope(tok, checks); + c->end(checks, i->classEnd); - c->end(checks, tok->link()); - - while (!checks.empty()) { - delete checks.back(); - checks.pop_back(); - } + // Cleanup + while (checks.size() > 1) { + delete checks.back(); + checks.pop_back(); } } } diff --git a/lib/executionpath.h b/lib/executionpath.h index b36c4a1ae..68fe4de63 100644 --- a/lib/executionpath.h +++ b/lib/executionpath.h @@ -23,6 +23,7 @@ class Token; class Check; +class SymbolDatabase; /** * Base class for Execution Paths checking @@ -126,7 +127,7 @@ public: }; -void checkExecutionPaths(const Token *tok, ExecutionPath *c); +void checkExecutionPaths(const SymbolDatabase *symbolDatabase, ExecutionPath *c); #endif diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 5bac3872c..a3f67df5d 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -1769,6 +1769,19 @@ private: "[test.cpp:4]: (error) Possible null pointer dereference: p\n" "[test.cpp:6]: (error) Possible null pointer dereference: p\n" "[test.cpp:7]: (error) Possible null pointer dereference: p\n", errout.str()); + + check("class Bar {\n" + " std::string s;\n" + " Bar() : s(0) {}\n" + "};\n" + "class Foo {\n" + " std::string s;\n" + " Foo();\n" + "};\n" + "Foo::Foo() : s(0) {}"); + TODO_ASSERT_EQUALS("[test.cpp:3]: (error) Null pointer dereference\n" + "[test.cpp:9]: (error) Null pointer dereference\n", + "[test.cpp:3]: (error) Null pointer dereference\n", errout.str()); } void functioncall() { // #3443 - function calls