From 100887429dd5b8475ced33c3ab644539573ebc3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 15 Jan 2018 15:54:09 +0100 Subject: [PATCH] Uninitialized variables: Whole program analysis for function calls --- lib/checkuninitvar.cpp | 165 +++++++++++++++++++++++++++++++++++++++++ lib/checkuninitvar.h | 32 +++++++- test/testuninitvar.cpp | 35 +++++++++ 3 files changed, 228 insertions(+), 4 deletions(-) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 929d949d6..2ca53049d 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -31,6 +31,8 @@ #include "tokenize.h" #include "valueflow.h" +#include + #include #include #include @@ -1280,3 +1282,166 @@ void CheckUninitVar::deadPointerError(const Token *pointer, const Token *alias) "deadpointer", "Dead pointer usage. Pointer '" + strpointer + "' is dead if it has been assigned '" + stralias + "' at line " + MathLib::toString(alias ? alias->linenr() : 0U) + ".", CWE825, false); } + +std::string CheckUninitVar::MyFileInfo::toString() const +{ + std::ostringstream ret; + for (std::list::const_iterator it = unsafeFunctionArgs.begin(); it != unsafeFunctionArgs.end(); ++it) { + ret << " functionName << '\"' + << " argnr=\"" << it->argnr << '\"' + << " fileName=\"" << it->location.fileName << '\"' + << " linenr=\"" << it->location.linenr << '\"' + << "/>\n"; + } + for (std::list::const_iterator it = uninitializedFunctionArgs.begin(); it != uninitializedFunctionArgs.end(); ++it) { + ret << " functionName << '\"' + << " argnr=\"" << it->argnr << '\"' + << " fileName=\"" << it->location.fileName << '\"' + << " linenr=\"" << it->location.linenr << '\"' + << "/>\n"; + } + return ret.str(); +} + +Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings * /*settings*/) const +{ + const SymbolDatabase * const symbolDatabase = tokenizer->getSymbolDatabase(); + std::list::const_iterator scope; + + MyFileInfo *fileInfo = new MyFileInfo; + + // Parse all functions in TU + for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) { + if (!scope->isExecutable() || scope->type != Scope::eFunction) + continue; + const Function *const function = scope->function; + + // Unsafe arguments.. + for (int argnr = 0; argnr < function->argCount(); ++argnr) { + const Variable * const argvar = function->getArgumentVar(argnr); + if (!argvar->isPointer()) + continue; + for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + if (tok->variable() != argvar) + continue; + if (!Token::Match(tok->astParent(), "*|[")) + break; + while (Token::Match(tok->astParent(), "*|[")) + tok = tok->astParent(); + if (Token::Match(tok->astParent(),"%cop%")) + fileInfo->unsafeFunctionArgs.push_back(MyFileInfo::FunctionArg(scope->className, argnr, tokenizer->list.file(tok), tok->linenr(), argvar->name())); + break; + } + } + + // Unsafe calls.. + for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + if (tok->str() != "(" || !tok->astOperand1() || !tok->astOperand2()) + continue; + if (Token::Match(tok->astOperand1(), "if|while|for")) + continue; + const std::vector args(getArguments(tok->previous())); + for (int i = 0; i < args.size(); ++i) { + const Token *argtok = args[i]; + if (!argtok || argtok->str() != "&" || argtok->astOperand2()) + continue; + argtok = argtok->astOperand1(); + if (!argtok || !argtok->valueType() || argtok->valueType()->pointer != 0) + continue; + if (argtok->values().size() != 1U) + continue; + const ValueFlow::Value &v = argtok->values().front(); + if (v.valueType != ValueFlow::Value::UNINIT || v.isInconclusive()) + continue; + fileInfo->uninitializedFunctionArgs.push_back(MyFileInfo::FunctionArg(tok->astOperand1()->str(), i, tokenizer->list.file(argtok), argtok->linenr(), argtok->str())); + } + } + } + + return fileInfo; +} + +Check::FileInfo * CheckUninitVar::loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const +{ + MyFileInfo *fileInfo = new MyFileInfo; + for (const tinyxml2::XMLElement *e = xmlElement->FirstChildElement(); e; e = e->NextSiblingElement()) { + if (std::strcmp(e->Name(),"unsafefunction")!=0 && std::strcmp(e->Name(),"uninitializedFunctionArgs")!=0) + continue; + const char *functionName = e->Attribute("functionName"); + if (!functionName) + continue; + const char *argnr = e->Attribute("argnr"); + if (!argnr || !MathLib::isInt(argnr)) + continue; + const char *fileName = e->Attribute("fileName"); + if (!fileName) + continue; + const char *linenr = e->Attribute("argnr"); + if (!linenr || !MathLib::isInt(linenr)) + continue; + const char *variableName = e->Attribute("variableName"); + if (!variableName) + continue; + const MyFileInfo::FunctionArg fa(functionName, MathLib::toLongNumber(argnr), fileName, MathLib::toLongNumber(linenr), variableName); + if (std::strcmp(e->Name(), "unsafefunction") == 0) + fileInfo->unsafeFunctionArgs.push_back(fa); + else + fileInfo->uninitializedFunctionArgs.push_back(fa); + } + return fileInfo; +} + +bool CheckUninitVar::analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) +{ + (void)settings; // This argument is unused + + // Merge all fileinfo.. + MyFileInfo all; + for (std::list::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) { + const MyFileInfo *fi = dynamic_cast(*it); + if (!fi) + continue; + all.unsafeFunctionArgs.insert(all.unsafeFunctionArgs.end(), fi->unsafeFunctionArgs.begin(), fi->unsafeFunctionArgs.end()); + all.uninitializedFunctionArgs.insert(all.uninitializedFunctionArgs.end(), fi->uninitializedFunctionArgs.begin(), fi->uninitializedFunctionArgs.end()); + } + + bool foundErrors = false; + + for (std::list::const_iterator it1 = all.uninitializedFunctionArgs.begin(); it1 != all.uninitializedFunctionArgs.end(); ++it1) { + const CheckUninitVar::MyFileInfo::FunctionArg &uninitializedFunctionArg = *it1; + for (std::list::const_iterator it2 = all.unsafeFunctionArgs.begin(); it2 != all.unsafeFunctionArgs.end(); ++it2) { + const CheckUninitVar::MyFileInfo::FunctionArg &unsafeFunctionArg = *it2; + if (uninitializedFunctionArg.functionName != unsafeFunctionArg.functionName || uninitializedFunctionArg.argnr != unsafeFunctionArg.argnr) + continue; + + ErrorLogger::ErrorMessage::FileLocation fileLoc1; + fileLoc1.setfile(uninitializedFunctionArg.location.fileName); + fileLoc1.line = uninitializedFunctionArg.location.linenr; + fileLoc1.setinfo("Calling function " + uninitializedFunctionArg.functionName + ", variable " + uninitializedFunctionArg.variableName + " is uninitialized"); + + ErrorLogger::ErrorMessage::FileLocation fileLoc2; + fileLoc2.setfile(unsafeFunctionArg.location.fileName); + fileLoc2.line = unsafeFunctionArg.location.linenr; + fileLoc2.setinfo("Using argument " + unsafeFunctionArg.variableName); + + std::list locationList; + locationList.push_back(fileLoc1); + locationList.push_back(fileLoc2); + + const ErrorLogger::ErrorMessage errmsg(locationList, + emptyString, + Severity::error, + "using argument " + unsafeFunctionArg.variableName + " that points at uninitialized variable " + uninitializedFunctionArg.variableName, + "uninitvar", + CWE908, false); + errorLogger.reportErr(errmsg); + + foundErrors = true; + } + } + + return foundErrors; +} + diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index bd714dc17..77b980856 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -91,13 +91,37 @@ public: /* data for multifile checking */ class MyFileInfo : public Check::FileInfo { public: - /* functions that must have initialized data */ - std::set uvarFunctions; + struct FunctionArg { + FunctionArg(const std::string &s, unsigned int i, const std::string &fileName, unsigned int linenr, const std::string &varname) : functionName(s), argnr(i), variableName(varname) { + location.fileName = fileName; + location.linenr = linenr; + } + std::string functionName; + unsigned int argnr; + std::string variableName; + struct { + std::string fileName; + unsigned int linenr; + } location; + }; - /* functions calls with uninitialized data */ - std::set functionCalls; + /* function arguments that must be initialized */ + std::list unsafeFunctionArgs; + + /* uninitialized function args */ + std::list uninitializedFunctionArgs; + + std::string toString() const; }; + /** @brief Parse current TU and extract file info */ + Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const; + + Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const; + + /** @brief Analyse all file infos for all TU */ + bool analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger); + void uninitstringError(const Token *tok, const std::string &varname, bool strncpy_); void uninitdataError(const Token *tok, const std::string &varname); void uninitvarError(const Token *tok, const std::string &varname); diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 5546dff16..0991c8561 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -84,6 +84,9 @@ private: // dead pointer TEST_CASE(deadPointer); + + // whole program analysis + TEST_CASE(ctu); } void checkUninitVar(const char code[], const char fname[] = "test.cpp", bool debugwarnings = false) { @@ -3969,6 +3972,38 @@ private: "}"); ASSERT_EQUALS("", errout.str()); } + + void ctu(const char code[]) { + // Clear the error buffer.. + errout.str(""); + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.simplifyTokenList2(); + + // Check code.. + std::list fileInfo; + CheckUninitVar check(&tokenizer, &settings, this); + fileInfo.push_back(check.getFileInfo(&tokenizer, &settings)); + check.analyseWholeProgram(fileInfo, settings, *this); + while (!fileInfo.empty()) { + delete fileInfo.back(); + fileInfo.pop_back(); + } + } + + void ctu() { + ctu("void f(int *p) {\n" + " a = *p + 3;\n" + "}\n" + "int main() {\n" + " int x;\n" + " f(&x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) using argument p that points at uninitialized variable x\n", errout.str()); + } }; REGISTER_TEST(TestUninitVar)