From f36c7c1245b06b8df6bb532277430c1337b52506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 1 Jun 2024 17:21:50 +0200 Subject: [PATCH] fix1 --- lib/cppcheck.cpp | 22 +++++++++- lib/cppcheck.h | 3 ++ lib/errorlogger.cpp | 30 +++++++------- lib/errorlogger.h | 3 ++ lib/preprocessor.cpp | 84 ++++++++++++++++++++++++++++++++++----- lib/preprocessor.h | 22 ++++++++++ test/helpers.cpp | 12 ++++++ test/helpers.h | 4 ++ test/testerrorlogger.cpp | 29 +++++++++++++- test/testpreprocessor.cpp | 31 +++++++++++++++ 10 files changed, 212 insertions(+), 28 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 7450f8ab68b..fa5e6b437d0 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -706,6 +706,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string } // Parse comments and then remove them + mRemarkComments = preprocessor.getRemarkComments(tokens1); preprocessor.inlineSuppressions(tokens1, mSettings.supprs.nomsg); if (mSettings.dump || !mSettings.addons.empty()) { std::ostringstream oss; @@ -1613,7 +1614,26 @@ void CppCheck::reportErr(const ErrorMessage &msg) mExitCode = 1; } - mErrorLogger.reportErr(msg); + std::string remark; + if (!msg.callStack.empty()) { + for (const auto& r: mRemarkComments) { + if (r.file != msg.callStack.back().getfile(false)) + continue; + if (r.lineNumber != msg.callStack.back().line) + continue; + remark = r.str; + break; + } + } + + if (!remark.empty()) { + ErrorMessage msg2(msg); + msg2.remark = remark; + mErrorLogger.reportErr(msg2); + } else { + mErrorLogger.reportErr(msg); + } + // check if plistOutput should be populated and the current output file is open and the error is not suppressed if (!mSettings.plistOutput.empty() && mPlistFile.is_open() && !mSettings.supprs.nomsg.isSuppressed(errorMessage)) { // add error to plist output file diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 2ca1f0238e9..6e14683e89f 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -45,6 +45,7 @@ struct FileSettings; class CheckUnusedFunctions; class Tokenizer; class FileWithDetails; +class RemarkComment; namespace simplecpp { class TokenList; } @@ -254,6 +255,8 @@ class CPPCHECKLIB CppCheck : ErrorLogger { std::ofstream mPlistFile; std::unique_ptr mUnusedFunctionsCheck; + + std::vector mRemarkComments; }; /// @} diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index e04a973f5d6..83946820517 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -274,11 +274,9 @@ std::string ErrorMessage::serialize() const serializeString(oss, severityToString(severity)); serializeString(oss, std::to_string(cwe.id)); serializeString(oss, std::to_string(hash)); + serializeString(oss, fixInvalidChars(remark)); serializeString(oss, file0); - if (certainty == Certainty::inconclusive) { - const std::string text("inconclusive"); - serializeString(oss, text); - } + serializeString(oss, (certainty == Certainty::inconclusive) ? "1" : "0"); const std::string saneShortMessage = fixInvalidChars(mShortMessage); const std::string saneVerboseMessage = fixInvalidChars(mVerboseMessage); @@ -312,9 +310,9 @@ void ErrorMessage::deserialize(const std::string &data) callStack.clear(); std::istringstream iss(data); - std::array results; + std::array results; std::size_t elem = 0; - while (iss.good() && elem < 7) { + while (iss.good() && elem < 9) { unsigned int len = 0; if (!(iss >> len)) throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid length"); @@ -332,11 +330,6 @@ 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 (temp == "inconclusive") { - certainty = Certainty::inconclusive; - continue; - } } results[elem++] = std::move(temp); @@ -345,7 +338,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 != 7) + if (elem != 9) throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - insufficient elements"); id = std::move(results[0]); @@ -362,9 +355,12 @@ void ErrorMessage::deserialize(const std::string &data) if (!strToInt(results[3], hash, &err)) throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid hash - " + err); } - file0 = std::move(results[4]); - mShortMessage = std::move(results[5]); - mVerboseMessage = std::move(results[6]); + remark = std::move(results[4]); + file0 = std::move(results[5]); + if (results[6] == "1") + certainty = Certainty::inconclusive; + mShortMessage = std::move(results[7]); + mVerboseMessage = std::move(results[8]); unsigned int stackSize = 0; if (!(iss >> stackSize)) @@ -496,6 +492,9 @@ std::string ErrorMessage::toXML() const if (!file0.empty()) printer.PushAttribute("file0", file0.c_str()); + if (!remark.empty()) + printer.PushAttribute("remark", fixInvalidChars(remark).c_str()); + for (std::list::const_reverse_iterator it = callStack.crbegin(); it != callStack.crend(); ++it) { printer.OpenElement("location", false); printer.PushAttribute("file", it->getfile().c_str()); @@ -641,6 +640,7 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm findAndReplace(result, "{severity}", severityToString(severity)); findAndReplace(result, "{cwe}", std::to_string(cwe.id)); findAndReplace(result, "{message}", verbose ? mVerboseMessage : mShortMessage); + findAndReplace(result, "{remark}", remark); if (!callStack.empty()) { if (result.find("{callstack}") != std::string::npos) findAndReplace(result, "{callstack}", ErrorLogger::callStackToString(callStack)); diff --git a/lib/errorlogger.h b/lib/errorlogger.h index c396781a31f..e936fae748f 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -171,6 +171,9 @@ class CPPCHECKLIB ErrorMessage { CWE cwe; Certainty certainty; + /** remark from REMARK comment */ + std::string remark; + /** Warning hash */ std::size_t hash; diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index bbd0f67ea57..738862e42eb 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -153,6 +153,19 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std: return true; } +static std::string getRelativeFilename(const simplecpp::Token* tok, const Settings &settings) { + std::string relativeFilename(tok->location.file()); + if (settings.relativePaths) { + for (const std::string & basePath : settings.basePaths) { + const std::string bp = basePath + "/"; + if (relativeFilename.compare(0,bp.size(),bp)==0) { + relativeFilename = relativeFilename.substr(bp.size()); + } + } + } + return Path::simplifyPath(relativeFilename); +} + static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Settings &settings, SuppressionList &suppressions, std::list &bad) { std::list inlineSuppressionsBlockBegin; @@ -193,16 +206,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett continue; // Relative filename - std::string relativeFilename(tok->location.file()); - if (settings.relativePaths) { - for (const std::string & basePath : settings.basePaths) { - const std::string bp = basePath + "/"; - if (relativeFilename.compare(0,bp.size(),bp)==0) { - relativeFilename = relativeFilename.substr(bp.size()); - } - } - } - relativeFilename = Path::simplifyPath(relativeFilename); + const std::string relativeFilename = getRelativeFilename(tok, settings); // Macro name std::string macroName; @@ -295,6 +299,17 @@ void Preprocessor::inlineSuppressions(const simplecpp::TokenList &tokens, Suppre } } +std::vector Preprocessor::getRemarkComments(const simplecpp::TokenList &tokens) const +{ + std::vector ret; + addRemarkComments(tokens, ret); + for (std::map::const_iterator it = mTokenLists.cbegin(); it != mTokenLists.cend(); ++it) { + if (it->second) + addRemarkComments(*it->second, ret); + } + return ret; +} + std::list Preprocessor::createDirectives(const simplecpp::TokenList &tokens) const { // directive list.. @@ -998,3 +1013,52 @@ void Preprocessor::simplifyPragmaAsmPrivate(simplecpp::TokenList *tokenList) tokenList->deleteToken(tok4->next); } } + + +void Preprocessor::addRemarkComments(const simplecpp::TokenList &tokens, std::vector &remarkComments) const +{ + for (const simplecpp::Token *tok = tokens.cfront(); tok; tok = tok->next) { + if (!tok->comment) + continue; + + const std::string& comment = tok->str(); + + // is it a remark comment? + const std::string::size_type pos1 = comment.find_first_not_of("/* \t"); + if (pos1 == std::string::npos) + continue; + const std::string::size_type pos2 = comment.find_first_of(": \t", pos1); + if (pos2 != pos1 + 6 || comment.compare(pos1, 6, "REMARK") != 0) + continue; + const std::string::size_type pos3 = comment.find_first_not_of(": \t", pos2); + if (pos3 == std::string::npos) + continue; + if (comment.compare(0,2,"/*") == 0 && pos3 + 2 >= tok->str().size()) + continue; + + const std::string::size_type pos4 = (comment.compare(0,2,"/*") == 0) ? comment.size()-2 : comment.size(); + const std::string remarkText = comment.substr(pos3, pos4-pos3); + + // Get remarked token + const simplecpp::Token* remarkedToken = nullptr; + for (const simplecpp::Token* after = tok->next; after; after = after->next) { + if (after->comment) + continue; + remarkedToken = after; + break; + } + for (const simplecpp::Token* prev = tok->previous; prev; prev = prev->previous) { + if (prev->comment) + continue; + if (sameline(prev, tok)) + remarkedToken = prev; + break; + } + + // Relative filename + const std::string relativeFilename = getRelativeFilename(remarkedToken, mSettings); + + // Add the suppressions. + remarkComments.emplace_back(relativeFilename, remarkedToken->location.line, remarkText); + } +} diff --git a/lib/preprocessor.h b/lib/preprocessor.h index 08c856f17fa..c8bdcbb90ba 100644 --- a/lib/preprocessor.h +++ b/lib/preprocessor.h @@ -60,6 +60,24 @@ struct CPPCHECKLIB Directive { Directive(std::string _file, const int _linenr, const std::string &_str); }; +class CPPCHECKLIB RemarkComment { +public: + RemarkComment(const std::string &file, const unsigned int lineNumber, const std::string &str) + : file(file) + , lineNumber(lineNumber) + , str(str) + {} + + /** name of file */ + std::string file; + + /** line number for the code that the remark comment is about */ + unsigned int lineNumber; + + /** remark text */ + std::string str; +}; + /// @addtogroup Core /// @{ @@ -96,6 +114,8 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor { std::set getConfigs(const simplecpp::TokenList &tokens) const; + std::vector getRemarkComments(const simplecpp::TokenList &tokens) const; + void handleErrors(const simplecpp::OutputList &outputList, bool throwError); bool loadFiles(const simplecpp::TokenList &rawtokens, std::vector &files); @@ -138,6 +158,8 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor { static bool hasErrors(const simplecpp::OutputList &outputList); + void addRemarkComments(const simplecpp::TokenList &tokens, std::vector &remarkComments) const; + const Settings& mSettings; ErrorLogger &mErrorLogger; diff --git a/test/helpers.cpp b/test/helpers.cpp index 4e4effd76ac..895d924f7cb 100644 --- a/test/helpers.cpp +++ b/test/helpers.cpp @@ -186,6 +186,18 @@ void PreprocessorHelper::preprocess(const char code[], std::vector tokenizer.setDirectives(std::move(directives)); } +std::vector PreprocessorHelper::getRemarkComments(const char code[], ErrorLogger& errorLogger) +{ + std::vector files{"test.cpp"}; + std::istringstream istr(code); + const simplecpp::TokenList tokens1(istr, files, files[0]); + + const Settings settings; + + const Preprocessor preprocessor(settings, errorLogger); + return preprocessor.getRemarkComments(tokens1); +} + bool LibraryHelper::loadxmldata(Library &lib, const char xmldata[], std::size_t len) { tinyxml2::XMLDocument doc; diff --git a/test/helpers.h b/test/helpers.h index 30bc9003221..16ea8817636 100644 --- a/test/helpers.h +++ b/test/helpers.h @@ -20,6 +20,7 @@ #define helpersH #include "config.h" +#include "preprocessor.h" #include "settings.h" #include "standards.h" #include "tokenize.h" @@ -170,6 +171,9 @@ class PreprocessorHelper static void preprocess(const char code[], std::vector &files, Tokenizer& tokenizer, ErrorLogger& errorlogger); static void preprocess(const char code[], std::vector &files, Tokenizer& tokenizer, ErrorLogger& errorlogger, const simplecpp::DUI& dui); + /** get remark comments */ + static std::vector getRemarkComments(const char code[], ErrorLogger& errorLogger); + private: static std::map getcode(const Settings& settings, ErrorLogger& errorlogger, const char code[], std::set cfgs, const std::string &filename = "file.c", SuppressionList *inlineSuppression = nullptr); }; diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index 32349d23e8c..79e1d7575d2 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -50,6 +50,7 @@ class TestErrorLogger : public TestFixture { TEST_CASE(CustomFormat2); TEST_CASE(CustomFormatLocations); TEST_CASE(ToXmlV2); + TEST_CASE(ToXmlV2RemarkComment); TEST_CASE(ToXmlV2Locations); TEST_CASE(ToXmlV2Encoding); TEST_CASE(FromXmlV2); @@ -62,6 +63,7 @@ class TestErrorLogger : public TestFixture { TEST_CASE(DeserializeInvalidInput); TEST_CASE(SerializeSanitize); TEST_CASE(SerializeFileLocation); + TEST_CASE(SerializeAndDeserializeRemark); TEST_CASE(substituteTemplateFormatStatic); TEST_CASE(substituteTemplateLocationStatic); @@ -237,6 +239,12 @@ class TestErrorLogger : public TestFixture { ASSERT_EQUALS(message, msg.toXML()); } + void ToXmlV2RemarkComment() const { + ErrorMessage msg({}, emptyString, Severity::warning, "", "id", Certainty::normal); + msg.remark = "remark"; + ASSERT_EQUALS(" ", msg.toXML()); + } + void ToXmlV2Locations() const { std::list locs = { fooCpp5, barCpp8_i }; ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); @@ -329,8 +337,9 @@ class TestErrorLogger : public TestFixture { "5 error" "1 0" "1 0" + "0 " "8 test.cpp" - "12 inconclusive" + "1 1" "17 Programming error" "17 Programming error" "0 ", msg_str); @@ -373,6 +382,7 @@ class TestErrorLogger : public TestFixture { "5 error" "7 invalid" // cwe "1 0" + "0 " "8 test.cpp" "17 Programming error" "17 Programming error" @@ -386,6 +396,8 @@ class TestErrorLogger : public TestFixture { "5 error" "1 0" "7 invalid" // hash + "1 0" + "0 " "8 test.cpp" "17 Programming error" "17 Programming error" @@ -399,6 +411,8 @@ class TestErrorLogger : public TestFixture { "5 error" "5 65536" // max +1 "1 0" + "1 0" + "0 " "8 test.cpp" "17 Programming error" "17 Programming error" @@ -418,7 +432,9 @@ class TestErrorLogger : public TestFixture { "5 error" "1 0" "1 0" + "0 " "3 1.c" + "1 0" "33 Illegal character in \"foo\\001bar\"" "33 Illegal character in \"foo\\001bar\"" "0 ", msg_str); @@ -444,7 +460,8 @@ class TestErrorLogger : public TestFixture { "1 0" "1 0" "0 " - "12 inconclusive" + "0 " + "1 1" "17 Programming error" "17 Programming error" "1 " @@ -459,6 +476,14 @@ class TestErrorLogger : public TestFixture { ASSERT_EQUALS("abcd:/,", msg2.callStack.front().getinfo()); } + void SerializeAndDeserializeRemark() const { + ErrorMessage msg({}, emptyString, Severity::warning, emptyString, "id", Certainty::normal); + msg.remark = "some remark"; + ErrorMessage msg2; + ASSERT_NO_THROW(msg2.deserialize(msg.serialize())); + ASSERT_EQUALS("some remark", msg2.remark); + } + void substituteTemplateFormatStatic() const { { diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index 77bdd3c83ac..f3d0f45a56e 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -199,6 +199,11 @@ class TestPreprocessor : public TestFixture { // inline suppression, missingInclude/missingIncludeSystem TEST_CASE(inline_suppressions); + // remark comment + TEST_CASE(remarkComment1); + TEST_CASE(remarkComment2); + TEST_CASE(remarkComment3); + // Using -D to predefine symbols TEST_CASE(predefine1); TEST_CASE(predefine2); @@ -1903,6 +1908,32 @@ class TestPreprocessor : public TestFixture { ignore_errout(); // we are not interested in the output } + void remarkComment1() { + const char code[] = "// REMARK: assignment with 1\n" + "x=1;\n"; + const auto remarkComments = PreprocessorHelper::getRemarkComments(code, *this); + ASSERT_EQUALS(1, remarkComments.size()); + ASSERT_EQUALS(2, remarkComments[0].lineNumber); + ASSERT_EQUALS("assignment with 1", remarkComments[0].str); + } + + void remarkComment2() { + const char code[] = "x=1; ///REMARK assignment with 1\n"; + const auto remarkComments = PreprocessorHelper::getRemarkComments(code, *this); + ASSERT_EQUALS(1, remarkComments.size()); + ASSERT_EQUALS(1, remarkComments[0].lineNumber); + ASSERT_EQUALS("assignment with 1", remarkComments[0].str); + } + + void remarkComment3() { + const char code[] = "/** REMARK: assignment with 1 */\n" + "x=1;\n"; + const auto remarkComments = PreprocessorHelper::getRemarkComments(code, *this); + ASSERT_EQUALS(1, remarkComments.size()); + ASSERT_EQUALS(2, remarkComments[0].lineNumber); + ASSERT_EQUALS("assignment with 1 ", remarkComments[0].str); + } + void predefine1() { const std::string src("#if defined X || Y\n" "Fred & Wilma\n"