Fix #15013 (warning hash: calculate hash for CTU warnings) (#8829)

Author: danmar

lib/errorlogger.cpp

@@ -61,10 +61,21 @@ const std::set<std::string> 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<std::size_t>(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<FileLocation> 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<FileLocation> 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<FileLocation> callStack, std::string file1,
 {
     // set the summary and verbose messages
     setmsg(msg);
+
+    if (hash == 0 && needsFallbackHash(this->id))
+        calculateWarningHashFromLocations();
 }
 
 ErrorMessage::ErrorMessage(const std::list<const Token*>& 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<const Token*>& 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<std::size_t>(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)

lib/errorlogger.h

@@ -211,6 +211,13 @@ class CPPCHECKLIB ErrorMessage {
 
     void calculateWarningHash(const std::list<const Token*>& 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;
 

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<ErrorMessage::FileLocation> 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<ErrorMessage::FileLocation> 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<ErrorMessage::FileLocation> locs1(1, fooCpp5);
+            ErrorMessage msg1(std::move(locs1), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal);
+
+            std::list<ErrorMessage::FileLocation> 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<ErrorMessage::FileLocation> locs1(1, fooCpp5);
+            ErrorMessage msg1(std::move(locs1), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal);
+
+            std::list<ErrorMessage::FileLocation> 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<ErrorMessage::FileLocation> locs1(1, fooCpp5);
+            ErrorMessage msg1(std::move(locs1), "", Severity::style, "Some warning.", "unusedFunction", Certainty::normal);
+
+            std::list<ErrorMessage::FileLocation> 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<ErrorMessage::FileLocation> locs1(1, barCpp8);
+            ErrorMessage msg1(std::move(locs1), "", Severity::error, "Some warning.", "ctuOneDefinitionRuleViolation", Certainty::normal);
+
+            std::list<ErrorMessage::FileLocation> 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<ErrorMessage::FileLocation> 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<ErrorMessage::FileLocation> locs(1, fooCpp5);
         ErrorMessage msg(std::move(locs), "", Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal);