walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Remove whole program analysis from 'uninitialized variables' and 'null pointer dereference' checkers. I think this logic can more or less be added in ValueFlow instead and then all ValueFlow checkers should get whole program analysis.

This commit is contained in:
Daniel Marjamäki 2018-02-06 14:56:17 +01:00
parent 71511f3131
commit 58066b1f0c
6 changed files with 0 additions and 541 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
lib/checknullpointer.cpp
View File

@ -20,7 +20,6 @@
//---------------------------------------------------------------------------
#include "checknullpointer.h"
#include "astutils.h"
#include "errorlogger.h"
#include "library.h"
#include "settings.h"
@ -548,26 +547,3 @@ void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value
CWE682, // unknown - pointer overflow
value && value->isInconclusive());
}
bool CheckNullPointer::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->classStart; tok2 != scope->classEnd; 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))
return false;
}
if (tok2->variable() != argvar)
continue;
if (!Token::Match(tok2->astParent(), "*|["))
return false;
*tok = tok2;
return true;
}
return false;
}

2
lib/checknullpointer.h
View File

@ -99,8 +99,6 @@ public:
nullPointerError(tok, "", &v, false);
}
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;
private:
/** Get error messages. Used by --errorlist */

326
lib/checkuninitvar.cpp
View File

@ -1282,329 +1282,3 @@ 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);
}
static void writeFunctionArgsXml(const std::list<CheckUninitVar::MyFileInfo::FunctionArg> &list, const std::string &elementName, std::ostream &out)
{
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it = list.begin(); it != list.end(); ++it)
out << " <" << elementName
<< " id=\"" << it->id << '\"'
<< " functionName=\"" << it->functionName << '\"'
<< " argnr=\"" << it->argnr << '\"'
<< " variableName=\"" << it->variableName << "\""
<< " fileName=\"" << it->location.fileName << '\"'
<< " linenr=\"" << it->location.linenr << '\"'
<< "/>\n";
}
std::string CheckUninitVar::MyFileInfo::toString() const
{
std::ostringstream ret;
writeFunctionArgsXml(uninitialized, "uninitialized", ret);
writeFunctionArgsXml(readData, "readData", ret);
writeFunctionArgsXml(nullPointer, "nullPointer", ret);
writeFunctionArgsXml(dereferenced, "dereferenced", ret);
writeFunctionArgsXml(nestedCall, "nestedCall", ret);
return ret.str();
}
#define FUNCTION_ID(function) _tokenizer->list.file(function->tokenDef) + ':' + MathLib::toString(function->tokenDef->linenr())
CheckUninitVar::MyFileInfo::FunctionArg::FunctionArg(const Tokenizer *_tokenizer, const Scope *scope, unsigned int argnr_, const Token *tok)
:
id(FUNCTION_ID(scope->function)),
functionName(scope->className),
argnr(argnr_),
argnr2(0),
variableName(scope->function->getArgumentVar(argnr-1)->name())
{
location.fileName = _tokenizer->list.file(tok);
location.linenr = tok->linenr();
}
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->classStart; tok2 != scope->classEnd; 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))
return false;
}
if (tok2->variable() != argvar)
continue;
if (!isVariableUsage(tok2, true, Alloc::ARRAY))
return false;
*tok = tok2;
return true;
}
return false;
}
static int isCallFunction(const Scope *scope, int argnr, const Token **tok)
{
const Variable * const argvar = scope->function->getArgumentVar(argnr);
if (!argvar->isPointer())
return -1;
for (const Token *tok2 = scope->classStart; tok2 != scope->classEnd; tok2 = tok2->next()) {
if (tok2->variable() != argvar)
continue;
if (!Token::Match(tok2->previous(), "[(,] %var% [,)]"))
break;
int argnr2 = 1;
const Token *prev = tok2;
while (prev && prev->str() != "(") {
if (Token::Match(prev,"]|)"))
prev = prev->link();
else if (prev->str() == ",")
++argnr2;
prev = prev->previous();
}
if (!prev || !Token::Match(prev->previous(), "%name% ("))
break;
if (!prev->astOperand1() || !prev->astOperand1()->function())
break;
*tok = prev->previous();
return argnr2;
}
return -1;
}
Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const
{
const CheckUninitVar checker(tokenizer, settings, nullptr);
return checker.getFileInfo();
}
Check::FileInfo *CheckUninitVar::getFileInfo() const
{
const SymbolDatabase * const symbolDatabase = _tokenizer->getSymbolDatabase();
std::list<Scope>::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 || !scope->function)
continue;
const Function *const function = scope->function;
// function calls where uninitialized data is passed by address
for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
if (tok->str() != "(" || !tok->astOperand1() || !tok->astOperand2())
continue;
if (!tok->astOperand1()->function())
continue;
const std::vector<const Token *> args(getArguments(tok->previous()));
for (int argnr = 0; argnr < args.size(); ++argnr) {
const Token *argtok = args[argnr];
if (!argtok)
continue;
if (argtok->valueType() && argtok->valueType()->pointer > 0) {
// null pointer..
const ValueFlow::Value *value = argtok->getValue(0);
if (value && !value->isInconclusive())
fileInfo->nullPointer.push_back(MyFileInfo::FunctionArg(FUNCTION_ID(tok->astOperand1()->function()), tok->astOperand1()->str(), argnr+1, _tokenizer->list.file(argtok), argtok->linenr(), argtok->str()));
}
// pointer to uninitialized data..
if (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->uninitialized.push_back(MyFileInfo::FunctionArg(FUNCTION_ID(tok->astOperand1()->function()), tok->astOperand1()->str(), argnr+1, _tokenizer->list.file(argtok), argtok->linenr(), argtok->str()));
}
}
// "Unsafe" functions unconditionally reads data before it is written..
CheckNullPointer checkNullPointer(_tokenizer, _settings, _errorLogger);
for (int argnr = 0; argnr < function->argCount(); ++argnr) {
const Token *tok;
if (isUnsafeFunction(&*scope, argnr, &tok))
fileInfo->readData.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok));
if (checkNullPointer.isUnsafeFunction(&*scope, argnr, &tok))
fileInfo->dereferenced.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok));
}
// Nested function calls
for (int argnr = 0; argnr < function->argCount(); ++argnr) {
const Token *tok;
int argnr2 = isCallFunction(&*scope, argnr, &tok);
if (argnr2 > 0) {
MyFileInfo::FunctionArg fa(_tokenizer, &*scope, argnr+1, tok);
fa.id = FUNCTION_ID(function);
fa.id2 = FUNCTION_ID(tok->function());
fa.argnr2 = argnr2;
fileInfo->nestedCall.push_back(fa);
}
}
}
return fileInfo;
}
Check::FileInfo * CheckUninitVar::loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const
{
MyFileInfo *fileInfo = nullptr;
for (const tinyxml2::XMLElement *e = xmlElement->FirstChildElement(); e; e = e->NextSiblingElement()) {
const char *id = e->Attribute("id");
if (!id)
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("linenr");
if (!linenr || !MathLib::isInt(linenr))
continue;
const char *variableName = e->Attribute("variableName");
if (!variableName)
continue;
const MyFileInfo::FunctionArg fa(id, functionName, MathLib::toLongNumber(argnr), fileName, MathLib::toLongNumber(linenr), variableName);
if (!fileInfo)
fileInfo = new MyFileInfo;
if (std::strcmp(e->Name(), "uninitialized") == 0)
fileInfo->uninitialized.push_back(fa);
else if (std::strcmp(e->Name(), "readData") == 0)
fileInfo->readData.push_back(fa);
else if (std::strcmp(e->Name(), "nullPointer") == 0)
fileInfo->nullPointer.push_back(fa);
else if (std::strcmp(e->Name(), "dereferenced") == 0)
fileInfo->dereferenced.push_back(fa);
else if (std::strcmp(e->Name(), "nestedCall") == 0)
fileInfo->nestedCall.push_back(fa);
else
throw InternalError(nullptr, "Wrong analyze info");
}
return fileInfo;
}
static bool findPath(const CheckUninitVar::MyFileInfo::FunctionArg &from,
const CheckUninitVar::MyFileInfo::FunctionArg &to,
const std::map<std::string, std::list<CheckUninitVar::MyFileInfo::FunctionArg>> &nestedCalls)
{
if (from.id == to.id && from.argnr == to.argnr)
return true;
const std::map<std::string, std::list<CheckUninitVar::MyFileInfo::FunctionArg>>::const_iterator nc = nestedCalls.find(from.id);
if (nc == nestedCalls.end())
return false;
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it = nc->second.begin(); it != nc->second.end(); ++it) {
if (from.id == it->id && from.argnr == it->argnr && it->id2 == to.id && it->argnr2 == to.argnr)
return true;
}
return false;
}
bool CheckUninitVar::analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger)
{
(void)settings; // This argument is unused
// Merge all fileinfo..
MyFileInfo all;
for (std::list<Check::FileInfo *>::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) {
const MyFileInfo *fi = dynamic_cast<MyFileInfo*>(*it);
if (!fi)
continue;
all.uninitialized.insert(all.uninitialized.end(), fi->uninitialized.begin(), fi->uninitialized.end());
all.readData.insert(all.readData.end(), fi->readData.begin(), fi->readData.end());
all.nullPointer.insert(all.nullPointer.end(), fi->nullPointer.begin(), fi->nullPointer.end());
all.dereferenced.insert(all.dereferenced.end(), fi->dereferenced.begin(), fi->dereferenced.end());
all.nestedCall.insert(all.nestedCall.end(), fi->nestedCall.begin(), fi->nestedCall.end());
}
bool foundErrors = false;
std::map<std::string, std::list<CheckUninitVar::MyFileInfo::FunctionArg>> nestedCalls;
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it = all.nestedCall.begin(); it != all.nestedCall.end(); ++it) {
std::list<CheckUninitVar::MyFileInfo::FunctionArg> &list = nestedCalls[it->id];
list.push_back(*it);
}
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it1 = all.uninitialized.begin(); it1 != all.uninitialized.end(); ++it1) {
const CheckUninitVar::MyFileInfo::FunctionArg &uninitialized = *it1;
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it2 = all.readData.begin(); it2 != all.readData.end(); ++it2) {
const CheckUninitVar::MyFileInfo::FunctionArg &readData = *it2;
if (!findPath(*it1, *it2, nestedCalls))
continue;
ErrorLogger::ErrorMessage::FileLocation fileLoc1;
fileLoc1.setfile(uninitialized.location.fileName);
fileLoc1.line = uninitialized.location.linenr;
fileLoc1.setinfo("Calling function " + uninitialized.functionName + ", variable " + uninitialized.variableName + " is uninitialized");
ErrorLogger::ErrorMessage::FileLocation fileLoc2;
fileLoc2.setfile(readData.location.fileName);
fileLoc2.line = readData.location.linenr;
fileLoc2.setinfo("Using argument " + readData.variableName);
std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
locationList.push_back(fileLoc1);
locationList.push_back(fileLoc2);
const ErrorLogger::ErrorMessage errmsg(locationList,
emptyString,
Severity::error,
"using argument " + readData.variableName + " that points at uninitialized variable " + uninitialized.variableName,
"ctuuninitvar",
CWE908, false);
errorLogger.reportErr(errmsg);
foundErrors = true;
}
}
// Null pointer checking
// TODO: This does not belong here.
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it1 = all.nullPointer.begin(); it1 != all.nullPointer.end(); ++it1) {
const CheckUninitVar::MyFileInfo::FunctionArg &nullPointer = *it1;
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it2 = all.dereferenced.begin(); it2 != all.dereferenced.end(); ++it2) {
const CheckUninitVar::MyFileInfo::FunctionArg &dereference = *it2;
if (!findPath(*it1, *it2, nestedCalls))
continue;
ErrorLogger::ErrorMessage::FileLocation fileLoc1;
fileLoc1.setfile(nullPointer.location.fileName);
fileLoc1.line = nullPointer.location.linenr;
fileLoc1.setinfo("Calling function " + nullPointer.functionName + ", " + MathLib::toString(nullPointer.argnr) + getOrdinalText(nullPointer.argnr) + " argument is null");
ErrorLogger::ErrorMessage::FileLocation fileLoc2;
fileLoc2.setfile(dereference.location.fileName);
fileLoc2.line = dereference.location.linenr;
fileLoc2.setinfo("Dereferencing argument " + dereference.variableName + " that is null");
std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
locationList.push_back(fileLoc1);
locationList.push_back(fileLoc2);
const ErrorLogger::ErrorMessage errmsg(locationList,
emptyString,
Severity::error,
"Null pointer dereference: " + dereference.variableName,
"ctunullpointer",
CWE476, false);
errorLogger.reportErr(errmsg);
foundErrors = true;
}
}
return foundErrors;
}

58
lib/checkuninitvar.h
View File

@ -88,61 +88,6 @@ public:
/** ValueFlow-based checking for uninitialized variables */
void valueFlowUninit();
/* data for multifile checking */
class MyFileInfo : public Check::FileInfo {
public:
struct FunctionArg {
FunctionArg(const std::string &id_, const std::string &functionName_, unsigned int argnr_, const std::string &fileName, unsigned int linenr, const std::string &varname)
: id(id_),
functionName(functionName_),
argnr(argnr_),
argnr2(0),
variableName(varname) {
location.fileName = fileName;
location.linenr = linenr;
}
FunctionArg(const Tokenizer *tokenizer, const Scope *scope, unsigned int argnr_, const Token *tok);
std::string id;
std::string id2;
std::string functionName;
unsigned int argnr;
unsigned int argnr2;
std::string variableName;
struct {
std::string fileName;
unsigned int linenr;
} location;
};
/** uninitialized function args */
std::list<FunctionArg> uninitialized;
/** function arguments that data are unconditionally read */
std::list<FunctionArg> readData;
/** null pointer function args */
std::list<FunctionArg> nullPointer;
/** function arguments that are unconditionally dereferenced */
std::list<FunctionArg> dereferenced;
/** function calls other function */
std::list<FunctionArg> nestedCall;
/** Convert MyFileInfo data into xml string */
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<Check::FileInfo*> &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);
@ -155,9 +100,6 @@ public:
void uninitStructMemberError(const Token *tok, const std::string &membername);
private:
Check::FileInfo *getFileInfo() const;
bool isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const;
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckUninitVar c(nullptr, settings, errorLogger);

63
test/testnullpointer.cpp
View File

@ -17,7 +17,6 @@
*/
#include "checknullpointer.h"
#include "checkuninitvar.h"
#include "library.h"
#include "settings.h"
#include "testsuite.h"
@ -106,7 +105,6 @@ private:
TEST_CASE(nullpointer_internal_error); // #5080
TEST_CASE(ticket6505);
TEST_CASE(subtract);
TEST_CASE(ctu);
}
void check(const char code[], bool inconclusive = false, const char filename[] = "test.cpp") {
@ -2574,67 +2572,6 @@ private:
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is overflow in pointer subtraction.\n", 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<Check::FileInfo*> 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 *fp) {\n"
" a = *fp;\n"
"}\n"
"int main() {\n"
" int *p = 0;\n"
" f(p);\n"
"}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Null pointer dereference: fp\n", errout.str());
ctu("void use(int *p) { a = *p + 3; }\n"
"void call(int x, int *p) { x++; use(p); }\n"
"int main() {\n"
" call(4,0);\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (error) Null pointer dereference: p\n", errout.str());
ctu("void dostuff(int *x, int *y) {\n"
" if (!var)\n"
" return -1;\n" // <- early return
" *x = *y;\n"
"}\n"
"\n"
"void f() {\n"
" dostuff(a, 0);\n"
"}");
ASSERT_EQUALS("", errout.str());
ctu("void dostuff(int *x, int *y) {\n"
" if (cond)\n"
" *y = -1;\n" // <- conditionally written
" *x = *y;\n"
"}\n"
"\n"
"void f() {\n"
" dostuff(a, 0);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestNullPointer)

68
test/testuninitvar.cpp
View File

@ -84,9 +84,6 @@ 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) {
@ -3979,71 +3976,6 @@ 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<Check::FileInfo*> 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;\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());
ctu("void use(int *p) { a = *p + 3; }\n"
"void call(int x, int *p) { x++; use(p); }\n"
"int main() {\n"
" int x;\n"
" call(4,&x);\n"
"}");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:1]: (error) using argument p that points at uninitialized variable x\n", errout.str());
ctu("void dostuff(int *x, int *y) {\n"
" if (!var)\n"
" return -1;\n" // <- early return
" *x = *y;\n"
"}\n"
"\n"
"void f() {\n"
" int x;\n"
" dostuff(a, &x);\n"
"}");
ASSERT_EQUALS("", errout.str());
ctu("void dostuff(int *x, int *y) {\n"
" if (cond)\n"
" *y = -1;\n" // <- conditionally written
" *x = *y;\n"
"}\n"
"\n"
"void f() {\n"
" int x;\n"
" dostuff(a, &x);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestUninitVar)