Skip to content

Commit de7e922

Browse files
danmardanmar
committed
Fixed cppcheck-opensource#6272 (Improve check: multifile checking in checkbufferoverrun)
1 parent 5786be9 commit de7e922

10 files changed

Lines changed: 192 additions & 64 deletions

‎cli/cppcheckexecutor.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha
786786
}
787787

788788
cppcheck.checkFunctionUsage();
789+
cppcheck.analyseWholeProgram();
789790
} else if (!ThreadExecutor::isEnabled()) {
790791
std::cout << "No thread support yet implemented for this platform." << std::endl;
791792
} else {

‎lib/check.h‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,26 +58,6 @@ class CPPCHECKLIB Check {
5858
return _instances;
5959
}
6060

61-
/**
62-
* analyse code - must be thread safe
63-
* @param tokens The tokens to analyse
64-
* @param result container where results are stored
65-
*/
66-
virtual void analyse(const Token *tokens, std::set<std::string> &result) const {
67-
// suppress compiler warnings
68-
(void)tokens;
69-
(void)result;
70-
}
71-
72-
/**
73-
* Save analysis data - the caller ensures thread safety
74-
* @param data The data where the results are saved
75-
*/
76-
virtual void saveAnalysisData(const std::set<std::string> &data) const {
77-
// suppress compiler warnings
78-
(void)data;
79-
}
80-
8161
/** run checks, the token list is not simplified */
8262
virtual void runChecks(const Tokenizer *, const Settings *, ErrorLogger *) {
8363
}
@@ -107,6 +87,23 @@ class CPPCHECKLIB Check {
10787
return _settings && _settings->inconclusive;
10888
}
10989

90+
/** Base class used for whole-program analysis */
91+
class FileInfo {
92+
public:
93+
FileInfo() {}
94+
virtual ~FileInfo() {}
95+
};
96+
97+
virtual FileInfo * getFileInfo(const Tokenizer *tokenizer) const {
98+
(void)tokenizer;
99+
return nullptr;
100+
}
101+
102+
virtual void analyseWholeProgram(const std::list<FileInfo*> &fileInfo, ErrorLogger &errorLogger) {
103+
(void)fileInfo;
104+
(void)errorLogger;
105+
}
106+
110107
protected:
111108
const Tokenizer * const _tokenizer;
112109
const Settings * const _settings;

‎lib/checkbufferoverrun.cpp‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,3 +1804,94 @@ void CheckBufferOverrun::writeOutsideBufferSizeError(const Token *tok, const std
18041804
"The number of bytes to write (" + MathLib::toString(writeLength) + " bytes) are bigger than the source buffer (" +MathLib::toString(stringLength)+ " bytes)."
18051805
" Please check the second and the third parameter of the function '"+strFunctionName+"'.");
18061806
}
1807+
1808+
Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer) const
1809+
{
1810+
MyFileInfo *fileInfo = new MyFileInfo;
1811+
1812+
// Array usage..
1813+
const SymbolDatabase* const symbolDatabase = tokenizer->getSymbolDatabase();
1814+
const std::size_t functions = symbolDatabase->functionScopes.size();
1815+
for (std::size_t i = 0; i < functions; ++i) {
1816+
const Scope * const scope = symbolDatabase->functionScopes[i];
1817+
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
1818+
if (Token::Match(tok, "%var% [") &&
1819+
Token::Match(tok->linkAt(1), "] !![") &&
1820+
tok->variable() &&
1821+
tok->variable()->isExtern() &&
1822+
tok->variable()->isGlobal() &&
1823+
tok->next()->astOperand2()) {
1824+
const ValueFlow::Value *value = tok->next()->astOperand2()->getMaxValue(false);
1825+
if (value && value->intvalue > 0) {
1826+
struct MyFileInfo::ArrayUsage arrayUsage;
1827+
arrayUsage.index = value->intvalue;
1828+
arrayUsage.fileName = tokenizer->list.file(tok);
1829+
arrayUsage.linenr = tok->linenr();
1830+
std::map<std::string, struct MyFileInfo::ArrayUsage>::iterator it = fileInfo->arrayUsage.find(tok->str());
1831+
if (it == fileInfo->arrayUsage.end() || it->second.index < arrayUsage.index)
1832+
fileInfo->arrayUsage[tok->str()] = arrayUsage;
1833+
}
1834+
}
1835+
}
1836+
}
1837+
1838+
// Arrays..
1839+
const std::list<Variable> &varlist = symbolDatabase->scopeList.front().varlist;
1840+
for (std::list<Variable>::const_iterator it = varlist.begin(); it != varlist.end(); ++it) {
1841+
const Variable &var = *it;
1842+
if (!var.isStatic() && var.isArray() && var.dimensions().size() == 1U)
1843+
fileInfo->arraySize[var.name()] = var.dimension(0U);
1844+
}
1845+
1846+
return fileInfo;
1847+
}
1848+
1849+
void CheckBufferOverrun::analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger)
1850+
{
1851+
// Merge all fileInfo
1852+
MyFileInfo all;
1853+
for (std::list<Check::FileInfo*>::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) {
1854+
const MyFileInfo *fi = dynamic_cast<MyFileInfo*>(*it);
1855+
if (!fi)
1856+
continue;
1857+
1858+
// merge array usage
1859+
for (std::map<std::string, struct MyFileInfo::ArrayUsage>::const_iterator it2 = fi->arrayUsage.begin(); it2 != fi->arrayUsage.end(); ++it2) {
1860+
std::map<std::string, struct MyFileInfo::ArrayUsage>::const_iterator allit = all.arrayUsage.find(it2->first);
1861+
if (allit == all.arrayUsage.end() || it2->second.index > allit->second.index)
1862+
all.arrayUsage[it2->first] = it2->second;
1863+
}
1864+
1865+
// merge array info
1866+
for (std::map<std::string, MathLib::bigint>::const_iterator it2 = fi->arraySize.begin(); it2 != fi->arraySize.end(); ++it2) {
1867+
std::map<std::string, MathLib::bigint>::const_iterator allit = all.arraySize.find(it2->first);
1868+
if (allit == all.arraySize.end())
1869+
all.arraySize[it2->first] = it2->second;
1870+
else
1871+
all.arraySize[it2->first] = -1;
1872+
}
1873+
}
1874+
1875+
// Check buffer usage
1876+
for (std::map<std::string, struct MyFileInfo::ArrayUsage>::const_iterator it = all.arrayUsage.begin(); it != all.arrayUsage.end(); ++it) {
1877+
std::map<std::string, MathLib::bigint>::const_iterator sz = all.arraySize.find(it->first);
1878+
if (sz != all.arraySize.end() && sz->second > 0 && sz->second < it->second.index) {
1879+
ErrorLogger::ErrorMessage::FileLocation fileLoc;
1880+
fileLoc.setfile(it->second.fileName);
1881+
fileLoc.line = it->second.linenr;
1882+
1883+
std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
1884+
locationList.push_back(fileLoc);
1885+
1886+
std::ostringstream ostr;
1887+
ostr << "Array " << it->first << '[' << sz->second << "] accessed at index " << it->second.index << " which is out of bounds";
1888+
1889+
const ErrorLogger::ErrorMessage errmsg(locationList,
1890+
Severity::error,
1891+
ostr.str(),
1892+
"arrayIndexOutOfBounds",
1893+
false);
1894+
errorLogger.reportErr(errmsg);
1895+
}
1896+
}
1897+
}

‎lib/checkbufferoverrun.h‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,28 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
203203
void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<MathLib::bigint> &index);
204204
void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<ValueFlow::Value> &index);
205205

206+
/* data for multifile checking */
207+
class MyFileInfo : public Check::FileInfo {
208+
public:
209+
struct ArrayUsage {
210+
MathLib::bigint index;
211+
std::string fileName;
212+
unsigned int linenr;
213+
};
214+
215+
/* key:arrayName */
216+
std::map<std::string, struct ArrayUsage> arrayUsage;
217+
218+
/* key:arrayName, data:arraySize */
219+
std::map<std::string, MathLib::bigint> arraySize;
220+
};
221+
222+
/** @brief Parse current TU and extract file info */
223+
Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const;
224+
225+
/** @brief Analyse all file infos for all TU */
226+
virtual void analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger);
227+
206228
private:
207229

208230
static bool isArrayOfStruct(const Token* tok, int &position);

‎lib/checkmemoryleak.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2678,7 +2678,7 @@ void CheckMemoryLeakNoVar::check()
26782678
std::set<std::string> uvarFunctions;
26792679
{
26802680
const CheckUninitVar c(_tokenizer, _settings, _errorLogger);
2681-
c.analyse(_tokenizer->tokens(), uvarFunctions);
2681+
c.analyseFunctions(_tokenizer, uvarFunctions);
26822682
}
26832683

26842684
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();

‎lib/checkuninitvar.cpp‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ class UninitVar : public ExecutionPath {
601601
}
602602
}
603603

604-
if (Token::Match(&tok, "%var% (") && uvarFunctions.find(tok.str()) == uvarFunctions.end()) {
604+
if (Token::Match(&tok, "%var% (")) {
605605
// sizeof/typeof doesn't dereference. A function name that is all uppercase
606606
// might be an unexpanded macro that uses sizeof/typeof
607607
if (Token::Match(&tok, "sizeof|typeof ("))
@@ -920,9 +920,6 @@ class UninitVar : public ExecutionPath {
920920

921921
public:
922922

923-
/** Functions that don't handle uninitialized variables well */
924-
static std::set<std::string> uvarFunctions;
925-
926923
static void analyseFunctions(const Token * const tokens, std::set<std::string> &func) {
927924
for (const Token *tok = tokens; tok; tok = tok->next()) {
928925
if (tok->str() == "{") {
@@ -1013,34 +1010,33 @@ class UninitVar : public ExecutionPath {
10131010
}
10141011
};
10151012

1016-
/** Functions that don't handle uninitialized variables well */
1017-
std::set<std::string> UninitVar::uvarFunctions;
1018-
1019-
10201013
/// @}
10211014

10221015

1023-
void CheckUninitVar::analyse(const Token * tokens, std::set<std::string> &func) const
1016+
Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer) const
1017+
{
1018+
MyFileInfo * mfi = new MyFileInfo;
1019+
analyseFunctions(tokenizer, mfi->uvarFunctions);
1020+
// TODO: add suspicious function calls
1021+
return mfi;
1022+
}
1023+
1024+
void CheckUninitVar::analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger)
10241025
{
1025-
UninitVar::analyseFunctions(tokens, func);
1026+
(void)fileInfo;
1027+
(void)errorLogger;
10261028
}
10271029

1028-
void CheckUninitVar::saveAnalysisData(const std::set<std::string> &data) const
1030+
void CheckUninitVar::analyseFunctions(const Tokenizer *tokenizer, std::set<std::string> &f) const
10291031
{
1030-
UninitVar::uvarFunctions.insert(data.begin(), data.end());
1032+
UninitVar::analyseFunctions(tokenizer->tokens(), f);
10311033
}
10321034

10331035
void CheckUninitVar::executionPaths()
10341036
{
10351037
// check if variable is accessed uninitialized..
1036-
{
1037-
// no writing if multiple threads are used (TODO: thread safe analysis?)
1038-
if (_settings->_jobs == 1)
1039-
UninitVar::analyseFunctions(_tokenizer->tokens(), UninitVar::uvarFunctions);
1040-
1041-
UninitVar c(this, _tokenizer->getSymbolDatabase(), &_settings->library, _tokenizer->isC());
1042-
checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c);
1043-
}
1038+
UninitVar c(this, _tokenizer->getSymbolDatabase(), &_settings->library, _tokenizer->isC());
1039+
checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c);
10441040
}
10451041

10461042

‎lib/checkuninitvar.h‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,23 @@ class CPPCHECKLIB CheckUninitVar : public Check {
6969
void deadPointer();
7070
void deadPointerError(const Token *pointer, const Token *alias);
7171

72-
/**
73-
* @brief Uninitialized variables: analyse functions to see how they work with uninitialized variables
74-
* @param tokens [in] the token list
75-
* @param func [out] names of functions that don't handle uninitialized variables well. the function names are added to the set. No clearing is made.
76-
*/
77-
void analyse(const Token * tokens, std::set<std::string> &func) const;
78-
79-
/** Save analysis results */
80-
void saveAnalysisData(const std::set<std::string> &data) const;
72+
/* data for multifile checking */
73+
class MyFileInfo : public Check::FileInfo {
74+
public:
75+
/* functions that must have initialized data */
76+
std::set<std::string> uvarFunctions;
77+
78+
/* functions calls with uninitialized data */
79+
std::set<std::string> functionCalls;
80+
};
81+
82+
/** @brief Parse current TU and extract file info */
83+
Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const;
84+
85+
/** @brief Analyse all file infos for all TU */
86+
virtual void analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger);
87+
88+
void analyseFunctions(const Tokenizer *tokenizer, std::set<std::string> &f) const;
8189

8290
/** @brief new type of check: check execution paths */
8391
void executionPaths();

‎lib/cppcheck.cpp‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ CppCheck::CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions)
4848

4949
CppCheck::~CppCheck()
5050
{
51+
while (!fileInfo.empty()) {
52+
delete fileInfo.back();
53+
fileInfo.pop_back();
54+
}
5155
S_timerResults.ShowResults(_settings._showtime);
5256
}
5357

@@ -309,19 +313,6 @@ void CppCheck::analyseFile(std::istream &fin, const std::string &filename)
309313
std::istringstream istr(code);
310314
tokenizer.tokenize(istr, filename.c_str());
311315
tokenizer.simplifyTokenList2();
312-
313-
// Analyse the tokens..
314-
std::set<std::string> data;
315-
for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {
316-
(*it)->analyse(tokenizer.tokens(), data);
317-
}
318-
319-
// Save analysis results..
320-
// TODO: This loop should be protected by a mutex or something like that
321-
// The saveAnalysisData must _not_ be called from many threads at the same time.
322-
for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {
323-
(*it)->saveAnalysisData(data);
324-
}
325316
}
326317

327318
//---------------------------------------------------------------------------
@@ -411,6 +402,13 @@ bool CppCheck::checkFile(const std::string &code, const char FileName[], std::se
411402
(*it)->runSimplifiedChecks(&_tokenizer, &_settings, this);
412403
}
413404

405+
// Analyse the tokens..
406+
for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {
407+
Check::FileInfo *fi = (*it)->getFileInfo(&_tokenizer);
408+
if (fi != nullptr)
409+
fileInfo.push_back(fi);
410+
}
411+
414412
if (_settings.terminated())
415413
return true;
416414

@@ -690,3 +688,11 @@ void CppCheck::getErrorMessages()
690688
Tokenizer::getErrorMessages(this, &_settings);
691689
Preprocessor::getErrorMessages(this, &_settings);
692690
}
691+
692+
void CppCheck::analyseWholeProgram()
693+
{
694+
// Analyse the tokens..
695+
for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it)
696+
(*it)->analyseWholeProgram(fileInfo, *this);
697+
}
698+

‎lib/cppcheck.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "config.h"
2525
#include "settings.h"
2626
#include "errorlogger.h"
27+
#include "check.h"
2728

2829
#include <string>
2930
#include <list>
@@ -134,6 +135,9 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
134135
_simplify = false;
135136
}
136137

138+
/** analyse whole program, run this after all TUs has been scanned. */
139+
void analyseWholeProgram();
140+
137141
private:
138142

139143
/** @brief There has been a internal error => Report information message */
@@ -210,6 +214,9 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
210214

211215
/** Simplify code? true by default */
212216
bool _simplify;
217+
218+
/** File info used for whole program analysis */
219+
std::list<Check::FileInfo*> fileInfo;
213220
};
214221

215222
/// @}

‎test/testuninitvar.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,7 @@ class TestUninitVar : public TestFixture {
17351735

17361736
std::set<std::string> f;
17371737
const CheckUninitVar check((const Tokenizer *)0, (const Settings *)0, (ErrorLogger *)0);
1738-
check.analyse(tokenizer.tokens(), f);
1738+
check.analyseFunctions(&tokenizer, f);
17391739

17401740
std::string ret;
17411741
for (std::set<std::string>::const_iterator it = f.begin(); it != f.end(); ++it)

0 commit comments

Comments
 (0)