fixed #13485 - symbol names in error messages were not serialized (danmar#7142)

Author: firewave

lib/errorlogger.cpp

@@ -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 += " ";
 
@@ -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");
@@ -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]);
@@ -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))

test/cli/other_test.py

@@ -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

test/testerrorlogger.cpp

@@ -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);
@@ -353,6 +353,7 @@ class TestErrorLogger : public TestFixture {
                       "1 1"
                       "17 Programming error"
                       "17 Programming error"
+                      "0 "
                       "0 ", msg_str);
 
         ErrorMessage msg2;
@@ -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");
@@ -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");
@@ -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)");
@@ -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;
@@ -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);
 
@@ -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