Fixed #3537 (Allow inline suppression comments for macros) (#5559)

This commit is contained in:
Daniel Marjamäki 2023-10-16 19:43:15 +02:00 committed by GitHub
parent ad4e688ff8
commit dd76504f82
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 152 additions and 30 deletions

View File

@ -33,7 +33,7 @@ Executor::Executor(const std::map<std::string, std::size_t> &files, const Settin
bool Executor::hasToLog(const ErrorMessage &msg) bool Executor::hasToLog(const ErrorMessage &msg)
{ {
if (!mSuppressions.isSuppressed(msg)) if (!mSuppressions.isSuppressed(msg, {}))
{ {
std::string errmsg = msg.toString(mSettings.verbose); std::string errmsg = msg.toString(mSettings.verbose);

View File

@ -394,7 +394,7 @@ void ProcessExecutor::reportInternalChildErr(const std::string &childname, const
"cppcheckError", "cppcheckError",
Certainty::normal); Certainty::normal);
if (!mSuppressions.isSuppressed(errmsg)) if (!mSuppressions.isSuppressed(errmsg, {}))
mErrorLogger.reportErr(errmsg); mErrorLogger.reportErr(errmsg);
} }

View File

@ -858,6 +858,13 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
} }
hasValidConfig = true; hasValidConfig = true;
// locations macros
mLocationMacros.clear();
for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (!tok->getMacroName().empty())
mLocationMacros[Location(files[tok->fileIndex()], tok->linenr())].emplace(tok->getMacroName());
}
// If only errors are printed, print filename after the check // If only errors are printed, print filename after the check
if (!mSettings.quiet && (!mCurrentConfig.empty() || checkCount > 1)) { if (!mSettings.quiet && (!mCurrentConfig.empty() || checkCount > 1)) {
std::string fixedpath = Path::simplifyPath(filename); std::string fixedpath = Path::simplifyPath(filename);
@ -1556,8 +1563,17 @@ void CppCheck::reportErr(const ErrorMessage &msg)
if (!mSettings.buildDir.empty()) if (!mSettings.buildDir.empty())
mAnalyzerInformation.reportErr(msg); mAnalyzerInformation.reportErr(msg);
std::set<std::string> macroNames;
if (!msg.callStack.empty()) {
const std::string &file = msg.callStack.back().getfile(false);
int lineNumber = msg.callStack.back().line;
const auto it = mLocationMacros.find(Location(file, lineNumber));
if (it != mLocationMacros.cend())
macroNames = it->second;
}
// TODO: only convert if necessary // TODO: only convert if necessary
const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg); const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg, macroNames);
if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) { if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) {
return; return;

View File

@ -34,6 +34,7 @@
#include <functional> #include <functional>
#include <list> #include <list>
#include <map> #include <map>
#include <set>
#include <string> #include <string>
#include <vector> #include <vector>
@ -223,6 +224,9 @@ private:
/** @brief Current preprocessor configuration */ /** @brief Current preprocessor configuration */
std::string mCurrentConfig; std::string mCurrentConfig;
using Location = std::pair<std::string, int>;
std::map<Location, std::set<std::string>> mLocationMacros; // What macros are used on a location?
unsigned int mExitCode{}; unsigned int mExitCode{};
bool mUseGlobalSuppressions; bool mUseGlobalSuppressions;

View File

@ -119,16 +119,17 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
const std::string suppressTypeString = const std::string suppressTypeString =
comment.substr(pos1 + cppchecksuppress.size() + 1, argumentLength); comment.substr(pos1 + cppchecksuppress.size() + 1, argumentLength);
if ("file" == suppressTypeString) { if ("file" == suppressTypeString)
errorType = Suppressions::Type::file; errorType = Suppressions::Type::file;
} else if ("begin" == suppressTypeString) { else if ("begin" == suppressTypeString)
errorType = Suppressions::Type::blockBegin; errorType = Suppressions::Type::blockBegin;
} else if ("end" == suppressTypeString) { else if ("end" == suppressTypeString)
errorType = Suppressions::Type::blockEnd; errorType = Suppressions::Type::blockEnd;
} else { else if ("macro" == suppressTypeString)
errorType = Suppressions::Type::macro;
else
return false; return false;
} }
}
if (comment[pos2] == '[') { if (comment[pos2] == '[') {
// multi suppress format // multi suppress format
@ -217,6 +218,15 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
} }
relativeFilename = Path::simplifyPath(relativeFilename); relativeFilename = Path::simplifyPath(relativeFilename);
// Macro name
std::string macroName;
if (tok->str() == "#" && tok->next && tok->next->str() == "define") {
const simplecpp::Token *macroNameTok = tok->next->next;
if (sameline(tok, macroNameTok) && macroNameTok->name) {
macroName = macroNameTok->str();
}
}
// Add the suppressions. // Add the suppressions.
for (Suppressions::Suppression &suppr : inlineSuppressions) { for (Suppressions::Suppression &suppr : inlineSuppressions) {
suppr.fileName = relativeFilename; suppr.fileName = relativeFilename;
@ -252,7 +262,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false // NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin"); bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin");
} }
} else if (Suppressions::Type::unique == suppr.type) { } else if (Suppressions::Type::unique == suppr.type || suppr.type == Suppressions::Type::macro) {
// special handling when suppressing { warnings for backwards compatibility // special handling when suppressing { warnings for backwards compatibility
const bool thisAndNextLine = tok->previous && const bool thisAndNextLine = tok->previous &&
tok->previous->previous && tok->previous->previous &&
@ -264,6 +274,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
suppr.thisAndNextLine = thisAndNextLine; suppr.thisAndNextLine = thisAndNextLine;
suppr.lineNumber = tok->location.line; suppr.lineNumber = tok->location.line;
suppr.macroName = macroName;
suppressions.addSuppression(std::move(suppr)); suppressions.addSuppression(std::move(suppr));
} else if (Suppressions::Type::file == suppr.type) { } else if (Suppressions::Type::file == suppr.type) {
if (onlyComments) if (onlyComments)

View File

@ -35,7 +35,7 @@
#include <tinyxml2.h> #include <tinyxml2.h>
Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg) Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg, const std::set<std::string> &macroNames)
{ {
Suppressions::ErrorMessage ret; Suppressions::ErrorMessage ret;
ret.hash = msg.hash; ret.hash = msg.hash;
@ -48,6 +48,7 @@ Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::
} }
ret.certainty = msg.certainty; ret.certainty = msg.certainty;
ret.symbolNames = msg.symbolNames(); ret.symbolNames = msg.symbolNames();
ret.macroNames = macroNames;
return ret; return ret;
} }
@ -308,7 +309,8 @@ bool Suppressions::Suppression::parseComment(std::string comment, std::string *e
"cppcheck-suppress", "cppcheck-suppress",
"cppcheck-suppress-begin", "cppcheck-suppress-begin",
"cppcheck-suppress-end", "cppcheck-suppress-end",
"cppcheck-suppress-file" "cppcheck-suppress-file",
"cppcheck-suppress-macro"
}; };
std::istringstream iss(comment.substr(2)); std::istringstream iss(comment.substr(2));
@ -343,6 +345,10 @@ bool Suppressions::Suppression::isSuppressed(const Suppressions::ErrorMessage &e
return false; return false;
if (!errorId.empty() && !matchglob(errorId, errmsg.errorId)) if (!errorId.empty() && !matchglob(errorId, errmsg.errorId))
return false; return false;
if (type == Suppressions::Type::macro) {
if (errmsg.macroNames.count(macroName) == 0)
return false;
} else {
if (!fileName.empty() && !matchglob(fileName, errmsg.getFileName())) if (!fileName.empty() && !matchglob(fileName, errmsg.getFileName()))
return false; return false;
if ((Suppressions::Type::unique == type) && (lineNumber != NO_LINE) && (lineNumber != errmsg.lineNumber)) { if ((Suppressions::Type::unique == type) && (lineNumber != NO_LINE) && (lineNumber != errmsg.lineNumber)) {
@ -351,6 +357,7 @@ bool Suppressions::Suppression::isSuppressed(const Suppressions::ErrorMessage &e
} }
if ((Suppressions::Type::block == type) && ((errmsg.lineNumber < lineBegin) || (errmsg.lineNumber > lineEnd))) if ((Suppressions::Type::block == type) && ((errmsg.lineNumber < lineBegin) || (errmsg.lineNumber > lineEnd)))
return false; return false;
}
if (!symbolName.empty()) { if (!symbolName.empty()) {
for (std::string::size_type pos = 0; pos < errmsg.symbolNames.size();) { for (std::string::size_type pos = 0; pos < errmsg.symbolNames.size();) {
const std::string::size_type pos2 = errmsg.symbolNames.find('\n',pos); const std::string::size_type pos2 = errmsg.symbolNames.find('\n',pos);
@ -412,11 +419,11 @@ bool Suppressions::isSuppressed(const Suppressions::ErrorMessage &errmsg, bool g
return returnValue; return returnValue;
} }
bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg) bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg, const std::set<std::string>& macroNames)
{ {
if (mSuppressions.empty()) if (mSuppressions.empty())
return false; return false;
return isSuppressed(Suppressions::ErrorMessage::fromErrorMessage(errmsg)); return isSuppressed(Suppressions::ErrorMessage::fromErrorMessage(errmsg, macroNames));
} }
void Suppressions::dump(std::ostream & out) const void Suppressions::dump(std::ostream & out) const
@ -445,6 +452,8 @@ std::list<Suppressions::Suppression> Suppressions::getUnmatchedLocalSuppressions
for (const Suppression &s : mSuppressions) { for (const Suppression &s : mSuppressions) {
if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked)) if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
continue; continue;
if (s.type == Suppressions::Type::macro)
continue;
if (s.hash > 0) if (s.hash > 0)
continue; continue;
if (!unusedFunctionChecking && s.errorId == "unusedFunction") if (!unusedFunctionChecking && s.errorId == "unusedFunction")

View File

@ -25,6 +25,7 @@
#include <cstddef> #include <cstddef>
#include <istream> #include <istream>
#include <list> #include <list>
#include <set>
#include <string> #include <string>
#include <utility> #include <utility>
#include <vector> #include <vector>
@ -42,7 +43,7 @@ class CPPCHECKLIB Suppressions {
public: public:
enum class Type { enum class Type {
unique, file, block, blockBegin, blockEnd unique, file, block, blockBegin, blockEnd, macro
}; };
struct CPPCHECKLIB ErrorMessage { struct CPPCHECKLIB ErrorMessage {
@ -55,8 +56,9 @@ public:
int lineNumber; int lineNumber;
Certainty certainty; Certainty certainty;
std::string symbolNames; std::string symbolNames;
std::set<std::string> macroNames;
static Suppressions::ErrorMessage fromErrorMessage(const ::ErrorMessage &msg); static Suppressions::ErrorMessage fromErrorMessage(const ::ErrorMessage &msg, const std::set<std::string> &macroNames);
private: private:
std::string mFileName; std::string mFileName;
}; };
@ -74,6 +76,8 @@ public:
return fileName < other.fileName; return fileName < other.fileName;
if (symbolName != other.symbolName) if (symbolName != other.symbolName)
return symbolName < other.symbolName; return symbolName < other.symbolName;
if (macroName != other.macroName)
return macroName < other.macroName;
if (hash != other.hash) if (hash != other.hash)
return hash < other.hash; return hash < other.hash;
if (thisAndNextLine != other.thisAndNextLine) if (thisAndNextLine != other.thisAndNextLine)
@ -90,6 +94,8 @@ public:
return false; return false;
if (symbolName != other.symbolName) if (symbolName != other.symbolName)
return false; return false;
if (macroName != other.macroName)
return false;
if (hash != other.hash) if (hash != other.hash)
return false; return false;
if (type != other.type) if (type != other.type)
@ -135,6 +141,7 @@ public:
int lineEnd = NO_LINE; int lineEnd = NO_LINE;
Type type = Type::unique; Type type = Type::unique;
std::string symbolName; std::string symbolName;
std::string macroName;
std::size_t hash{}; std::size_t hash{};
bool thisAndNextLine{}; // Special case for backwards compatibility: { // cppcheck-suppress something bool thisAndNextLine{}; // Special case for backwards compatibility: { // cppcheck-suppress something
bool matched{}; bool matched{};
@ -200,7 +207,7 @@ public:
* @param errmsg error message * @param errmsg error message
* @return true if this error is suppressed. * @return true if this error is suppressed.
*/ */
bool isSuppressed(const ::ErrorMessage &errmsg); bool isSuppressed(const ::ErrorMessage &errmsg, const std::set<std::string>& macroNames);
/** /**
* @brief Create an xml dump of suppressions * @brief Create an xml dump of suppressions

View File

@ -106,6 +106,9 @@ struct TokenImpl {
// original name like size_t // original name like size_t
std::string* mOriginalName{}; std::string* mOriginalName{};
// If this token came from a macro replacement list, this is the name of that macro
std::string mMacroName;
// ValueType // ValueType
ValueType* mValueType{}; ValueType* mValueType{};
@ -461,10 +464,7 @@ public:
setFlag(fIsStandardType, b); setFlag(fIsStandardType, b);
} }
bool isExpandedMacro() const { bool isExpandedMacro() const {
return getFlag(fIsExpandedMacro); return !mImpl->mMacroName.empty();
}
void isExpandedMacro(const bool m) {
setFlag(fIsExpandedMacro, m);
} }
bool isCast() const { bool isCast() const {
return getFlag(fIsCast); return getFlag(fIsCast);
@ -763,6 +763,13 @@ public:
setFlag(fIsTemplateArg, value); setFlag(fIsTemplateArg, value);
} }
std::string getMacroName() const {
return mImpl->mMacroName;
}
void setMacroName(std::string name) {
mImpl->mMacroName = std::move(name);
}
template<size_t count> template<size_t count>
static const Token *findsimplematch(const Token * const startTok, const char (&pattern)[count]) { static const Token *findsimplematch(const Token * const startTok, const char (&pattern)[count]) {
return findsimplematch(startTok, pattern, count-1); return findsimplematch(startTok, pattern, count-1);
@ -1305,7 +1312,7 @@ private:
fIsPointerCompare = (1ULL << 2), fIsPointerCompare = (1ULL << 2),
fIsLong = (1ULL << 3), fIsLong = (1ULL << 3),
fIsStandardType = (1ULL << 4), fIsStandardType = (1ULL << 4),
fIsExpandedMacro = (1ULL << 5), //fIsExpandedMacro = (1ULL << 5),
fIsCast = (1ULL << 6), fIsCast = (1ULL << 6),
fIsAttributeConstructor = (1ULL << 7), // __attribute__((constructor)) __attribute__((constructor(priority))) fIsAttributeConstructor = (1ULL << 7), // __attribute__((constructor)) __attribute__((constructor(priority)))
fIsAttributeDestructor = (1ULL << 8), // __attribute__((destructor)) __attribute__((destructor(priority))) fIsAttributeDestructor = (1ULL << 8), // __attribute__((destructor)) __attribute__((destructor(priority)))

View File

@ -806,7 +806,7 @@ namespace {
if (pointerType) { if (pointerType) {
tok->insertToken("const"); tok->insertToken("const");
tok->next()->column(tok->column()); tok->next()->column(tok->column());
tok->next()->isExpandedMacro(tok->previous()->isExpandedMacro()); tok->next()->setMacroName(tok->previous()->getMacroName());
tok->deletePrevious(); tok->deletePrevious();
} }
} }
@ -7169,7 +7169,7 @@ void Tokenizer::simplifyVarDecl(Token * tokBegin, const Token * const tokEnd, co
endDecl = endDecl->next(); endDecl = endDecl->next();
endDecl->next()->isSplittedVarDeclEq(true); endDecl->next()->isSplittedVarDeclEq(true);
endDecl->insertToken(varName->str()); endDecl->insertToken(varName->str());
endDecl->next()->isExpandedMacro(varName->isExpandedMacro()); endDecl->next()->setMacroName(varName->getMacroName());
continue; continue;
} }
//non-VLA case //non-VLA case

View File

@ -306,6 +306,7 @@ void TokenList::insertTokens(Token *dest, const Token *src, nonneg int n)
dest->varId(src->varId()); dest->varId(src->varId());
dest->tokType(src->tokType()); dest->tokType(src->tokType());
dest->flags(src->flags()); dest->flags(src->flags());
dest->setMacroName(src->getMacroName());
src = src->next(); src = src->next();
--n; --n;
} }
@ -363,7 +364,7 @@ void TokenList::createTokens(simplecpp::TokenList&& tokenList)
mTokensFrontBack.back->fileIndex(tok->location.fileIndex); mTokensFrontBack.back->fileIndex(tok->location.fileIndex);
mTokensFrontBack.back->linenr(tok->location.line); mTokensFrontBack.back->linenr(tok->location.line);
mTokensFrontBack.back->column(tok->location.col); mTokensFrontBack.back->column(tok->location.col);
mTokensFrontBack.back->isExpandedMacro(!tok->macro.empty()); mTokensFrontBack.back->setMacroName(tok->macro);
tok = tok->next; tok = tok->next;
if (tok) if (tok)

View File

@ -530,6 +530,41 @@ Suppressing multiple ids in one comment by using []:
// cppcheck-suppress [aaaa, bbbb] // cppcheck-suppress [aaaa, bbbb]
Suppressing warnings `aaaa` on a block of code:
// cppcheck-suppress-begin aaaa
...
// cppcheck-suppress-end aaaa
Suppressing multiple ids on a block of code:
// cppcheck-suppress-begin [aaaa, bbbb]
...
// cppcheck-suppress-end [aaaa, bbbb]
Suppressing warnings `aaaa` for a whole file:
// cppcheck-suppress-file aaaa
Suppressing multiple ids for a whole file:
// cppcheck-suppress-file [aaaa, bbbb]
Suppressing warnings `aaaa` where macro is used:
// cppcheck-suppress-macro aaaa
#define MACRO ...
...
x = MACRO; // <- aaaa warnings are suppressed here
Suppressing multiple ids where macro is used:
// cppcheck-suppress-macro [aaaa, bbbb]
#define MACRO ...
...
x = MACRO; // <- aaaa and bbbb warnings are suppressed here
### Comment before code or on same line ### Comment before code or on same line
The comment can be put before the code or at the same line as the code. The comment can be put before the code or at the same line as the code.

View File

@ -551,6 +551,21 @@ Suppressing multiple ids for a whole file:
// cppcheck-suppress-file [aaaa, bbbb] // cppcheck-suppress-file [aaaa, bbbb]
Suppressing warnings `aaaa` where macro is used:
// cppcheck-suppress-macro aaaa
#define MACRO ...
...
x = MACRO; // <- aaaa warnings are suppressed here
Suppressing multiple ids where macro is used:
// cppcheck-suppress-macro [aaaa, bbbb]
#define MACRO ...
...
x = MACRO; // <- aaaa and bbbb warnings are suppressed here
### Comment before code or on same line ### Comment before code or on same line
The comment can be put before the code or at the same line as the code. The comment can be put before the code or at the same line as the code.

View File

@ -24,3 +24,7 @@ Other:
- "--project" can also no longer be used in conjunction with additional source files. - "--project" can also no longer be used in conjunction with additional source files.
- If a addon cannot be found it will bail out immediately instead of continously writing errors and failing the analysis at the end. - If a addon cannot be found it will bail out immediately instead of continously writing errors and failing the analysis at the end.
- Added CMake option "BUILD_MANPAGE" which adds the "man" target which will build the manpage. This requires xsltproc and the docbook XSLs to be installed. - Added CMake option "BUILD_MANPAGE" which adds the "man" target which will build the manpage. This requires xsltproc and the docbook XSLs to be installed.
- Improved inline suppressions:
- You can suppress warnings in a block of code using "-begin" and "-end".
- You can suppress warnings in current file using "-file".
- You can suppress all warnings where macro is used using "-macro"

View File

@ -777,6 +777,19 @@ private:
" int a; return a;\n" " int a; return a;\n"
"}\n", "}\n",
"uninitvar")); "uninitvar"));
// cppcheck-suppress-macro
(this->*check)("// cppcheck-suppress-macro zerodiv\n"
"#define DIV(A,B) A/B\n"
"a = DIV(10,0);\n",
"");
ASSERT_EQUALS("", errout.str());
(this->*check)("// cppcheck-suppress-macro abc\n"
"#define DIV(A,B) A/B\n"
"a = DIV(10,1);\n",
"");
ASSERT_EQUALS("", errout.str()); // <- no unmatched suppression reported for macro suppression
} }
void suppressionsSettings() { void suppressionsSettings() {