From a788512d6641fa9c06d837e27ea28d506b9d22d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 26 Dec 2018 19:17:49 +0100 Subject: [PATCH] CTU: Refactor isUnsafeFunction --- lib/checknullpointer.cpp | 49 ++++++--------------------------------- lib/checknullpointer.h | 6 +---- lib/checkuninitvar.cpp | 50 ++++++++-------------------------------- lib/ctu.cpp | 45 ++++++++++++++++++++++++++++++++++++ lib/ctu.h | 2 ++ 5 files changed, 65 insertions(+), 87 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 548de7a81..1f687acf2 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -596,55 +596,20 @@ std::string CheckNullPointer::MyFileInfo::toString() const return CTU::toString(unsafeUsage); } -bool CheckNullPointer::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const +static bool isUnsafeUsage(const Check *check, const Token *vartok) { - const Variable * const argvar = scope->function->getArgumentVar(argnr); - if (!argvar->isPointer()) - return false; - for (const Token *tok2 = scope->bodyStart; tok2 != scope->bodyEnd; tok2 = tok2->next()) { - if (Token::simpleMatch(tok2, ") {")) { - tok2 = tok2->linkAt(1); - if (Token::findmatch(tok2->link(), "return|throw", tok2)) - return false; - if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, mSettings, mTokenizer->isCPP())) - return false; - } - if (tok2->variable() != argvar) - continue; - if (!Token::Match(tok2->astParent(), "*|[")) - return false; - *tok = tok2; - return true; - } - return false; + (void)check; + return Token::Match(vartok->astParent(), "*|["); } Check::FileInfo *CheckNullPointer::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { - const CheckNullPointer checker(tokenizer, settings, nullptr); - return checker.getFileInfo(); -} - -Check::FileInfo *CheckNullPointer::getFileInfo() const -{ - const SymbolDatabase * const symbolDatabase = mTokenizer->getSymbolDatabase(); + const std::list &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, nullptr, ::isUnsafeUsage); + if (unsafeUsage.empty()) + return nullptr; MyFileInfo *fileInfo = new MyFileInfo; - - // Parse all functions in TU - for (const Scope &scope : symbolDatabase->scopeList) { - if (!scope.isExecutable() || scope.type != Scope::eFunction || !scope.function) - continue; - const Function *const function = scope.function; - - // "Unsafe" functions unconditionally reads data before it is written.. - for (int argnr = 0; argnr < function->argCount(); ++argnr) { - const Token *tok; - if (isUnsafeFunction(&scope, argnr, &tok)) - fileInfo->unsafeUsage.push_back(CTU::FileInfo::UnsafeUsage(CTU::getFunctionId(mTokenizer, function), argnr+1, tok->str(), CTU::FileInfo::Location(mTokenizer,tok))); - } - } - + fileInfo->unsafeUsage = unsafeUsage; return fileInfo; } diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index ea6b1b328..d120c2877 100644 --- a/lib/checknullpointer.h +++ b/lib/checknullpointer.h @@ -101,8 +101,6 @@ public: } void nullPointerError(const Token *tok, const std::string &varname, const ValueFlow::Value* value, bool inconclusive); - bool isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const; - /* data for multifile checking */ class MyFileInfo : public Check::FileInfo { public: @@ -114,7 +112,7 @@ public: }; /** @brief Parse current TU and extract file info */ - Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const; + Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const override; Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const; @@ -122,8 +120,6 @@ public: bool analyseWholeProgram(const CTU::FileInfo *ctu, const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger); private: - Check::FileInfo *getFileInfo() const; - /** Get error messages. Used by --errorlist */ void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override { CheckNullPointer c(nullptr, settings, errorLogger); diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 3ab5d3fd8..c022a9312 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1284,56 +1284,26 @@ std::string CheckUninitVar::MyFileInfo::toString() const return CTU::toString(unsafeUsage); } -bool CheckUninitVar::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const -{ - const Variable * const argvar = scope->function->getArgumentVar(argnr); - if (!argvar->isPointer()) - return false; - for (const Token *tok2 = scope->bodyStart; tok2 != scope->bodyEnd; tok2 = tok2->next()) { - if (Token::simpleMatch(tok2, ") {")) { - tok2 = tok2->linkAt(1); - if (Token::findmatch(tok2->link(), "return|throw", tok2)) - return false; - if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, mSettings, mTokenizer->isCPP())) - return false; - } - if (tok2->variable() != argvar) - continue; - if (!isVariableUsage(tok2, true, Alloc::ARRAY)) - return false; - *tok = tok2; - return true; - } - return false; -} - - Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { const CheckUninitVar checker(tokenizer, settings, nullptr); return checker.getFileInfo(); } +static bool isVariableUsage(const Check *check, const Token *vartok) +{ + const CheckUninitVar *c = dynamic_cast(check); + return c && c->isVariableUsage(vartok, true, CheckUninitVar::Alloc::ARRAY); +} + Check::FileInfo *CheckUninitVar::getFileInfo() const { - const SymbolDatabase * const symbolDatabase = mTokenizer->getSymbolDatabase(); + const std::list &unsafeUsage = CTU::getUnsafeUsage(mTokenizer, mSettings, this, ::isVariableUsage); + if (unsafeUsage.empty()) + return nullptr; MyFileInfo *fileInfo = new MyFileInfo; - - // Parse all functions in TU - for (const Scope &scope : symbolDatabase->scopeList) { - if (!scope.isExecutable() || scope.type != Scope::eFunction || !scope.function) - continue; - const Function *const function = scope.function; - - // "Unsafe" functions unconditionally reads data before it is written.. - for (int argnr = 0; argnr < function->argCount(); ++argnr) { - const Token *tok; - if (isUnsafeFunction(&scope, argnr, &tok)) - fileInfo->unsafeUsage.push_back(CTU::FileInfo::UnsafeUsage(CTU::getFunctionId(mTokenizer, function), argnr+1, tok->str(), CTU::FileInfo::Location(mTokenizer,tok))); - } - } - + fileInfo->unsafeUsage = unsafeUsage ; return fileInfo; } diff --git a/lib/ctu.cpp b/lib/ctu.cpp index baf338607..74488b507 100644 --- a/lib/ctu.cpp +++ b/lib/ctu.cpp @@ -295,6 +295,51 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) return fileInfo; } +static bool isUnsafeFunction(const Tokenizer *tokenizer, const Settings *settings, const Scope *scope, int argnr, const Token **tok, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok)) +{ + const Variable * const argvar = scope->function->getArgumentVar(argnr); + if (!argvar->isPointer()) + return false; + for (const Token *tok2 = scope->bodyStart; tok2 != scope->bodyEnd; tok2 = tok2->next()) { + if (Token::simpleMatch(tok2, ") {")) { + tok2 = tok2->linkAt(1); + if (Token::findmatch(tok2->link(), "return|throw", tok2)) + return false; + if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, settings, tokenizer->isCPP())) + return false; + } + if (tok2->variable() != argvar) + continue; + if (!isUnsafeUsage(check, tok2)) + return false; + *tok = tok2; + return true; + } + return false; +} + +std::list CTU::getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok)) +{ + std::list unsafeUsage; + + // Parse all functions in TU + const SymbolDatabase * const symbolDatabase = tokenizer->getSymbolDatabase(); + + for (const Scope &scope : symbolDatabase->scopeList) { + if (!scope.isExecutable() || scope.type != Scope::eFunction || !scope.function) + continue; + const Function *const function = scope.function; + + // "Unsafe" functions unconditionally reads data before it is written.. + for (int argnr = 0; argnr < function->argCount(); ++argnr) { + const Token *tok; + if (isUnsafeFunction(tokenizer, settings, &scope, argnr, &tok, check, isUnsafeUsage)) + unsafeUsage.push_back(CTU::FileInfo::UnsafeUsage(CTU::getFunctionId(tokenizer, function), argnr+1, tok->str(), CTU::FileInfo::Location(tokenizer,tok))); + } + } + + return unsafeUsage; +} static bool findPath(const CTU::FileInfo::FunctionCall &from, const CTU::FileInfo::UnsafeUsage &to, diff --git a/lib/ctu.h b/lib/ctu.h index 4b0e6ea1f..0c458b12d 100644 --- a/lib/ctu.h +++ b/lib/ctu.h @@ -107,6 +107,8 @@ namespace CTU { /** @brief Parse current TU and extract file info */ FileInfo *getFileInfo(const Tokenizer *tokenizer); + std::list getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok)); + std::list loadUnsafeUsageListFromXml(const tinyxml2::XMLElement *xmlElement); }