From: Csaba Dabis Date: Fri, 16 Aug 2019 01:53:14 +0000 (+0000) Subject: [analyzer] Analysis: Silence checkers X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=27cb4e064171b635c6cd9ea16ef3a7d9deacce7e;p=clang [analyzer] Analysis: Silence checkers Summary: This patch introduces a new `analyzer-config` configuration: `-analyzer-config silence-checkers` which could be used to silence the given checkers. It accepts a semicolon separated list, packed into quotation marks, e.g: `-analyzer-config silence-checkers="core.DivideZero;core.NullDereference"` It could be used to "disable" core checkers, so they model the analysis as before, just if some of them are too noisy it prevents to emit reports. This patch also adds support for that new option to the scan-build. Passing the option `-disable-checker core.DivideZero` to the scan-build will be transferred to `-analyzer-config silence-checkers=core.DivideZero`. Reviewed By: NoQ, Szelethus Differential Revision: https://reviews.llvm.org/D66042 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@369078 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticCommonKinds.td b/include/clang/Basic/DiagnosticCommonKinds.td index ca2faf59d7..e5ad50fd0f 100644 --- a/include/clang/Basic/DiagnosticCommonKinds.td +++ b/include/clang/Basic/DiagnosticCommonKinds.td @@ -300,7 +300,7 @@ def err_omp_more_one_clause : Error< "directive '#pragma omp %0' cannot contain more than one '%1' clause%select{| with '%3' name modifier| with 'source' dependence}2">; // Static Analyzer Core -def err_unknown_analyzer_checker : Error< +def err_unknown_analyzer_checker_or_package : Error< "no analyzer checkers or packages are associated with '%0'">; def note_suggest_disabling_all_checkers : Note< "use -analyzer-disable-all-checks to disable all static analyzer checkers">; diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def index 70bd476b6c..95dc3524ce 100644 --- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def +++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def @@ -380,12 +380,6 @@ ANALYZER_OPTION( "Value: \"constructors\", \"destructors\", \"methods\".", "destructors") -ANALYZER_OPTION_DEPENDS_ON_USER_MODE( - StringRef, IPAMode, "ipa", - "Controls the mode of inter-procedural analysis. Value: \"none\", " - "\"basic-inlining\", \"inlining\", \"dynamic\", \"dynamic-bifurcate\".", - /* SHALLOW_VAL */ "inlining", /* DEEP_VAL */ "dynamic-bifurcate") - ANALYZER_OPTION( StringRef, ExplorationStrategy, "exploration_strategy", "Value: \"dfs\", \"bfs\", \"unexplored_first\", " @@ -393,5 +387,17 @@ ANALYZER_OPTION( "\"bfs_block_dfs_contents\".", "unexplored_first_queue") +ANALYZER_OPTION( + StringRef, RawSilencedCheckersAndPackages, "silence-checkers", + "A semicolon separated list of checker and package names to silence. " + "Silenced checkers will not emit reports, but the modeling remain enabled.", + "") + +ANALYZER_OPTION_DEPENDS_ON_USER_MODE( + StringRef, IPAMode, "ipa", + "Controls the mode of inter-procedural analysis. Value: \"none\", " + "\"basic-inlining\", \"inlining\", \"dynamic\", \"dynamic-bifurcate\".", + /* SHALLOW_VAL */ "inlining", /* DEEP_VAL */ "dynamic-bifurcate") + #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE #undef ANALYZER_OPTION diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h index 9630a229bd..609932ad70 100644 --- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -164,7 +164,40 @@ public: using ConfigTable = llvm::StringMap; static std::vector - getRegisteredCheckers(bool IncludeExperimental = false); + getRegisteredCheckers(bool IncludeExperimental = false) { + static const StringRef StaticAnalyzerChecks[] = { +#define GET_CHECKERS +#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) FULLNAME, +#include "clang/StaticAnalyzer/Checkers/Checkers.inc" +#undef CHECKER +#undef GET_CHECKERS + }; + std::vector Checkers; + for (StringRef CheckerName : StaticAnalyzerChecks) { + if (!CheckerName.startswith("debug.") && + (IncludeExperimental || !CheckerName.startswith("alpha."))) + Checkers.push_back(CheckerName); + } + return Checkers; + } + + static std::vector + getRegisteredPackages(bool IncludeExperimental = false) { + static const StringRef StaticAnalyzerPackages[] = { +#define GET_PACKAGES +#define PACKAGE(FULLNAME) FULLNAME, +#include "clang/StaticAnalyzer/Checkers/Checkers.inc" +#undef PACKAGE +#undef GET_PACKAGES + }; + std::vector Packages; + for (StringRef PackageName : StaticAnalyzerPackages) { + if (PackageName != "debug" && + (IncludeExperimental || PackageName != "alpha")) + Packages.push_back(PackageName); + } + return Packages; + } /// Convenience function for printing options or checkers and their /// description in a formatted manner. If \p MinLineWidth is set to 0, no line @@ -188,9 +221,11 @@ public: std::pair EntryDescPair, size_t EntryWidth, size_t InitialPad, size_t MinLineWidth = 0); + /// Pairs of checker/package name and enable/disable. + std::vector> CheckersAndPackages; - /// Pair of checker name and enable/disable. - std::vector> CheckersControlList; + /// Vector of checker/package names which will not emit warnings. + std::vector SilencedCheckersAndPackages; /// A key-value table of use-specified configuration values. // TODO: This shouldn't be public. diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index fb5f1cc5ac..66684e5c96 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -324,18 +324,18 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args, getLastArgIntValue(Args, OPT_analyzer_inline_max_stack_depth, Opts.InlineMaxStackDepth, Diags); - Opts.CheckersControlList.clear(); + Opts.CheckersAndPackages.clear(); for (const Arg *A : Args.filtered(OPT_analyzer_checker, OPT_analyzer_disable_checker)) { A->claim(); - bool enable = (A->getOption().getID() == OPT_analyzer_checker); + bool IsEnabled = A->getOption().getID() == OPT_analyzer_checker; // We can have a list of comma separated checker names, e.g: // '-analyzer-checker=cocoa,unix' - StringRef checkerList = A->getValue(); - SmallVector checkers; - checkerList.split(checkers, ","); - for (auto checker : checkers) - Opts.CheckersControlList.emplace_back(checker, enable); + StringRef CheckerAndPackageList = A->getValue(); + SmallVector CheckersAndPackages; + CheckerAndPackageList.split(CheckersAndPackages, ","); + for (const StringRef CheckerOrPackage : CheckersAndPackages) + Opts.CheckersAndPackages.emplace_back(CheckerOrPackage, IsEnabled); } // Go through the analyzer configuration options. @@ -479,6 +479,32 @@ static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts, !llvm::sys::fs::is_directory(AnOpts.ModelPath)) Diags->Report(diag::err_analyzer_config_invalid_input) << "model-path" << "a filename"; + + // FIXME: Here we try to validate the silenced checkers or packages are valid. + // The current approach only validates the registered checkers which does not + // contain the runtime enabled checkers and optimally we would validate both. + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { + std::vector Checkers = + AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); + std::vector Packages = + AnOpts.getRegisteredPackages(/*IncludeExperimental=*/true); + + SmallVector CheckersAndPackages; + AnOpts.RawSilencedCheckersAndPackages.split(CheckersAndPackages, ";"); + + for (const StringRef CheckerOrPackage : CheckersAndPackages) { + bool IsChecker = CheckerOrPackage.contains('.'); + bool IsValidName = + IsChecker ? llvm::find(Checkers, CheckerOrPackage) != Checkers.end() + : llvm::find(Packages, CheckerOrPackage) != Packages.end(); + + if (!IsValidName) + Diags->Report(diag::err_unknown_analyzer_checker_or_package) + << CheckerOrPackage; + + AnOpts.SilencedCheckersAndPackages.emplace_back(CheckerOrPackage); + } + } } static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) { diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp index 71abe2ae6c..01ac2bc83b 100644 --- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -30,25 +30,6 @@ using namespace clang; using namespace ento; using namespace llvm; -std::vector -AnalyzerOptions::getRegisteredCheckers(bool IncludeExperimental /* = false */) { - static const StringRef StaticAnalyzerChecks[] = { -#define GET_CHECKERS -#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) \ - FULLNAME, -#include "clang/StaticAnalyzer/Checkers/Checkers.inc" -#undef CHECKER -#undef GET_CHECKERS - }; - std::vector Result; - for (StringRef CheckName : StaticAnalyzerChecks) { - if (!CheckName.startswith("debug.") && - (IncludeExperimental || !CheckName.startswith("alpha."))) - Result.push_back(CheckName); - } - return Result; -} - void AnalyzerOptions::printFormattedEntry( llvm::raw_ostream &Out, std::pair EntryDescPair, diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 4cfcdee1c3..37a3ddecf7 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -1924,15 +1924,22 @@ PathDiagnosticBuilder::PathDiagnosticBuilder( std::unique_ptr PathDiagnosticBuilder::generate(const PathDiagnosticConsumer *PDC) const { - - if (!PDC->shouldGenerateDiagnostics()) - return generateEmptyDiagnosticForReport(R, getSourceManager()); - PathDiagnosticConstruct Construct(PDC, ErrorNode, R); const SourceManager &SM = getSourceManager(); const BugReport *R = getBugReport(); const AnalyzerOptions &Opts = getAnalyzerOptions(); + StringRef ErrorTag = ErrorNode->getLocation().getTag()->getTagDescription(); + + // See whether we need to silence the checker/package. + // FIXME: This will not work if the report was emitted with an incorrect tag. + for (const std::string &CheckerOrPackage : Opts.SilencedCheckersAndPackages) { + if (ErrorTag.startswith(CheckerOrPackage)) + return nullptr; + } + + if (!PDC->shouldGenerateDiagnostics()) + return generateEmptyDiagnosticForReport(R, getSourceManager()); // Construct the final (warning) event for the bug report. auto EndNotes = VisitorsDiagnostics->find(ErrorNode); @@ -2029,7 +2036,6 @@ PathDiagnosticBuilder::generate(const PathDiagnosticConsumer *PDC) const { return std::move(Construct.PD); } - //===----------------------------------------------------------------------===// // Methods for BugType and subclasses. //===----------------------------------------------------------------------===// @@ -2646,9 +2652,13 @@ GRBugReporter::generatePathDiagnostics( Optional PDB = PathDiagnosticBuilder::findValidReport(bugReports, *this); - if (PDB) - for (PathDiagnosticConsumer *PC : consumers) - (*Out)[PC] = PDB->generate(PC); + if (PDB) { + for (PathDiagnosticConsumer *PC : consumers) { + if (std::unique_ptr PD = PDB->generate(PC)) { + (*Out)[PC] = std::move(PD); + } + } + } return Out; } diff --git a/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp index 3fd4c36947..322304b0fb 100644 --- a/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ b/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -200,12 +200,12 @@ CheckerRegistry::CheckerRegistry( // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the // command line. - for (const std::pair &Opt : AnOpts.CheckersControlList) { + for (const std::pair &Opt : AnOpts.CheckersAndPackages) { CheckerInfoListRange CheckerForCmdLineArg = getMutableCheckersForCmdLineArg(Opt.first); if (CheckerForCmdLineArg.begin() == CheckerForCmdLineArg.end()) { - Diags.Report(diag::err_unknown_analyzer_checker) << Opt.first; + Diags.Report(diag::err_unknown_analyzer_checker_or_package) << Opt.first; Diags.Report(diag::note_suggest_disabling_all_checkers); } @@ -468,9 +468,10 @@ isOptionContainedIn(const CheckerRegistry::CmdLineOptionList &OptionList, void CheckerRegistry::validateCheckerOptions() const { for (const auto &Config : AnOpts.Config) { - StringRef SuppliedChecker; + StringRef SuppliedCheckerOrPackage; StringRef SuppliedOption; - std::tie(SuppliedChecker, SuppliedOption) = Config.getKey().split(':'); + std::tie(SuppliedCheckerOrPackage, SuppliedOption) = + Config.getKey().split(':'); if (SuppliedOption.empty()) continue; @@ -483,21 +484,24 @@ void CheckerRegistry::validateCheckerOptions() const { // Since lower_bound would look for the first element *not less* than "cor", // it would return with an iterator to the first checker in the core, so we // we really have to use find here, which uses operator==. - auto CheckerIt = llvm::find(Checkers, CheckerInfo(SuppliedChecker)); + auto CheckerIt = + llvm::find(Checkers, CheckerInfo(SuppliedCheckerOrPackage)); if (CheckerIt != Checkers.end()) { - isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedChecker, + isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedCheckerOrPackage, SuppliedOption, AnOpts, Diags); continue; } - auto PackageIt = llvm::find(Packages, PackageInfo(SuppliedChecker)); + auto PackageIt = + llvm::find(Packages, PackageInfo(SuppliedCheckerOrPackage)); if (PackageIt != Packages.end()) { - isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedChecker, + isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedCheckerOrPackage, SuppliedOption, AnOpts, Diags); continue; } - Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker; + Diags.Report(diag::err_unknown_analyzer_checker_or_package) + << SuppliedCheckerOrPackage; } } diff --git a/test/Analysis/analyzer-config.c b/test/Analysis/analyzer-config.c index 7cba52fdab..99e1173d5d 100644 --- a/test/Analysis/analyzer-config.c +++ b/test/Analysis/analyzer-config.c @@ -82,6 +82,7 @@ // CHECK-NEXT: region-store-small-struct-limit = 2 // CHECK-NEXT: report-in-main-source-file = false // CHECK-NEXT: serialize-stats = false +// CHECK-NEXT: silence-checkers = "" // CHECK-NEXT: stable-report-filename = false // CHECK-NEXT: suppress-c++-stdlib = true // CHECK-NEXT: suppress-inlined-defensive-checks = true @@ -92,4 +93,4 @@ // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: widen-loops = false // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 89 +// CHECK-NEXT: num-entries = 90 diff --git a/test/Analysis/silence-checkers-and-packages-core-all.cpp b/test/Analysis/silence-checkers-and-packages-core-all.cpp new file mode 100644 index 0000000000..0805cfc877 --- /dev/null +++ b/test/Analysis/silence-checkers-and-packages-core-all.cpp @@ -0,0 +1,39 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core -analyzer-config \ +// RUN: silence-checkers=core \ +// RUN: -verify %s + +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core -analyzer-config \ +// RUN: silence-checkers="core.DivideZero;core.NullDereference" \ +// RUN: -verify %s + +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core -analyzer-config \ +// RUN: silence-checkers=core.NullDeref \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-ERROR + +// CHECK-CHECKER-ERROR: (frontend): no analyzer checkers or packages +// CHECK-CHECKER-ERROR-SAME: are associated with 'core.NullDeref' + +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core -analyzer-config \ +// RUN: silence-checkers=coreModeling \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-PACKAGE-ERROR + +// CHECK-PACKAGE-ERROR: (frontend): no analyzer checkers or packages +// CHECK-PACKAGE-ERROR-SAME: are associated with 'coreModeling' + +void test_disable_core_div_by_zero() { + (void)(1 / 0); + // expected-warning@-1 {{division by zero is undefined}} + // no-warning: 'Division by zero' +} + +void test_disable_null_deref(int *p) { + if (p) + return; + + int x = p[0]; + // no-warning: Array access (from variable 'p') results in a null pointer dereference +} diff --git a/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp b/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp new file mode 100644 index 0000000000..3930f5a602 --- /dev/null +++ b/test/Analysis/silence-checkers-and-packages-core-div-by-zero.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core -analyzer-config \ +// RUN: silence-checkers=core.DivideZero \ +// RUN: -verify %s + +void test_disable_core_div_by_zero() { + (void)(1 / 0); + // expected-warning@-1 {{division by zero is undefined}} + // no-warning: 'Division by zero' +} + +void test_disable_null_deref(int *p) { + if (p) + return; + + int x = p[0]; + // expected-warning@-1 {{Array access (from variable 'p') results in a null pointer dereference}} +} diff --git a/tools/scan-build/bin/scan-build b/tools/scan-build/bin/scan-build index 903e19a290..37c94d5ec3 100755 --- a/tools/scan-build/bin/scan-build +++ b/tools/scan-build/bin/scan-build @@ -57,6 +57,7 @@ my %Options = ( KeepEmpty => 0, # Don't remove output directory even with 0 results. EnableCheckers => {}, DisableCheckers => {}, + SilenceCheckers => {}, Excludes => [], UseCC => undef, # C compiler to use for compilation. UseCXX => undef, # C++ compiler to use for compilation. @@ -1742,9 +1743,15 @@ sub ProcessArgs { if ($arg eq "-disable-checker") { shift @$Args; my $Checker = shift @$Args; - # Store $NumArgs to preserve the order the checkers were disabled. - $Options{DisableCheckers}{$Checker} = $NumArgs; - delete $Options{EnableCheckers}{$Checker}; + # Store $NumArgs to preserve the order the checkers are disabled/silenced. + # See whether it is a core checker to disable. That means we do not want + # to emit a report from that checker so we have to silence it. + if (index($Checker, "core") == 0) { + $Options{SilenceCheckers}{$Checker} = $NumArgs; + } else { + $Options{DisableCheckers}{$Checker} = $NumArgs; + delete $Options{EnableCheckers}{$Checker}; + } next; } @@ -1882,6 +1889,11 @@ foreach (sort { $Options{DisableCheckers}{$a} <=> $Options{DisableCheckers}{$b} # Push checkers in order they were disabled. push @AnalysesToRun, "-analyzer-disable-checker", $_; } +foreach (sort { $Options{SilenceCheckers}{$a} <=> $Options{SilenceCheckers}{$b} } + keys %{$Options{SilenceCheckers}}) { + # Push checkers in order they were silenced. + push @AnalysesToRun, "-analyzer-config silence-checker", $_; +} if ($Options{AnalyzeHeaders}) { push @AnalysesToRun, "-analyzer-opt-analyze-headers"; } if ($Options{AnalyzerStats}) { push @AnalysesToRun, '-analyzer-checker=debug.Stats'; } if ($Options{MaxLoop} > 0) { push @AnalysesToRun, "-analyzer-max-loop $Options{MaxLoop}"; } diff --git a/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp b/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp index d8988a0ee3..109e762892 100644 --- a/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp +++ b/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp @@ -46,7 +46,7 @@ public: std::unique_ptr AnalysisConsumer = CreateAnalysisConsumer(Compiler); AnalysisConsumer->AddDiagnosticConsumer(new DiagConsumer(DiagsOutput)); - Compiler.getAnalyzerOpts()->CheckersControlList = { + Compiler.getAnalyzerOpts()->CheckersAndPackages = { {"custom.CustomChecker", true}}; AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) { Registry.addChecker("custom.CustomChecker", "Description", "");