Skip to content

Commit 6c57803

Browse files
authored
fixed danmar#6366 - some unique errors were omitted (danmar#4377)
1 parent a78fdf5 commit 6c57803

File tree

10 files changed

+215
-23
lines changed

10 files changed

+215
-23
lines changed

Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ TESTOBJ = test/fixture.o \
286286
test/testcppcheck.o \
287287
test/testerrorlogger.o \
288288
test/testexceptionsafety.o \
289+
test/testexecutor.o \
289290
test/testfilelister.o \
290291
test/testfilesettings.o \
291292
test/testfunctions.o \
@@ -751,6 +752,9 @@ test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h l
751752
test/testexceptionsafety.o: test/testexceptionsafety.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/checkexceptionsafety.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
752753
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testexceptionsafety.cpp
753754

755+
test/testexecutor.o: test/testexecutor.cpp cli/executor.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h
756+
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testexecutor.cpp
757+
754758
test/testfilelister.o: test/testfilelister.cpp cli/filelister.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/utils.h test/fixture.h
755759
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testfilelister.cpp
756760

cli/cppcheckexecutor.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -641,23 +641,24 @@ void StdLogger::reportErr(const ErrorMessage &msg)
641641
if (msg.severity == Severity::internal)
642642
return;
643643

644-
// TODO: we generate a different message here then we log below
645-
// TODO: there should be no need for verbose and default messages here
646-
// Alert only about unique errors
647-
if (!mSettings.emitDuplicates && !mShownErrors.insert(msg.toString(mSettings.verbose)).second)
648-
return;
649-
650644
ErrorMessage msgCopy = msg;
651645
msgCopy.guideline = getGuideline(msgCopy.id, mSettings.reportType,
652646
mGuidelineMapping, msgCopy.severity);
653647
msgCopy.classification = getClassification(msgCopy.guideline, mSettings.reportType);
654648

649+
// TODO: there should be no need for verbose and default messages here
650+
const std::string msgStr = msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
651+
652+
// Alert only about unique errors
653+
if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second)
654+
return;
655+
655656
if (mSettings.outputFormat == Settings::OutputFormat::sarif)
656657
mSarifReport.addFinding(std::move(msgCopy));
657658
else if (mSettings.outputFormat == Settings::OutputFormat::xml)
658659
reportErr(msgCopy.toXML());
659660
else
660-
reportErr(msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation));
661+
reportErr(msgStr);
661662
}
662663

663664
/**

cli/executor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ bool Executor::hasToLog(const ErrorMessage &msg)
4646
if (!mSuppressions.nomsg.isSuppressed(msg, {}))
4747
{
4848
// TODO: there should be no need for verbose and default messages here
49-
std::string errmsg = msg.toString(mSettings.verbose);
49+
std::string errmsg = msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
5050
if (errmsg.empty())
5151
return false;
5252

lib/cppcheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ class CppCheck::CppCheckLogger : public ErrorLogger
203203
}
204204

205205
// TODO: there should be no need for the verbose and default messages here
206-
std::string errmsg = msg.toString(mSettings.verbose);
206+
std::string errmsg = msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
207207
if (errmsg.empty())
208208
return;
209209

samples/unreadVariable/out.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
samples\unreadVariable\bad.cpp:5:34: style: Variable 's2' is assigned a value that is never used. [unreadVariable]
22
std::string s1 = "test1", s2 = "test2";
33
^
4+
samples\unreadVariable\bad.cpp:5:31: style: Variable 's2' is assigned a value that is never used. [unreadVariable]
5+
std::string s1 = "test1", s2 = "test2";
6+
^

test/cli/other_test.py

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3171,21 +3171,21 @@ def test_check_headers(tmp_path):
31713171
test_file_h = tmp_path / 'test.h'
31723172
with open(test_file_h, 'wt') as f:
31733173
f.write(
3174-
"""
3175-
inline void hdr()
3176-
{
3177-
(void)(*((int*)0));
3178-
}
3179-
""")
3174+
"""
3175+
inline void hdr()
3176+
{
3177+
(void)(*((int*)0));
3178+
}
3179+
""")
31803180

31813181
test_file_c = tmp_path / 'test.c'
31823182
with open(test_file_c, 'wt') as f:
31833183
f.write(
3184-
"""
3185-
#include "test.h"
3186-
3187-
void f() {}
3188-
""")
3184+
"""
3185+
#include "test.h"
3186+
3187+
void f() {}
3188+
""")
31893189

31903190
args = [
31913191
'-q',
@@ -3196,4 +3196,32 @@ def test_check_headers(tmp_path):
31963196
exitcode, stdout, stderr = cppcheck(args)
31973197
assert exitcode == 0, stdout
31983198
assert stdout.splitlines() == []
3199-
assert stderr.splitlines() == [] # no error since the header is not checked
3199+
assert stderr.splitlines() == [] # no error since the header is not checked
3200+
3201+
3202+
3203+
def test_unique_error(tmp_path): # #6366
3204+
test_file = tmp_path / 'test.c'
3205+
with open(test_file, 'wt') as f:
3206+
f.write(
3207+
"""void f()
3208+
{
3209+
const long m[9] = {};
3210+
long a=m[9], b=m[9];
3211+
(void)a;
3212+
(void)b;
3213+
}
3214+
""")
3215+
3216+
args = [
3217+
'-q',
3218+
'--template=simple',
3219+
str(test_file)
3220+
]
3221+
exitcode, stdout, stderr = cppcheck(args)
3222+
assert exitcode == 0, stdout
3223+
assert stdout.splitlines() == []
3224+
assert stderr.splitlines() == [
3225+
"{}:4:13: error: Array 'm[9]' accessed at index 9, which is out of bounds. [arrayIndexOutOfBounds]".format(test_file),
3226+
"{}:4:21: error: Array 'm[9]' accessed at index 9, which is out of bounds. [arrayIndexOutOfBounds]".format(test_file)
3227+
]

test/cli/whole-program_test.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,11 @@ def test_nullpointer_file0_builddir_j(tmpdir):
372372
os.mkdir(build_dir)
373373
__test_nullpointer_file0(['-j2', '--cppcheck-build-dir={}'.format(build_dir)])
374374

375-
@pytest.mark.parametrize("single_file", (False,True))
375+
# TODO: this only succeeded because it depedent on the bugged unqiue message handling
376+
@pytest.mark.parametrize("single_file", [
377+
False,
378+
pytest.param(True, marks=pytest.mark.xfail(strict=True)),
379+
])
376380
def test_nullpointer_out_of_memory(tmpdir, single_file):
377381
"""Ensure that there are not duplicate warnings related to memory/resource allocation failures
378382
https://trac.cppcheck.net/ticket/13521

test/testcppcheck.cpp

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ class TestCppcheck : public TestFixture {
5959
TEST_CASE(checkWithFS);
6060
TEST_CASE(suppress_error_library);
6161
TEST_CASE(unique_errors);
62+
TEST_CASE(unique_errors_2);
6263
TEST_CASE(isPremiumCodingStandardId);
6364
TEST_CASE(getDumpFileContentsRawTokens);
6465
TEST_CASE(getDumpFileContentsLibrary);
@@ -170,7 +171,7 @@ class TestCppcheck : public TestFixture {
170171
ASSERT_EQUALS(0, errorLogger.ids.size());
171172
}
172173

173-
// TODO: hwo to actually get duplicated findings
174+
// TODO: how to actually get duplicated findings
174175
void unique_errors() const
175176
{
176177
ScopedFile file("inc.h",
@@ -196,11 +197,57 @@ class TestCppcheck : public TestFixture {
196197
// the internal errorlist is cleared after each check() call
197198
ASSERT_EQUALS(2, errorLogger.errmsgs.size());
198199
auto it = errorLogger.errmsgs.cbegin();
200+
ASSERT_EQUALS("a.cpp", it->file0);
199201
ASSERT_EQUALS("nullPointer", it->id);
200202
++it;
203+
ASSERT_EQUALS("b.cpp", it->file0);
201204
ASSERT_EQUALS("nullPointer", it->id);
202205
}
203206

207+
void unique_errors_2() const
208+
{
209+
ScopedFile test_file("c.cpp",
210+
"void f()\n"
211+
"{\n"
212+
"const long m[9] = {};\n"
213+
"long a=m[9], b=m[9];\n"
214+
"(void)a;\n"
215+
"(void)b;\n"
216+
"}");
217+
218+
Settings s;
219+
// this is the "simple" format
220+
s.templateFormat = "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]";
221+
Suppressions supprs;
222+
ErrorLogger2 errorLogger;
223+
CppCheck cppcheck(s, supprs, errorLogger, false, {});
224+
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file.path())));
225+
// TODO: how to properly disable these warnings?
226+
errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
227+
return msg.id == "logChecker";
228+
}), errorLogger.errmsgs.end());
229+
// the internal errorlist is cleared after each check() call
230+
ASSERT_EQUALS(2, errorLogger.errmsgs.size());
231+
auto it = errorLogger.errmsgs.cbegin();
232+
ASSERT_EQUALS("c.cpp", it->file0);
233+
ASSERT_EQUALS(1, it->callStack.size());
234+
{
235+
auto stack = it->callStack.cbegin();
236+
ASSERT_EQUALS(4, stack->line);
237+
ASSERT_EQUALS(9, stack->column);
238+
}
239+
ASSERT_EQUALS("arrayIndexOutOfBounds", it->id);
240+
++it;
241+
ASSERT_EQUALS("c.cpp", it->file0);
242+
ASSERT_EQUALS(1, it->callStack.size());
243+
{
244+
auto stack = it->callStack.cbegin();
245+
ASSERT_EQUALS(4, stack->line);
246+
ASSERT_EQUALS(17, stack->column);
247+
}
248+
ASSERT_EQUALS("arrayIndexOutOfBounds", it->id);
249+
}
250+
204251
void isPremiumCodingStandardId() const {
205252
Suppressions supprs;
206253
ErrorLogger2 errorLogger;

test/testexecutor.cpp

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2025 Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include "executor.h"
20+
#include "filesettings.h"
21+
#include "fixture.h"
22+
#include "suppressions.h"
23+
24+
#include <stdexcept>
25+
26+
class DummyExecutor : public Executor
27+
{
28+
public:
29+
DummyExecutor(const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger)
30+
: Executor(files, fileSettings, settings, suppressions, errorLogger)
31+
{}
32+
33+
unsigned int check() override
34+
{
35+
throw std::runtime_error("not implemented");
36+
}
37+
38+
bool hasToLog_(const ErrorMessage &msg)
39+
{
40+
return hasToLog(msg);
41+
}
42+
};
43+
44+
class TestExecutor : public TestFixture {
45+
public:
46+
TestExecutor() : TestFixture("TestExecutor") {}
47+
48+
private:
49+
void run() override {
50+
TEST_CASE(hasToLogDefault);
51+
TEST_CASE(hasToLogSimple);
52+
}
53+
54+
void hasToLogDefault() {
55+
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
56+
const std::list<FileSettings> fileSettings;
57+
Suppressions supprs;
58+
DummyExecutor executor(files, fileSettings, settingsDefault, supprs, *this);
59+
60+
ErrorMessage::FileLocation loc1("test.c", 1, 2);
61+
ErrorMessage msg({std::move(loc1)}, "test.c", Severity::error, "error", "id", Certainty::normal);
62+
63+
ASSERT(executor.hasToLog_(msg));
64+
ASSERT(!executor.hasToLog_(msg));
65+
66+
ErrorMessage::FileLocation loc2("test.c", 1, 12);
67+
msg.callStack = {std::move(loc2)};
68+
69+
// TODO: the default message does not include the column
70+
TODO_ASSERT(executor.hasToLog_(msg));
71+
72+
msg.id = "id2";
73+
74+
// TODO: the default message does not include the id
75+
TODO_ASSERT(executor.hasToLog_(msg));
76+
}
77+
78+
void hasToLogSimple() {
79+
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
80+
const std::list<FileSettings> fileSettings;
81+
Settings settings;
82+
// this is the "simple" format
83+
settings.templateFormat = "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]";
84+
Suppressions supprs;
85+
DummyExecutor executor(files, fileSettings, settings, supprs, *this);
86+
87+
ErrorMessage::FileLocation loc1("test.c", 1, 2);
88+
ErrorMessage msg({std::move(loc1)}, "test.c", Severity::error, "error", "id", Certainty::normal);
89+
90+
ASSERT(executor.hasToLog_(msg));
91+
ASSERT(!executor.hasToLog_(msg));
92+
93+
ErrorMessage::FileLocation loc2("test.c", 1, 12);
94+
msg.callStack = {std::move(loc2)};
95+
96+
ASSERT(executor.hasToLog_(msg));
97+
98+
msg.id = "id2";
99+
100+
ASSERT(executor.hasToLog_(msg));
101+
}
102+
};
103+
104+
REGISTER_TEST(TestExecutor)

test/testrunner.vcxproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
<ClCompile Include="testcppcheck.cpp" />
6262
<ClCompile Include="testerrorlogger.cpp" />
6363
<ClCompile Include="testexceptionsafety.cpp" />
64+
<ClCompile Include="testexecutor.cpp" />
6465
<ClCompile Include="testfilelister.cpp" />
6566
<ClCompile Include="testfilesettings.cpp" />
6667
<ClCompile Include="testfunctions.cpp" />

0 commit comments

Comments
 (0)