Fix false negatives for local suppressions

Introduce a new bool setting jointSuppressionReport
that will be set by the analyseWholeProgram() code path.

When the flag is enabled, unmatched suppressions are
collected after running the final whole program analysis
to prevent false positives for the unusedFunction check.

The check functions in the unit test
for single / multi file suppressions were unified.
This commit is contained in:
Thomas Jarosch 2015-01-07 19:26:16 +01:00
parent 5a8574cc05
commit ec21134817
8 changed files with 82 additions and 44 deletions

View File

@ -760,6 +760,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha
unsigned int returnValue = 0;
if (settings._jobs == 1) {
// Single process
settings.jointSuppressionReport = true;
std::size_t totalfilesize = 0;
for (std::map<std::string, std::size_t>::const_iterator i = _files.begin(); i != _files.end(); ++i) {
@ -799,8 +800,17 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha
returnValue = executor.check();
}
if (settings.isEnabled("information") || settings.checkConfiguration)
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(settings._jobs == 1 && settings.isEnabled("unusedFunction")));
if (settings.isEnabled("information") || settings.checkConfiguration) {
const bool enableUnusedFunctionCheck = cppcheck.unusedFunctionCheckIsEnabled();
if (settings.jointSuppressionReport) {
for (std::map<std::string, std::size_t>::const_iterator i = _files.begin(); i != _files.end(); ++i) {
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(i->first, enableUnusedFunctionCheck));
}
}
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(enableUnusedFunctionCheck));
}
if (!settings.checkConfiguration) {
cppcheck.tooManyConfigsError("",0U);

View File

@ -246,8 +246,11 @@ unsigned int CppCheck::processFile(const std::string& filename, std::istream& fi
internalError(filename, e.errorMessage);
}
if (_settings.isEnabled("information") || _settings.checkConfiguration)
reportUnmatchedSuppressions(_settings.nomsg.getUnmatchedLocalSuppressions(filename, _settings._jobs == 1 && _settings.isEnabled("unusedFunction")));
// In jointSuppressionReport mode, unmatched suppressions are
// collected after all files are processed
if (!_settings.jointSuppressionReport && (_settings.isEnabled("information") || _settings.checkConfiguration)) {
reportUnmatchedSuppressions(_settings.nomsg.getUnmatchedLocalSuppressions(filename, unusedFunctionCheckIsEnabled()));
}
_errorList.clear();
return exitcode;
@ -677,8 +680,12 @@ void CppCheck::getErrorMessages()
void CppCheck::analyseWholeProgram()
{
// Analyse the tokens..
// Analyse the tokens
for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it)
(*it)->analyseWholeProgram(fileInfo, *this);
}
bool CppCheck::unusedFunctionCheckIsEnabled() const
{
return (_settings._jobs == 1 && _settings.isEnabled("unusedFunction"));
}

View File

@ -132,6 +132,10 @@ public:
/** analyse whole program, run this after all TUs has been scanned. */
void analyseWholeProgram();
/** Check if the user wants to check for unused functions
* and if it's possible at all */
bool unusedFunctionCheckIsEnabled() const;
private:
/** @brief There has been an internal error => Report information message */

View File

@ -30,7 +30,9 @@ Settings::Settings()
debugFalsePositive(false),
dump(false),
exceptionHandling(false),
inconclusive(false), experimental(false),
inconclusive(false),
jointSuppressionReport(false),
experimental(false),
_errorsOnly(false),
_inlineSuppressions(false),
_verbose(false),

View File

@ -72,6 +72,11 @@ public:
/** @brief Inconclusive checks */
bool inconclusive;
/** @brief Collect unmatched suppressions in one run.
* This delays the reporting until all files are checked.
* It is needed by checks that analyse the whole code base. */
bool jointSuppressionReport;
/**
* When this flag is false (default) then experimental
* heuristics and checks are disabled.

View File

@ -265,13 +265,12 @@ bool Suppressions::isSuppressedLocal(const std::string &errorId, const std::stri
return _suppressions[errorId].isSuppressedLocal(file, line);
}
std::list<Suppressions::SuppressionEntry> Suppressions::getUnmatchedLocalSuppressions(const std::string &file, bool unusedFunctionChecking) const
std::list<Suppressions::SuppressionEntry> Suppressions::getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const
{
(void)unusedFunctionChecking;
std::list<SuppressionEntry> r;
for (std::map<std::string, FileMatcher>::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) {
if (i->first == "unusedFunction")
continue; // unusedFunction is not a "local" suppression
if (!unusedFunctionChecking && i->first == "unusedFunction")
continue;
std::map<std::string, std::map<unsigned int, bool> >::const_iterator f = i->second._files.find(file);
if (f != i->second._files.end()) {
@ -285,7 +284,7 @@ std::list<Suppressions::SuppressionEntry> Suppressions::getUnmatchedLocalSuppres
return r;
}
std::list<Suppressions::SuppressionEntry> Suppressions::getUnmatchedGlobalSuppressions(bool unusedFunctionChecking) const
std::list<Suppressions::SuppressionEntry> Suppressions::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const
{
std::list<SuppressionEntry> r;
for (std::map<std::string, FileMatcher>::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) {

View File

@ -133,13 +133,13 @@ public:
* @brief Returns list of unmatched local (per-file) suppressions.
* @return list of unmatched suppressions
*/
std::list<SuppressionEntry> getUnmatchedLocalSuppressions(const std::string &file, bool unusedFunctionChecking) const;
std::list<SuppressionEntry> getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const;
/**
* @brief Returns list of unmatched global (glob pattern) suppressions.
* @return list of unmatched suppressions
*/
std::list<SuppressionEntry> getUnmatchedGlobalSuppressions(bool unusedFunctionChecking) const;
std::list<SuppressionEntry> getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const;
};
/// @}

View File

@ -22,6 +22,8 @@
#include "cppcheckexecutor.h"
#include "threadexecutor.h"
#include <string>
#include <map>
#include <sstream>
extern std::ostringstream errout;
@ -115,8 +117,29 @@ private:
ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "a.c", 123));
}
void reportSuppressions(const Settings &settings, const std::map<std::string, std::string> &files) {
// make it verbose that this check is disabled
const bool unusedFunctionCheck = false;
if (settings.jointSuppressionReport) {
for (std::map<std::string, std::string>::const_iterator i = files.begin(); i != files.end(); ++i) {
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheck));
}
}
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(unusedFunctionCheck));
}
// Check the suppression
void checkSuppression(const char code[], const std::string &suppression = emptyString) {
std::map<std::string, std::string> files;
files["test.cpp"] = code;
checkSuppression(files, suppression);
}
// Check the suppression for multiple files
void checkSuppression(std::map<std::string, std::string> &files, const std::string &suppression = emptyString) {
// Clear the error log
errout.str("");
@ -124,14 +147,18 @@ private:
Settings& settings = cppCheck.settings();
settings._inlineSuppressions = true;
settings.addEnabled("information");
settings.jointSuppressionReport = true;
if (!suppression.empty()) {
std::string r = settings.nomsg.addSuppressionLine(suppression);
ASSERT_EQUALS("", r);
}
cppCheck.check("test.cpp", code);
for (std::map<std::string, std::string>::const_iterator file = files.begin(); file != files.end(); ++file) {
cppCheck.check(file->first, file->second);
}
cppCheck.analyseWholeProgram();
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(true));
reportSuppressions(settings, files);
}
void checkSuppressionThreads(const char code[], const std::string &suppression = emptyString) {
@ -154,25 +181,11 @@ private:
executor.check();
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(false));
}
std::map<std::string, std::string> files_for_report;
for (std::map<std::string, std::size_t>::const_iterator file = files.begin(); file != files.end(); ++file)
files_for_report[file->first] = "";
// Check the suppression for multiple files
void checkSuppression(const char *names[], const char *codes[], const std::string &suppression = emptyString) {
// Clear the error log
errout.str("");
CppCheck cppCheck(*this, true);
Settings& settings = cppCheck.settings();
settings._inlineSuppressions = true;
settings.addEnabled("information");
if (!suppression.empty())
settings.nomsg.addSuppressionLine(suppression);
for (int i = 0; names[i] != NULL; ++i)
cppCheck.check(names[i], codes[i]);
reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(true));
reportSuppressions(settings, files_for_report);
}
void runChecks(void (TestSuppressions::*check)(const char[], const std::string &)) {
@ -303,18 +316,16 @@ private:
}
void suppressionsMultiFile() {
const char *names[] = {"abc.cpp", "xyz.cpp", NULL};
const char *codes[] = {
"void f() {\n"
"}\n",
"void f() {\n"
" int a;\n"
" a++;\n"
"}\n",
};
std::map<std::string, std::string> files;
files["abc.cpp"] = "void f() {\n"
"}\n";
files["xyz.cpp"] = "void f() {\n"
" int a;\n"
" a++;\n"
"}\n";
// suppress uninitvar for this file and line
checkSuppression(names, codes, "uninitvar:xyz.cpp:3");
checkSuppression(files, "uninitvar:xyz.cpp:3");
ASSERT_EQUALS("", errout.str());
}
@ -327,7 +338,7 @@ private:
void inlinesuppress_unusedFunction() const { // #4210, #4946 - wrong report of "unmatchedSuppression" for "unusedFunction"
Suppressions suppressions;
suppressions.addSuppression("unusedFunction", "test.c", 3U);
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", true).empty());
ASSERT_EQUALS(true, !suppressions.getUnmatchedLocalSuppressions("test.c", true).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(true).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", false).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(false).empty());