Skip to content

Commit

Permalink
Fix #13523 CTU: Mention including cpp file for error in header (#7174)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrchr-github authored Jan 8, 2025
1 parent 01b6141 commit 77a6705
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 15 deletions.
3 changes: 2 additions & 1 deletion lib/check.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,12 @@ class CPPCHECKLIB Check {
/** Base class used for whole-program analysis */
class CPPCHECKLIB FileInfo {
public:
FileInfo() = default;
explicit FileInfo(std::string f0 = {}) : file0(std::move(f0)) {}
virtual ~FileInfo() = default;
virtual std::string toString() const {
return std::string();
}
std::string file0;
};

virtual FileInfo * getFileInfo(const Tokenizer& /*tokenizer*/, const Settings& /*settings*/) const {
Expand Down
12 changes: 7 additions & 5 deletions lib/checkbufferoverrun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,7 @@ namespace
/** data for multifile checking */
class MyFileInfo : public Check::FileInfo {
public:
using Check::FileInfo::FileInfo;
/** unsafe array index usage */
std::list<CTU::FileInfo::UnsafeUsage> unsafeArrayIndex;

Expand Down Expand Up @@ -951,7 +952,7 @@ Check::FileInfo *CheckBufferOverrun::getFileInfo(const Tokenizer &tokenizer, con
if (unsafeArrayIndex.empty() && unsafePointerArith.empty()) {
return nullptr;
}
auto *fileInfo = new MyFileInfo;
auto *fileInfo = new MyFileInfo(tokenizer.list.getFiles()[0]);
fileInfo->unsafeArrayIndex = unsafeArrayIndex;
fileInfo->unsafePointerArith = unsafePointerArith;
return fileInfo;
Expand Down Expand Up @@ -998,14 +999,15 @@ bool CheckBufferOverrun::analyseWholeProgram(const CTU::FileInfo *ctu, const std
if (!fi)
continue;
for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafeArrayIndex)
foundErrors |= analyseWholeProgram1(callsMap, unsafeUsage, 1, errorLogger, settings.maxCtuDepth);
foundErrors |= analyseWholeProgram1(callsMap, unsafeUsage, 1, errorLogger, settings.maxCtuDepth, fi->file0);
for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafePointerArith)
foundErrors |= analyseWholeProgram1(callsMap, unsafeUsage, 2, errorLogger, settings.maxCtuDepth);
foundErrors |= analyseWholeProgram1(callsMap, unsafeUsage, 2, errorLogger, settings.maxCtuDepth, fi->file0);
}
return foundErrors;
}

bool CheckBufferOverrun::analyseWholeProgram1(const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger, int maxCtuDepth)
bool CheckBufferOverrun::analyseWholeProgram1(const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage,
int type, ErrorLogger &errorLogger, int maxCtuDepth, const std::string& file0)
{
const CTU::FileInfo::FunctionCall *functionCall = nullptr;

Expand Down Expand Up @@ -1038,7 +1040,7 @@ bool CheckBufferOverrun::analyseWholeProgram1(const std::map<std::string, std::l
}

const ErrorMessage errorMessage(locationList,
emptyString,
file0,
Severity::error,
errmsg,
errorId,
Expand Down
3 changes: 2 additions & 1 deletion lib/checkbufferoverrun.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
static bool isCtuUnsafePointerArith(const Settings &settings, const Token *argtok, MathLib::bigint *offset);

Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
static bool analyseWholeProgram1(const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger, int maxCtuDepth);
static bool analyseWholeProgram1(const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage,
int type, ErrorLogger &errorLogger, int maxCtuDepth, const std::string& file0);


static std::string myName() {
Expand Down
5 changes: 3 additions & 2 deletions lib/checkclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3566,6 +3566,7 @@ namespace
/* multifile checking; one definition rule violations */
class MyFileInfo : public Check::FileInfo {
public:
using Check::FileInfo::FileInfo;
struct NameLoc {
std::string className;
std::string fileName;
Expand Down Expand Up @@ -3662,7 +3663,7 @@ Check::FileInfo *CheckClass::getFileInfo(const Tokenizer &tokenizer, const Setti
if (classDefinitions.empty())
return nullptr;

auto *fileInfo = new MyFileInfo;
auto *fileInfo = new MyFileInfo(tokenizer.list.getFiles()[0]);
fileInfo->classDefinitions.swap(classDefinitions);
return fileInfo;
}
Expand Down Expand Up @@ -3728,7 +3729,7 @@ bool CheckClass::analyseWholeProgram(const CTU::FileInfo *ctu, const std::list<C
locationList.emplace_back(it->second.fileName, it->second.lineNumber, it->second.column);

const ErrorMessage errmsg(std::move(locationList),
emptyString,
fi->file0,
Severity::error,
"$symbol:" + nameLoc.className +
"\nThe one definition rule is violated, different classes/structs have the same name '$symbol'",
Expand Down
5 changes: 3 additions & 2 deletions lib/checknullpointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ namespace
/* data for multifile checking */
class MyFileInfo : public Check::FileInfo {
public:
using Check::FileInfo::FileInfo;
/** function arguments that are dereferenced without checking if they are null */
std::list<CTU::FileInfo::UnsafeUsage> unsafeUsage;

Expand All @@ -617,7 +618,7 @@ Check::FileInfo *CheckNullPointer::getFileInfo(const Tokenizer &tokenizer, const
if (unsafeUsage.empty())
return nullptr;

auto *fileInfo = new MyFileInfo;
auto *fileInfo = new MyFileInfo(tokenizer.list.getFiles()[0]);
fileInfo->unsafeUsage = unsafeUsage;
return fileInfo;
}
Expand Down Expand Up @@ -667,7 +668,7 @@ bool CheckNullPointer::analyseWholeProgram(const CTU::FileInfo *ctu, const std::
continue;

const ErrorMessage errmsg(locationList,
emptyString,
fi->file0,
warning ? Severity::warning : Severity::error,
"Null pointer dereference: " + unsafeUsage.myArgumentName,
"ctunullpointer",
Expand Down
5 changes: 3 additions & 2 deletions lib/checkuninitvar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,7 @@ namespace
/* data for multifile checking */
class MyFileInfo : public Check::FileInfo {
public:
using Check::FileInfo::FileInfo;
/** function arguments that data are unconditionally read */
std::list<CTU::FileInfo::UnsafeUsage> unsafeUsage;

Expand All @@ -1725,7 +1726,7 @@ Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer &tokenizer, const S
if (unsafeUsage.empty())
return nullptr;

auto *fileInfo = new MyFileInfo;
auto *fileInfo = new MyFileInfo(tokenizer.list.getFiles()[0]);
fileInfo->unsafeUsage = unsafeUsage;
return fileInfo;
}
Expand Down Expand Up @@ -1769,7 +1770,7 @@ bool CheckUninitVar::analyseWholeProgram(const CTU::FileInfo *ctu, const std::li
continue;

const ErrorMessage errmsg(locationList,
emptyString,
fi->file0,
Severity::error,
"Using argument " + unsafeUsage.myArgumentName + " that points at uninitialized variable " + functionCall->callArgumentExpression,
"ctuuninitvar",
Expand Down
7 changes: 5 additions & 2 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2041,8 +2041,11 @@ unsigned int CppCheck::analyseWholeProgram(const std::string &buildDir, const st
}
// cppcheck-suppress shadowFunction - TODO: fix this
for (const Check *check : Check::instances()) {
if (checkClassAttr == check->name())
fileInfoList.push_back(check->loadFileInfoFromXml(e));
if (checkClassAttr == check->name()) {
Check::FileInfo* fi = check->loadFileInfoFromXml(e);
fi->file0 = filesTxtLine.substr(firstColon + 2);
fileInfoList.push_back(fi);
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions test/cli/whole-program/nullpointer1.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#include "nullpointer1.h"
8 changes: 8 additions & 0 deletions test/cli/whole-program/nullpointer1.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#include "nullpointer1_1.h"

template<typename T>
void f(T* p) {
if (sizeof(T) == 4)
p = nullptr;
g(p);
}
4 changes: 4 additions & 0 deletions test/cli/whole-program/nullpointer1_1.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
template<typename T>
void g(T* p) {
*p = 0;
};
33 changes: 33 additions & 0 deletions test/cli/whole-program_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import json
import shutil
from testutils import cppcheck
import xml.etree.ElementTree as ET

__script_dir = os.path.dirname(os.path.abspath(__file__))

Expand Down Expand Up @@ -358,3 +359,35 @@ def test_checkclass_project_builddir_j(tmpdir):
build_dir = os.path.join(tmpdir, 'b1')
os.mkdir(build_dir)
__test_checkclass_project(tmpdir, ['-j2', '--cppcheck-build-dir={}'.format(build_dir)])

def __test_nullpointer_file0(extra_args):
args = [
'-q',
'--xml',
'--error-exitcode=1',
'whole-program/nullpointer1.cpp'
]

args += extra_args

ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
results = ET.fromstring(stderr)
file0 = ''
for e in results.findall('errors/error[@id="ctunullpointer"]'):
file0 = e.attrib['file0']

assert ret == 1, stdout if stdout else stderr
assert stdout == ''
assert file0 == 'whole-program/nullpointer1.cpp', stderr

def test_nullpointer_file0():
__test_nullpointer_file0(['-j1'])

@pytest.mark.xfail(strict=True) # no CTU without builddir
def test_nullpointer_file0_j():
__test_nullpointer_file0(['-j2', '--no-cppcheck-build-dir'])

def test_nullpointer_file0_builddir_j(tmpdir):
build_dir = os.path.join(tmpdir, 'b1')
os.mkdir(build_dir)
__test_nullpointer_file0(['-j2', '--cppcheck-build-dir={}'.format(build_dir)])

0 comments on commit 77a6705

Please sign in to comment.