From a6e227a73c9b8b37cd67685346d94cf2fb3db2f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 26 Dec 2018 15:56:10 +0100 Subject: [PATCH] CTU: Refactoring; getErrorPath --- lib/checknullpointer.cpp | 44 +++++++++++-------------- lib/checkuninitvar.cpp | 43 +++++++++++------------- lib/ctu.cpp | 70 +++++++++++++++++++++++++++------------- lib/ctu.h | 9 ++++-- 4 files changed, 92 insertions(+), 74 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 8840db026..548de7a81 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -593,10 +593,7 @@ void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value std::string CheckNullPointer::MyFileInfo::toString() const { - std::ostringstream ret; - for (const CTU::FileInfo::UnsafeUsage &u : unsafeUsage) - ret << u.toString(); - return ret.str(); + return CTU::toString(unsafeUsage); } bool CheckNullPointer::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const @@ -676,30 +673,25 @@ bool CheckNullPointer::analyseWholeProgram(const CTU::FileInfo *ctu, const std:: if (!fi) continue; for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafeUsage) { - for (const CTU::FileInfo::FunctionCall &functionCall : ctu->functionCalls) { - if (functionCall.valueType != ValueFlow::Value::ValueType::INT) - continue; - if (functionCall.argvalue != 0) - continue; + const std::list &locationList = + ctu->getErrorPath(CTU::FileInfo::InvalidValueType::null, + unsafeUsage, + nestedCallsMap, + "Dereferencing argument ARG that is null", + nullptr); + if (locationList.empty()) + continue; - const std::list &locationList = - ctu->getErrorPath(functionCall, - unsafeUsage, - nestedCallsMap, - "Dereferencing argument ARG that is null"); - if (locationList.empty()) - continue; + const ErrorLogger::ErrorMessage errmsg(locationList, + emptyString, + Severity::error, + "Null pointer dereference: " + unsafeUsage.argumentName, + "ctunullpointer", + CWE476, false); + errorLogger.reportErr(errmsg); - const ErrorLogger::ErrorMessage errmsg(locationList, - emptyString, - Severity::error, - "Null pointer dereference: " + unsafeUsage.argumentName, - "ctunullpointer", - CWE476, false); - errorLogger.reportErr(errmsg); - - foundErrors = true; - } + foundErrors = true; + break; } } diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index b555810e9..3ab5d3fd8 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1281,10 +1281,7 @@ void CheckUninitVar::deadPointerError(const Token *pointer, const Token *alias) std::string CheckUninitVar::MyFileInfo::toString() const { - std::ostringstream ret; - for (const CTU::FileInfo::UnsafeUsage &u : unsafeUsage) - ret << u.toString(); - return ret.str(); + return CTU::toString(unsafeUsage); } bool CheckUninitVar::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const @@ -1330,7 +1327,6 @@ Check::FileInfo *CheckUninitVar::getFileInfo() const const Function *const function = scope.function; // "Unsafe" functions unconditionally reads data before it is written.. - CheckNullPointer checkNullPointer(mTokenizer, mSettings, mErrorLogger); for (int argnr = 0; argnr < function->argCount(); ++argnr) { const Token *tok; if (isUnsafeFunction(&scope, argnr, &tok)) @@ -1366,28 +1362,27 @@ bool CheckUninitVar::analyseWholeProgram(const CTU::FileInfo *ctu, const std::li if (!fi) continue; for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafeUsage) { - for (const CTU::FileInfo::FunctionCall &functionCall : ctu->functionCalls) { - if (functionCall.valueType != ValueFlow::Value::ValueType::UNINIT) - continue; + const CTU::FileInfo::FunctionCall *functionCall = nullptr; - const std::list &locationList = - ctu->getErrorPath(functionCall, - unsafeUsage, - nestedCallsMap, - "Using argument ARG"); - if (locationList.empty()) - continue; + const std::list &locationList = + ctu->getErrorPath(CTU::FileInfo::InvalidValueType::uninit, + unsafeUsage, + nestedCallsMap, + "Using argument ARG", + &functionCall); + if (locationList.empty()) + continue; - const ErrorLogger::ErrorMessage errmsg(locationList, - emptyString, - Severity::error, - "Using argument " + unsafeUsage.argumentName + " that points at uninitialized variable " + functionCall.argumentExpression, - "ctuuninitvar", - CWE908, false); - errorLogger.reportErr(errmsg); + const ErrorLogger::ErrorMessage errmsg(locationList, + emptyString, + Severity::error, + "Using argument " + unsafeUsage.argumentName + " that points at uninitialized variable " + functionCall->argumentExpression, + "ctuuninitvar", + CWE908, false); + errorLogger.reportErr(errmsg); - foundErrors = true; - } + foundErrors = true; + break; } } return foundErrors; diff --git a/lib/ctu.cpp b/lib/ctu.cpp index 33906a963..baf338607 100644 --- a/lib/ctu.cpp +++ b/lib/ctu.cpp @@ -82,6 +82,14 @@ std::string CTU::FileInfo::UnsafeUsage::toString() const return out.str(); } +std::string CTU::toString(const std::list &unsafeUsage) +{ + std::ostringstream ret; + for (const CTU::FileInfo::UnsafeUsage &u : unsafeUsage) + ret << u.toString(); + return ret.str(); +} + CTU::FileInfo::NestedCall::NestedCall(const Tokenizer *tokenizer, const Scope *scope, unsigned int argnr_, const Token *tok) : id(getFunctionId(tokenizer, scope->function)), @@ -331,34 +339,52 @@ static std::string replacestr(std::string s, const std::string &from, const std: return s; } -std::list CTU::FileInfo::getErrorPath(const CTU::FileInfo::FunctionCall &functionCall, +std::list CTU::FileInfo::getErrorPath(InvalidValueType invalidValue, const CTU::FileInfo::UnsafeUsage &unsafeUsage, const std::map> &nestedCallsMap, - const char info[]) const + const char info[], + const FunctionCall * * const functionCallPtr) const { std::list locationList; - if (!findPath(functionCall, unsafeUsage, nestedCallsMap)) + for (const FunctionCall &functionCall : functionCalls) { + + if (invalidValue == CTU::FileInfo::InvalidValueType::null && + (functionCall.valueType != ValueFlow::Value::ValueType::INT || functionCall.argvalue != 0)) { + continue; + } + if (invalidValue == CTU::FileInfo::InvalidValueType::uninit && + functionCall.valueType != ValueFlow::Value::ValueType::UNINIT) { + continue; + } + + if (!findPath(functionCall, unsafeUsage, nestedCallsMap)) + continue; + + if (functionCallPtr) + *functionCallPtr = &functionCall; + + std::string value1; + if (functionCall.valueType == ValueFlow::Value::ValueType::INT) + value1 = "null"; + else if (functionCall.valueType == ValueFlow::Value::ValueType::UNINIT) + value1 = "uninitialized"; + + ErrorLogger::ErrorMessage::FileLocation fileLoc1; + fileLoc1.setfile(functionCall.location.fileName); + fileLoc1.line = functionCall.location.linenr; + fileLoc1.setinfo("Calling function " + functionCall.functionName + ", " + MathLib::toString(functionCall.argnr) + getOrdinalText(functionCall.argnr) + " argument is " + value1); + + ErrorLogger::ErrorMessage::FileLocation fileLoc2; + fileLoc2.setfile(unsafeUsage.location.fileName); + fileLoc2.line = unsafeUsage.location.linenr; + fileLoc2.setinfo(replacestr(info, "ARG", unsafeUsage.argumentName)); + + locationList.push_back(fileLoc1); + locationList.push_back(fileLoc2); + return locationList; - - std::string value1; - if (functionCall.valueType == ValueFlow::Value::ValueType::INT) - value1 = "null"; - else if (functionCall.valueType == ValueFlow::Value::ValueType::UNINIT) - value1 = "uninitialized"; - - ErrorLogger::ErrorMessage::FileLocation fileLoc1; - fileLoc1.setfile(functionCall.location.fileName); - fileLoc1.line = functionCall.location.linenr; - fileLoc1.setinfo("Calling function " + functionCall.functionName + ", " + MathLib::toString(functionCall.argnr) + getOrdinalText(functionCall.argnr) + " argument is " + value1); - - ErrorLogger::ErrorMessage::FileLocation fileLoc2; - fileLoc2.setfile(unsafeUsage.location.fileName); - fileLoc2.line = unsafeUsage.location.linenr; - fileLoc2.setinfo(replacestr(info, "ARG", unsafeUsage.argumentName)); - - locationList.push_back(fileLoc1); - locationList.push_back(fileLoc2); + } return locationList; } diff --git a/lib/ctu.h b/lib/ctu.h index 31d60dc10..4b0e6ea1f 100644 --- a/lib/ctu.h +++ b/lib/ctu.h @@ -91,12 +91,17 @@ namespace CTU { void loadFromXml(const tinyxml2::XMLElement *xmlElement); std::map> getNestedCallsMap() const; - std::list getErrorPath(const FunctionCall &functionCall, + enum InvalidValueType { null, uninit }; + + std::list getErrorPath(InvalidValueType invalidValue, const UnsafeUsage &unsafeUsage, const std::map> &nestedCallsMap, - const char info[]) const; + const char info[], + const FunctionCall * * const functionCallPtr) const; }; + std::string toString(const std::list &unsafeUsage); + std::string getFunctionId(const Tokenizer *tokenizer, const Function *function); /** @brief Parse current TU and extract file info */