diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index d57ad999734..b34761ac557 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -61,10 +61,21 @@ const std::set ErrorLogger::mCriticalErrorIds{ "unknownMacro" }; +static std::size_t sdbm(const std::string& hashString) { + return std::accumulate(hashString.cbegin(), hashString.cend(), std::size_t{0}, [](std::size_t h, unsigned char c) { + return static_cast(c) + (h << 6) + (h << 16) - h; + }); +} + ErrorMessage::ErrorMessage() : severity(Severity::none), cwe(0U), certainty(Certainty::normal) {} +static bool needsFallbackHash(const std::string &id) +{ + return startsWith(id, "ctu") || id == "unusedFunction" || id == "staticFunction"; +} + // TODO: id and msg are swapped compared to other calls ErrorMessage::ErrorMessage(std::list callStack, std::string file1, Severity severity, const std::string &msg, std::string id, Certainty certainty) : callStack(std::move(callStack)), // locations for this error message @@ -76,6 +87,9 @@ ErrorMessage::ErrorMessage(std::list callStack, std::string file1, { // set the summary and verbose messages setmsg(msg); + + if (hash == 0 && needsFallbackHash(this->id)) + calculateWarningHashFromLocations(); } @@ -90,6 +104,9 @@ ErrorMessage::ErrorMessage(std::list callStack, std::string file1, { // set the summary and verbose messages setmsg(msg); + + if (hash == 0 && needsFallbackHash(this->id)) + calculateWarningHashFromLocations(); } ErrorMessage::ErrorMessage(const std::list& callstack, const TokenList* list, Severity severity, std::string id, const std::string& msg, Certainty certainty) @@ -300,9 +317,25 @@ void ErrorMessage::calculateWarningHash(const std::list& callstack // hash algorithm: sdbm // any hash algorithm can be used but it has to be the same hash on different platforms and compilers - hash = std::accumulate(hashString.cbegin(), hashString.cend(), std::size_t{0}, [](std::size_t h, unsigned char c) { - return static_cast(c) + (h << 6) + (h << 16) - h; - }); + hash = sdbm(hashString); +} + +void ErrorMessage::calculateWarningHashFromLocations() +{ + // No token information is available for this warning (e.g. whole-program/CTU + // checks, unusedFunction, staticFunction) so calculateWarningHash() can't be + // used. Instead hash the id, message and all filenames/notes in the callstack. + std::string hashString = id + '\n' + mShortMessage; + for (const FileLocation &loc : callStack) { + std::string fileName = loc.getfile(false); + if (Path::isAbsolute(fileName)) + fileName = fileName.substr(fileName.rfind('/') + 1); + hashString += '\n' + fileName + '\n' + loc.getinfo(); + } + + // hash algorithm: sdbm + // any hash algorithm can be used but it has to be the same hash on different platforms and compilers + hash = sdbm(hashString); } static void serializeString(std::string &oss, const std::string & str) diff --git a/lib/errorlogger.h b/lib/errorlogger.h index b28fdba244e..20d1669cc7b 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -211,6 +211,13 @@ class CPPCHECKLIB ErrorMessage { void calculateWarningHash(const std::list& callstack); + /** + * Fallback hash calculation for warnings that have no token information + * (e.g. whole-program/CTU checks, unusedFunction, staticFunction). Hashes + * the id, message and all filenames/notes in the callstack instead. + */ + void calculateWarningHashFromLocations(); + /** Short message */ std::string mShortMessage; diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index c849d3a0c61..8245523f799 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -49,6 +49,7 @@ class TestErrorLogger : public TestFixture { TEST_CASE(FileLocationSetFile2); TEST_CASE(ErrorMessageConstruct); TEST_CASE(ErrorMessageConstructLocations); + TEST_CASE(ErrorMessageHashFallback); TEST_CASE(ErrorMessageVerbose); TEST_CASE(ErrorMessageVerboseLocations); TEST_CASE(ErrorMessageVerboseSymbol); @@ -267,6 +268,77 @@ class TestErrorLogger : public TestFixture { ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(true, templateFormat, "")); } + // unusedFunction/staticFunction/ctu* warnings carry no token information, so + // ErrorMessage::calculateWarningHash() (which needs tokens) can't compute a + // hash for them. calculateWarningHashFromLocations() is the fallback used + // instead - it hashes the id, message and all location filenames/notes. + void ErrorMessageHashFallback() const { + // ids that don't need a fallback hash still get none + { + std::list locs(1, fooCpp5); + ErrorMessage msg(std::move(locs), "", Severity::style, "Some warning.", "someOtherId", Certainty::normal); + ASSERT_EQUALS(0, msg.hash); + } + + // unusedFunction, staticFunction and any ctu* id get a non-zero fallback hash + for (const std::string& id : { std::string("unusedFunction"), std::string("staticFunction"), std::string("ctuOneDefinitionRuleViolation") }) { + std::list locs(1, fooCpp5); + ErrorMessage msg(std::move(locs), "", Severity::style, "Some warning.", id, Certainty::normal); + ASSERT(msg.hash != 0); + } + + // same id/message/locations => same hash + { + std::list locs1(1, fooCpp5); + ErrorMessage msg1(std::move(locs1), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal); + + std::list locs2(1, fooCpp5); + ErrorMessage msg2(std::move(locs2), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal); + + ASSERT_EQUALS(msg1.hash, msg2.hash); + } + + // different message => different hash + { + std::list locs1(1, fooCpp5); + ErrorMessage msg1(std::move(locs1), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal); + + std::list locs2(1, fooCpp5); + ErrorMessage msg2(std::move(locs2), "", Severity::style, "Some other warning.", "unusedFunction", Certainty::normal); + + ASSERT(msg1.hash != msg2.hash); + } + + // different location filename => different hash + { + std::list locs1(1, fooCpp5); + ErrorMessage msg1(std::move(locs1), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal); + + std::list locs2(1, barCpp8); + ErrorMessage msg2(std::move(locs2), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal); + + ASSERT(msg1.hash != msg2.hash); + } + + // different location note (info) => different hash + { + std::list locs1(1, barCpp8); + ErrorMessage msg1(std::move(locs1), "", Severity::error, "Some warning.", "ctuOneDefinitionRuleViolation", Certainty::normal); + + std::list locs2(1, barCpp8_i); + ErrorMessage msg2(std::move(locs2), "", Severity::error, "Some warning.", "ctuOneDefinitionRuleViolation", Certainty::normal); + + ASSERT(msg1.hash != msg2.hash); + } + + // hash shows up in the XML output + { + std::list locs(1, fooCpp5); + ErrorMessage msg(std::move(locs), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal); + ASSERT(msg.toXML().find(" hash=\"") != std::string::npos); + } + } + void ErrorMessageVerbose() const { std::list locs(1, fooCpp5); ErrorMessage msg(std::move(locs), "", Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal);