Skip to content

Commit 58066b1

Browse files
danmardanmar
committed
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.
1 parent 71511f3 commit 58066b1

File tree

6 files changed

+0
-541
lines changed

6 files changed

+0
-541
lines changed

‎lib/checknullpointer.cpp‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
//---------------------------------------------------------------------------
2121
#include "checknullpointer.h"
2222

23-
#include "astutils.h"
2423
#include "errorlogger.h"
2524
#include "library.h"
2625
#include "settings.h"
@@ -548,26 +547,3 @@ void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value
548547
CWE682, // unknown - pointer overflow
549548
value && value->isInconclusive());
550549
}
551-
552-
bool CheckNullPointer::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const
553-
{
554-
const Variable * const argvar = scope->function->getArgumentVar(argnr);
555-
if (!argvar->isPointer())
556-
return false;
557-
for (const Token *tok2 = scope->classStart; tok2 != scope->classEnd; tok2 = tok2->next()) {
558-
if (Token::simpleMatch(tok2, ") {")) {
559-
tok2 = tok2->linkAt(1);
560-
if (Token::findmatch(tok2->link(), "return|throw", tok2))
561-
return false;
562-
if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, _settings))
563-
return false;
564-
}
565-
if (tok2->variable() != argvar)
566-
continue;
567-
if (!Token::Match(tok2->astParent(), "*|["))
568-
return false;
569-
*tok = tok2;
570-
return true;
571-
}
572-
return false;
573-
}

‎lib/checknullpointer.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ class CPPCHECKLIB CheckNullPointer : public Check {
9999
nullPointerError(tok, "", &v, false);
100100
}
101101
void nullPointerError(const Token *tok, const std::string &varname, const ValueFlow::Value* value, bool inconclusive);
102-
103-
bool isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const;
104102
private:
105103

106104
/** Get error messages. Used by --errorlist */

‎lib/checkuninitvar.cpp‎

Lines changed: 0 additions & 326 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,329 +1282,3 @@ void CheckUninitVar::deadPointerError(const Token *pointer, const Token *alias)
12821282
"deadpointer",
12831283
"Dead pointer usage. Pointer '" + strpointer + "' is dead if it has been assigned '" + stralias + "' at line " + MathLib::toString(alias ? alias->linenr() : 0U) + ".", CWE825, false);
12841284
}
1285-
1286-
static void writeFunctionArgsXml(const std::list<CheckUninitVar::MyFileInfo::FunctionArg> &list, const std::string &elementName, std::ostream &out)
1287-
{
1288-
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it = list.begin(); it != list.end(); ++it)
1289-
out << " <" << elementName
1290-
<< " id=\"" << it->id << '\"'
1291-
<< " functionName=\"" << it->functionName << '\"'
1292-
<< " argnr=\"" << it->argnr << '\"'
1293-
<< " variableName=\"" << it->variableName << "\""
1294-
<< " fileName=\"" << it->location.fileName << '\"'
1295-
<< " linenr=\"" << it->location.linenr << '\"'
1296-
<< "/>\n";
1297-
}
1298-
1299-
std::string CheckUninitVar::MyFileInfo::toString() const
1300-
{
1301-
std::ostringstream ret;
1302-
writeFunctionArgsXml(uninitialized, "uninitialized", ret);
1303-
writeFunctionArgsXml(readData, "readData", ret);
1304-
writeFunctionArgsXml(nullPointer, "nullPointer", ret);
1305-
writeFunctionArgsXml(dereferenced, "dereferenced", ret);
1306-
writeFunctionArgsXml(nestedCall, "nestedCall", ret);
1307-
return ret.str();
1308-
}
1309-
1310-
#define FUNCTION_ID(function) _tokenizer->list.file(function->tokenDef) + ':' + MathLib::toString(function->tokenDef->linenr())
1311-
1312-
CheckUninitVar::MyFileInfo::FunctionArg::FunctionArg(const Tokenizer *_tokenizer, const Scope *scope, unsigned int argnr_, const Token *tok)
1313-
:
1314-
id(FUNCTION_ID(scope->function)),
1315-
functionName(scope->className),
1316-
argnr(argnr_),
1317-
argnr2(0),
1318-
variableName(scope->function->getArgumentVar(argnr-1)->name())
1319-
{
1320-
location.fileName = _tokenizer->list.file(tok);
1321-
location.linenr = tok->linenr();
1322-
}
1323-
1324-
bool CheckUninitVar::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const
1325-
{
1326-
const Variable * const argvar = scope->function->getArgumentVar(argnr);
1327-
if (!argvar->isPointer())
1328-
return false;
1329-
for (const Token *tok2 = scope->classStart; tok2 != scope->classEnd; tok2 = tok2->next()) {
1330-
if (Token::simpleMatch(tok2, ") {")) {
1331-
tok2 = tok2->linkAt(1);
1332-
if (Token::findmatch(tok2->link(), "return|throw", tok2))
1333-
return false;
1334-
if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, _settings))
1335-
return false;
1336-
}
1337-
if (tok2->variable() != argvar)
1338-
continue;
1339-
if (!isVariableUsage(tok2, true, Alloc::ARRAY))
1340-
return false;
1341-
*tok = tok2;
1342-
return true;
1343-
}
1344-
return false;
1345-
}
1346-
1347-
static int isCallFunction(const Scope *scope, int argnr, const Token **tok)
1348-
{
1349-
const Variable * const argvar = scope->function->getArgumentVar(argnr);
1350-
if (!argvar->isPointer())
1351-
return -1;
1352-
for (const Token *tok2 = scope->classStart; tok2 != scope->classEnd; tok2 = tok2->next()) {
1353-
if (tok2->variable() != argvar)
1354-
continue;
1355-
if (!Token::Match(tok2->previous(), "[(,] %var% [,)]"))
1356-
break;
1357-
int argnr2 = 1;
1358-
const Token *prev = tok2;
1359-
while (prev && prev->str() != "(") {
1360-
if (Token::Match(prev,"]|)"))
1361-
prev = prev->link();
1362-
else if (prev->str() == ",")
1363-
++argnr2;
1364-
prev = prev->previous();
1365-
}
1366-
if (!prev || !Token::Match(prev->previous(), "%name% ("))
1367-
break;
1368-
if (!prev->astOperand1() || !prev->astOperand1()->function())
1369-
break;
1370-
*tok = prev->previous();
1371-
return argnr2;
1372-
}
1373-
return -1;
1374-
}
1375-
1376-
Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const
1377-
{
1378-
const CheckUninitVar checker(tokenizer, settings, nullptr);
1379-
return checker.getFileInfo();
1380-
}
1381-
1382-
Check::FileInfo *CheckUninitVar::getFileInfo() const
1383-
{
1384-
const SymbolDatabase * const symbolDatabase = _tokenizer->getSymbolDatabase();
1385-
std::list<Scope>::const_iterator scope;
1386-
1387-
MyFileInfo *fileInfo = new MyFileInfo;
1388-
1389-
// Parse all functions in TU
1390-
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
1391-
if (!scope->isExecutable() || scope->type != Scope::eFunction || !scope->function)
1392-
continue;
1393-
const Function *const function = scope->function;
1394-
1395-
// function calls where uninitialized data is passed by address
1396-
for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
1397-
if (tok->str() != "(" || !tok->astOperand1() || !tok->astOperand2())
1398-
continue;
1399-
if (!tok->astOperand1()->function())
1400-
continue;
1401-
const std::vector<const Token *> args(getArguments(tok->previous()));
1402-
for (int argnr = 0; argnr < args.size(); ++argnr) {
1403-
const Token *argtok = args[argnr];
1404-
if (!argtok)
1405-
continue;
1406-
if (argtok->valueType() && argtok->valueType()->pointer > 0) {
1407-
// null pointer..
1408-
const ValueFlow::Value *value = argtok->getValue(0);
1409-
if (value && !value->isInconclusive())
1410-
fileInfo->nullPointer.push_back(MyFileInfo::FunctionArg(FUNCTION_ID(tok->astOperand1()->function()), tok->astOperand1()->str(), argnr+1, _tokenizer->list.file(argtok), argtok->linenr(), argtok->str()));
1411-
}
1412-
// pointer to uninitialized data..
1413-
if (argtok->str() != "&" || argtok->astOperand2())
1414-
continue;
1415-
argtok = argtok->astOperand1();
1416-
if (!argtok || !argtok->valueType() || argtok->valueType()->pointer != 0)
1417-
continue;
1418-
if (argtok->values().size() != 1U)
1419-
continue;
1420-
const ValueFlow::Value &v = argtok->values().front();
1421-
if (v.valueType != ValueFlow::Value::UNINIT || v.isInconclusive())
1422-
continue;
1423-
fileInfo->uninitialized.push_back(MyFileInfo::FunctionArg(FUNCTION_ID(tok->astOperand1()->function()), tok->astOperand1()->str(), argnr+1, _tokenizer->list.file(argtok), argtok->linenr(), argtok->str()));
1424-
}
1425-
}
1426-
1427-
// "Unsafe" functions unconditionally reads data before it is written..
1428-
CheckNullPointer checkNullPointer(_tokenizer, _settings, _errorLogger);
1429-
for (int argnr = 0; argnr < function->argCount(); ++argnr) {
1430-
const Token *tok;
1431-
if (isUnsafeFunction(&*scope, argnr, &tok))
1432-
fileInfo->readData.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok));
1433-
if (checkNullPointer.isUnsafeFunction(&*scope, argnr, &tok))
1434-
fileInfo->dereferenced.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok));
1435-
}
1436-
1437-
// Nested function calls
1438-
for (int argnr = 0; argnr < function->argCount(); ++argnr) {
1439-
const Token *tok;
1440-
int argnr2 = isCallFunction(&*scope, argnr, &tok);
1441-
if (argnr2 > 0) {
1442-
MyFileInfo::FunctionArg fa(_tokenizer, &*scope, argnr+1, tok);
1443-
fa.id = FUNCTION_ID(function);
1444-
fa.id2 = FUNCTION_ID(tok->function());
1445-
fa.argnr2 = argnr2;
1446-
fileInfo->nestedCall.push_back(fa);
1447-
}
1448-
}
1449-
}
1450-
1451-
return fileInfo;
1452-
}
1453-
1454-
Check::FileInfo * CheckUninitVar::loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const
1455-
{
1456-
MyFileInfo *fileInfo = nullptr;
1457-
for (const tinyxml2::XMLElement *e = xmlElement->FirstChildElement(); e; e = e->NextSiblingElement()) {
1458-
const char *id = e->Attribute("id");
1459-
if (!id)
1460-
continue;
1461-
const char *functionName = e->Attribute("functionName");
1462-
if (!functionName)
1463-
continue;
1464-
const char *argnr = e->Attribute("argnr");
1465-
if (!argnr || !MathLib::isInt(argnr))
1466-
continue;
1467-
const char *fileName = e->Attribute("fileName");
1468-
if (!fileName)
1469-
continue;
1470-
const char *linenr = e->Attribute("linenr");
1471-
if (!linenr || !MathLib::isInt(linenr))
1472-
continue;
1473-
const char *variableName = e->Attribute("variableName");
1474-
if (!variableName)
1475-
continue;
1476-
const MyFileInfo::FunctionArg fa(id, functionName, MathLib::toLongNumber(argnr), fileName, MathLib::toLongNumber(linenr), variableName);
1477-
if (!fileInfo)
1478-
fileInfo = new MyFileInfo;
1479-
if (std::strcmp(e->Name(), "uninitialized") == 0)
1480-
fileInfo->uninitialized.push_back(fa);
1481-
else if (std::strcmp(e->Name(), "readData") == 0)
1482-
fileInfo->readData.push_back(fa);
1483-
else if (std::strcmp(e->Name(), "nullPointer") == 0)
1484-
fileInfo->nullPointer.push_back(fa);
1485-
else if (std::strcmp(e->Name(), "dereferenced") == 0)
1486-
fileInfo->dereferenced.push_back(fa);
1487-
else if (std::strcmp(e->Name(), "nestedCall") == 0)
1488-
fileInfo->nestedCall.push_back(fa);
1489-
else
1490-
throw InternalError(nullptr, "Wrong analyze info");
1491-
}
1492-
return fileInfo;
1493-
}
1494-
1495-
static bool findPath(const CheckUninitVar::MyFileInfo::FunctionArg &from,
1496-
const CheckUninitVar::MyFileInfo::FunctionArg &to,
1497-
const std::map<std::string, std::list<CheckUninitVar::MyFileInfo::FunctionArg>> &nestedCalls)
1498-
{
1499-
if (from.id == to.id && from.argnr == to.argnr)
1500-
return true;
1501-
1502-
const std::map<std::string, std::list<CheckUninitVar::MyFileInfo::FunctionArg>>::const_iterator nc = nestedCalls.find(from.id);
1503-
if (nc == nestedCalls.end())
1504-
return false;
1505-
1506-
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it = nc->second.begin(); it != nc->second.end(); ++it) {
1507-
if (from.id == it->id && from.argnr == it->argnr && it->id2 == to.id && it->argnr2 == to.argnr)
1508-
return true;
1509-
}
1510-
1511-
return false;
1512-
}
1513-
1514-
bool CheckUninitVar::analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger)
1515-
{
1516-
(void)settings; // This argument is unused
1517-
1518-
// Merge all fileinfo..
1519-
MyFileInfo all;
1520-
for (std::list<Check::FileInfo *>::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) {
1521-
const MyFileInfo *fi = dynamic_cast<MyFileInfo*>(*it);
1522-
if (!fi)
1523-
continue;
1524-
all.uninitialized.insert(all.uninitialized.end(), fi->uninitialized.begin(), fi->uninitialized.end());
1525-
all.readData.insert(all.readData.end(), fi->readData.begin(), fi->readData.end());
1526-
all.nullPointer.insert(all.nullPointer.end(), fi->nullPointer.begin(), fi->nullPointer.end());
1527-
all.dereferenced.insert(all.dereferenced.end(), fi->dereferenced.begin(), fi->dereferenced.end());
1528-
all.nestedCall.insert(all.nestedCall.end(), fi->nestedCall.begin(), fi->nestedCall.end());
1529-
}
1530-
1531-
bool foundErrors = false;
1532-
1533-
std::map<std::string, std::list<CheckUninitVar::MyFileInfo::FunctionArg>> nestedCalls;
1534-
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it = all.nestedCall.begin(); it != all.nestedCall.end(); ++it) {
1535-
std::list<CheckUninitVar::MyFileInfo::FunctionArg> &list = nestedCalls[it->id];
1536-
list.push_back(*it);
1537-
}
1538-
1539-
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it1 = all.uninitialized.begin(); it1 != all.uninitialized.end(); ++it1) {
1540-
const CheckUninitVar::MyFileInfo::FunctionArg &uninitialized = *it1;
1541-
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it2 = all.readData.begin(); it2 != all.readData.end(); ++it2) {
1542-
const CheckUninitVar::MyFileInfo::FunctionArg &readData = *it2;
1543-
1544-
if (!findPath(*it1, *it2, nestedCalls))
1545-
continue;
1546-
1547-
ErrorLogger::ErrorMessage::FileLocation fileLoc1;
1548-
fileLoc1.setfile(uninitialized.location.fileName);
1549-
fileLoc1.line = uninitialized.location.linenr;
1550-
fileLoc1.setinfo("Calling function " + uninitialized.functionName + ", variable " + uninitialized.variableName + " is uninitialized");
1551-
1552-
ErrorLogger::ErrorMessage::FileLocation fileLoc2;
1553-
fileLoc2.setfile(readData.location.fileName);
1554-
fileLoc2.line = readData.location.linenr;
1555-
fileLoc2.setinfo("Using argument " + readData.variableName);
1556-
1557-
std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
1558-
locationList.push_back(fileLoc1);
1559-
locationList.push_back(fileLoc2);
1560-
1561-
const ErrorLogger::ErrorMessage errmsg(locationList,
1562-
emptyString,
1563-
Severity::error,
1564-
"using argument " + readData.variableName + " that points at uninitialized variable " + uninitialized.variableName,
1565-
"ctuuninitvar",
1566-
CWE908, false);
1567-
errorLogger.reportErr(errmsg);
1568-
1569-
foundErrors = true;
1570-
}
1571-
}
1572-
1573-
// Null pointer checking
1574-
// TODO: This does not belong here.
1575-
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it1 = all.nullPointer.begin(); it1 != all.nullPointer.end(); ++it1) {
1576-
const CheckUninitVar::MyFileInfo::FunctionArg &nullPointer = *it1;
1577-
for (std::list<CheckUninitVar::MyFileInfo::FunctionArg>::const_iterator it2 = all.dereferenced.begin(); it2 != all.dereferenced.end(); ++it2) {
1578-
const CheckUninitVar::MyFileInfo::FunctionArg &dereference = *it2;
1579-
1580-
if (!findPath(*it1, *it2, nestedCalls))
1581-
continue;
1582-
1583-
ErrorLogger::ErrorMessage::FileLocation fileLoc1;
1584-
fileLoc1.setfile(nullPointer.location.fileName);
1585-
fileLoc1.line = nullPointer.location.linenr;
1586-
fileLoc1.setinfo("Calling function " + nullPointer.functionName + ", " + MathLib::toString(nullPointer.argnr) + getOrdinalText(nullPointer.argnr) + " argument is null");
1587-
1588-
ErrorLogger::ErrorMessage::FileLocation fileLoc2;
1589-
fileLoc2.setfile(dereference.location.fileName);
1590-
fileLoc2.line = dereference.location.linenr;
1591-
fileLoc2.setinfo("Dereferencing argument " + dereference.variableName + " that is null");
1592-
1593-
std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
1594-
locationList.push_back(fileLoc1);
1595-
locationList.push_back(fileLoc2);
1596-
1597-
const ErrorLogger::ErrorMessage errmsg(locationList,
1598-
emptyString,
1599-
Severity::error,
1600-
"Null pointer dereference: " + dereference.variableName,
1601-
"ctunullpointer",
1602-
CWE476, false);
1603-
errorLogger.reportErr(errmsg);
1604-
1605-
foundErrors = true;
1606-
}
1607-
}
1608-
1609-
return foundErrors;
1610-
}

0 commit comments

Comments
 (0)