Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Comment thread
chrchr-github marked this conversation as resolved.
/** 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
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;
};
31 changes: 31 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,33 @@ 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'):
if e.attrib['id'] == 'ctunullpointer':
if 'file0' in e.attrib:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this is necessary, but there was a KeyError without it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That means that it doesn't contain that key. [] acts like at()in C++.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but there should only be one ctunullpointer error, which has the file0 attribute.
XML shown here: https://github.com/danmar/cppcheck/actions/runs/12605458944/job/35134122531
KeyError: https://github.com/danmar/cppcheck/actions/runs/12605258027/job/35133607619

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a few more asserts on the results. Like the amount of returned nodes and such.

You could also limit the XPath to only return the node in question by adding [@id='ctunullpointer'].

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a few more asserts on the results. Like the amount of returned nodes and such.

Not so easy, since getting both nullpointer and ctunullpointer seems wrong already. With -j2, we get an additional ctunullpointer (https://trac.cppcheck.net/ticket/11883)

You could also limit the XPath to only return the node in question by adding [@id='ctunullpointer'].

Done.

file0 = e.attrib['file0']
assert file0 == 'whole-program/nullpointer1.cpp'
assert stdout == ''
assert ret == 1, stdout


def test_nullpointer_file0():
__test_nullpointer_file0(['-j1'])
Comment thread
chrchr-github marked this conversation as resolved.

def test_nullpointer_file0_j():
build_dir = os.path.join(tmpdir, 'b1')
Comment thread
chrchr-github marked this conversation as resolved.
os.mkdir(build_dir)
__test_nullpointer_file0(['-j2', '--cppcheck-build-dir={}'.format(build_dir)])