From 229832e72e70055893cdd10bade7ac86b292d844 Mon Sep 17 00:00:00 2001 From: dummyunit Date: Thu, 22 Apr 2021 15:28:33 +0300 Subject: [PATCH] Read error locations in the correct order from XML (#3226) When ErrorMessage::callStack elements are serialized to XML they are saved in the reverse order. But when they read back from XML they are added at the end of the list. Thus the round trip via XML reverses the order of ErrorMessage::callStack. From the user point of view it looks like the usage of the --cppcheck-build-dir option sometimes (when the file wasn't reanalyzed, but that is hard to spot) results in incorrect location info for some diagnostic messages. Moreover, when the first location matches some suppression rule and the last doesn't match any (or vice versa), usage of --cppcheck-build-dir results in some diagnostic messages appearing and disappearing seemingly at random (again, depending on whether the file was reanalyzed or not). --- Makefile | 2 +- lib/errorlogger.cpp | 2 +- test/testerrorlogger.cpp | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index a30c8765f..f35d85695 100644 --- a/Makefile +++ b/Makefile @@ -632,7 +632,7 @@ test/testconstructors.o: test/testconstructors.cpp lib/astutils.h lib/check.h li test/testcppcheck.o: test/testcppcheck.cpp lib/analyzerinfo.h lib/check.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testcppcheck.o test/testcppcheck.cpp -test/testerrorlogger.o: test/testerrorlogger.cpp lib/analyzerinfo.h lib/check.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h test/testsuite.h +test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/check.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h test/testsuite.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testerrorlogger.o test/testerrorlogger.cpp test/testexceptionsafety.o: test/testexceptionsafety.cpp lib/astutils.h lib/check.h lib/checkexceptionsafety.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 5c1c65059..4aeba864c 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -216,7 +216,7 @@ ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) const char *info = strinfo ? strinfo : ""; const int line = strline ? std::atoi(strline) : 0; const int column = strcolumn ? std::atoi(strcolumn) : 0; - callStack.emplace_back(file, info, line, column); + callStack.emplace_front(file, info, line, column); } } } diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index c4d14c637..c8d1ccd26 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -22,6 +22,7 @@ #include "suppressions.h" #include "testsuite.h" +#include #include #include @@ -49,6 +50,7 @@ private: TEST_CASE(ToXmlV2); TEST_CASE(ToXmlV2Locations); TEST_CASE(ToXmlV2Encoding); + TEST_CASE(FromXmlV2); // Inconclusive results in xml reports.. TEST_CASE(InconclusiveXml); @@ -231,6 +233,38 @@ private: } } + void FromXmlV2() const { + const char xmldata[] = "\n" + "\n" + " \n" + " \n" + ""; + tinyxml2::XMLDocument doc; + doc.Parse(xmldata, sizeof(xmldata)); + ErrorMessage msg(doc.FirstChildElement()); + ASSERT_EQUALS("errorId", msg.id); + ASSERT_EQUALS(Severity::error, msg.severity); + ASSERT_EQUALS(123u, msg.cwe.id); + ASSERT_EQUALS(Certainty::inconclusive, msg.certainty); + ASSERT_EQUALS("Programming error.", msg.shortMessage()); + ASSERT_EQUALS("Verbose error", msg.verboseMessage()); + ASSERT_EQUALS(456u, msg.hash); + ASSERT_EQUALS(2u, msg.callStack.size()); + ASSERT_EQUALS("foo.cpp", msg.callStack.front().getfile()); + ASSERT_EQUALS(5, msg.callStack.front().line); + ASSERT_EQUALS(2u, msg.callStack.front().column); + ASSERT_EQUALS("bar.cpp", msg.callStack.back().getfile()); + ASSERT_EQUALS(8, msg.callStack.back().line); + ASSERT_EQUALS(1u, msg.callStack.back().column); + } + void InconclusiveXml() const { // Location std::list locs(1, fooCpp5);