Skip to content

Commit

Permalink
fixed #13485 - symbol names in error messages were not serialized (#7142
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@firewave
firewave authored Jan 2, 2025
1 parent 7db4566 commit a3cb101
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 8 deletions.
8 changes: 5 additions & 3 deletions lib/errorlogger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ std::string ErrorMessage::serialize() const

serializeString(oss, saneShortMessage);
serializeString(oss, saneVerboseMessage);
serializeString(oss, mSymbolNames);
oss += std::to_string(callStack.size());
oss += " ";

Expand Down Expand Up @@ -311,9 +312,9 @@ void ErrorMessage::deserialize(const std::string &data)
callStack.clear();

std::istringstream iss(data);
std::array<std::string, 9> results;
std::array<std::string, 10> results;
std::size_t elem = 0;
while (iss.good() && elem < 9) {
while (iss.good() && elem < 10) {
unsigned int len = 0;
if (!(iss >> len))
throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid length");
Expand All @@ -339,7 +340,7 @@ void ErrorMessage::deserialize(const std::string &data)
if (!iss.good())
throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - premature end of data");

if (elem != 9)
if (elem != 10)
throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - insufficient elements");

id = std::move(results[0]);
Expand All @@ -362,6 +363,7 @@ void ErrorMessage::deserialize(const std::string &data)
certainty = Certainty::inconclusive;
mShortMessage = std::move(results[7]);
mVerboseMessage = std::move(results[8]);
mSymbolNames = std::move(results[9]);

unsigned int stackSize = 0;
if (!(iss >> stackSize))
Expand Down
48 changes: 47 additions & 1 deletion test/cli/other_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2672,4 +2672,50 @@ def test_duplicate_suppressions_mixed(tmp_path):
assert stdout.splitlines() == [
"cppcheck: error: suppression 'uninitvar' already exists"
]
assert stderr == ''
assert stderr == ''


@pytest.mark.xfail(strict=True)
def test_xml_builddir(tmp_path): # #13391 / #13485
build_dir = tmp_path / 'b1'
os.mkdir(build_dir)

test_file = tmp_path / 'test.cpp'
with open(test_file, 'wt') as f:
f.write("""
void f(const void* p)
{
if(p) {}
(void)*p; // REMARK: boom
}
""")

args = [
'-q',
'--enable=style',
'--cppcheck-build-dir={}'.format(build_dir),
'--xml',
str(test_file)
]
exitcode_1, stdout_1, stderr_1 = cppcheck(args)
assert exitcode_1 == 0, stdout_1
assert stdout_1 == ''
# TODO: handle version
assert (stderr_1 ==
'''<?xml version="1.0" encoding="UTF-8"?>
<results version="2">
<cppcheck version="2.17 dev"/>
<errors>
<error id="nullPointerRedundantCheck" severity="warning" msg="Either the condition &apos;p&apos; is redundant or there is possible null pointer dereference: p." verbose="Either the condition &apos;p&apos; is redundant or there is possible null pointer dereference: p." cwe="476" file0="{}" remark="boom">
<location file="{}" line="5" column="12" info="Null pointer dereference"/>
<location file="{}" line="4" column="8" info="Assuming that condition &apos;p&apos; is not redundant"/>
<symbol>p</symbol>
</error>
</errors>
</results>
'''.format(test_file, test_file, test_file))

exitcode_2, stdout_2, stderr_2 = cppcheck(args)
assert exitcode_1 == exitcode_2, stdout_2
assert stdout_1 == stdout_2
assert stderr_1 == stderr_2
26 changes: 22 additions & 4 deletions test/testerrorlogger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class TestErrorLogger : public TestFixture {
TEST_CASE(DeserializeInvalidInput);
TEST_CASE(SerializeSanitize);
TEST_CASE(SerializeFileLocation);
TEST_CASE(SerializeAndDeserializeRemark);
TEST_CASE(SerializeAndDeserialize);

TEST_CASE(substituteTemplateFormatStatic);
TEST_CASE(substituteTemplateLocationStatic);
Expand Down Expand Up @@ -353,6 +353,7 @@ class TestErrorLogger : public TestFixture {
"1 1"
"17 Programming error"
"17 Programming error"
"0 "
"0 ", msg_str);

ErrorMessage msg2;
Expand Down Expand Up @@ -397,6 +398,7 @@ class TestErrorLogger : public TestFixture {
"8 test.cpp"
"17 Programming error"
"17 Programming error"
"0 "
"0 ";
ErrorMessage msg;
ASSERT_THROW_INTERNAL_EQUALS(msg.deserialize(str), INTERNAL, "Internal Error: Deserialization of error message failed - invalid CWE ID - not an integer");
Expand All @@ -412,6 +414,7 @@ class TestErrorLogger : public TestFixture {
"8 test.cpp"
"17 Programming error"
"17 Programming error"
"0 "
"0 ";
ErrorMessage msg;
ASSERT_THROW_INTERNAL_EQUALS(msg.deserialize(str), INTERNAL, "Internal Error: Deserialization of error message failed - invalid hash - not an integer");
Expand All @@ -427,6 +430,7 @@ class TestErrorLogger : public TestFixture {
"8 test.cpp"
"17 Programming error"
"17 Programming error"
"0 "
"0 ";
ErrorMessage msg;
ASSERT_THROW_INTERNAL_EQUALS(msg.deserialize(str), INTERNAL, "Internal Error: Deserialization of error message failed - invalid CWE ID - out of range (limits)");
Expand All @@ -448,6 +452,7 @@ class TestErrorLogger : public TestFixture {
"1 0"
"33 Illegal character in \"foo\\001bar\""
"33 Illegal character in \"foo\\001bar\""
"0 "
"0 ", msg_str);

ErrorMessage msg2;
Expand Down Expand Up @@ -475,6 +480,7 @@ class TestErrorLogger : public TestFixture {
"1 1"
"17 Programming error"
"17 Programming error"
"0 "
"1 "
"27 654\t33\t[]:;,()\t:/,;\tabcd:/,", msg_str);

Expand All @@ -487,12 +493,24 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS("abcd:/,", msg2.callStack.front().getinfo());
}

void SerializeAndDeserializeRemark() const {
ErrorMessage msg({}, emptyString, Severity::warning, emptyString, "id", Certainty::normal);
void SerializeAndDeserialize() const {
ErrorMessage msg({}, emptyString, Severity::warning, "$symbol:var\nmessage $symbol", "id", Certainty::normal);
msg.remark = "some remark";

ErrorMessage msg2;
ASSERT_NO_THROW(msg2.deserialize(msg.serialize()));
ASSERT_EQUALS("some remark", msg2.remark);

ASSERT_EQUALS(msg.callStack.size(), msg2.callStack.size());
ASSERT_EQUALS(msg.file0, msg2.file0);
ASSERT_EQUALS_ENUM(msg.severity, msg2.severity);
ASSERT_EQUALS(msg.shortMessage(), msg2.shortMessage());
ASSERT_EQUALS(msg.verboseMessage(), msg2.verboseMessage());
ASSERT_EQUALS(msg.id, msg2.id);
ASSERT_EQUALS_ENUM(msg.certainty, msg2.certainty);
ASSERT_EQUALS(msg.cwe.id, msg2.cwe.id);
ASSERT_EQUALS(msg.hash, msg2.hash);
ASSERT_EQUALS(msg.remark, msg2.remark);
ASSERT_EQUALS(msg.symbolNames(), msg2.symbolNames());
}

void substituteTemplateFormatStatic() const
Expand Down

0 comments on commit a3cb101

Please sign in to comment.