From 9f7fc9838a21e0d7c67233f9dd2d17a63f6b0655 Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Wed, 1 May 2019 19:56:47 +0000 Subject: [PATCH] [analyzer] Don't display implementation checkers under -analyzer-checker-help, but do under the new flag -analyzer-checker-help-hidden During my work on analyzer dependencies, I created a great amount of new checkers that emitted no diagnostics at all, and were purely modeling some function or another. However, the user shouldn't really disable/enable these by hand, hence this patch, which hides these by default. I intentionally chose not to hide alpha checkers, because they have a scary enough name, in my opinion, to cause no surprise when they emit false positives or cause crashes. The patch introduces the Hidden bit into the TableGen files (you may remember it before I removed it in D53995), and checkers that are either marked as hidden, or are in a package that is marked hidden won't be displayed under -analyzer-checker-help. -analyzer-checker-help-hidden, a new flag meant for developers only, displays the full list. Differential Revision: https://reviews.llvm.org/D60925 llvm-svn: 359720 --- clang/include/clang/Driver/CC1Options.td | 6 ++- .../Checkers/BuiltinCheckerRegistration.h | 2 +- .../clang/StaticAnalyzer/Checkers/CheckerBase.td | 6 +++ .../clang/StaticAnalyzer/Checkers/Checkers.td | 47 ++++++++++++++-------- .../clang/StaticAnalyzer/Core/AnalyzerOptions.h | 1 + .../StaticAnalyzer/Frontend/CheckerRegistry.h | 15 ++++--- clang/lib/Frontend/CompilerInvocation.cpp | 1 + .../lib/FrontendTool/ExecuteCompilerInvocation.cpp | 16 ++++---- clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp | 2 +- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp | 4 +- .../StaticAnalyzer/Frontend/CheckerRegistry.cpp | 12 ++++-- clang/test/Analysis/show-checker-list.c | 11 +++++ clang/utils/TableGen/ClangSACheckersEmitter.cpp | 20 ++++++++- 13 files changed, 103 insertions(+), 40 deletions(-) create mode 100644 clang/test/Analysis/show-checker-list.c diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td index 2825a92..d18c37e 100644 --- a/clang/include/clang/Driver/CC1Options.td +++ b/clang/include/clang/Driver/CC1Options.td @@ -107,7 +107,7 @@ def analyzer_checker : Separate<["-"], "analyzer-checker">, ValuesCode<[{ const char *Values = #define GET_CHECKERS - #define CHECKER(FULLNAME, CLASS, HT, DOC_URI) FULLNAME "," + #define CHECKER(FULLNAME, CLASS, HT, DOC_URI, IS_HIDDEN) FULLNAME "," #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef GET_CHECKERS #define GET_PACKAGES @@ -130,6 +130,10 @@ def analyzer_disable_all_checks : Flag<["-"], "analyzer-disable-all-checks">, def analyzer_checker_help : Flag<["-"], "analyzer-checker-help">, HelpText<"Display the list of analyzer checkers that are available">; +def analyzer_checker_help_hidden : Flag<["-"], "analyzer-checker-help-hidden">, + HelpText<"Display the list of analyzer checkers that are available, " + "including modeling checkers">; + def analyzer_config_help : Flag<["-"], "analyzer-config-help">, HelpText<"Display the list of -analyzer-config options">; diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h b/clang/include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h index 51b30029..c773233 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h +++ b/clang/include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h @@ -26,7 +26,7 @@ class CheckerManager; class CheckerRegistry; #define GET_CHECKERS -#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI) \ +#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) \ void register##CLASS(CheckerManager &mgr); \ bool shouldRegister##CLASS(const LangOptions &LO); #include "clang/StaticAnalyzer/Checkers/Checkers.inc" diff --git a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td index 0b0f92e..c381d4b 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td @@ -49,6 +49,7 @@ class Package { // This field is optional. list PackageOptions; Package ParentPackage; + bit Hidden = 0; } /// Describes a 'super' package that holds another package inside it. This is @@ -91,6 +92,7 @@ class Checker { list Dependencies; bits<2> Documentation; Package ParentPackage; + bit Hidden = 0; } /// Describes a list of checker options. @@ -108,3 +110,7 @@ class CheckerOptions opts> { class Dependencies Deps = []> { list Dependencies = Deps; } + +/// Marks a checker or a package hidden. Hidden entries won't be displayed in +/// -analyzer-checker-help, which is desirable for alpha or modeling checkers. +class Hidden { bit Hidden = 1; } diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 0b16a2a..911edd8 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -20,7 +20,7 @@ include "CheckerBase.td" def Alpha : Package<"alpha">; def Core : Package<"core">; -def CoreBuiltin : Package<"builtin">, ParentPackage; +def CoreBuiltin : Package<"builtin">, ParentPackage, Hidden; def CoreUninitialized : Package<"uninitialized">, ParentPackage; def CoreAlpha : Package<"core">, ParentPackage; @@ -97,10 +97,10 @@ def LLVMAlpha : Package<"llvm">, ParentPackage; // The APIModeling package is for checkers that model APIs and don't perform // any diagnostics. These checkers are always turned on; this package is // intended for API modeling that is not controlled by the target triple. -def APIModeling : Package<"apiModeling">; -def GoogleAPIModeling : Package<"google">, ParentPackage; +def APIModeling : Package<"apiModeling">, Hidden; +def GoogleAPIModeling : Package<"google">, ParentPackage, Hidden; -def Debug : Package<"debug">; +def Debug : Package<"debug">, Hidden; def CloneDetectionAlpha : Package<"clone">, ParentPackage; @@ -141,7 +141,8 @@ def UndefResultChecker : Checker<"UndefinedBinaryOperatorResult">, def StackAddrEscapeBase : Checker<"StackAddrEscapeBase">, HelpText<"Generate information about stack address escapes.">, - Documentation; + Documentation, + Hidden; def StackAddrEscapeChecker : Checker<"StackAddressEscape">, HelpText<"Check that addresses to stack memory do not escape the function">, @@ -154,7 +155,8 @@ def DynamicTypePropagation : Checker<"DynamicTypePropagation">, def NonnullGlobalConstantsChecker: Checker<"NonnilStringConstants">, HelpText<"Assume that const string-like globals are non-null">, - Documentation; + Documentation, + Hidden; } // end "core" @@ -231,7 +233,8 @@ let ParentPackage = Nullability in { def NullabilityBase : Checker<"NullabilityBase">, HelpText<"Stores information during the analysis about nullability.">, - Documentation; + Documentation, + Hidden; def NullPassedToNonnullChecker : Checker<"NullPassedToNonnull">, HelpText<"Warns when a null pointer is passed to a pointer which has a " @@ -336,7 +339,8 @@ def CStringModeling : Checker<"CStringModeling">, HelpText<"The base of several CString related checkers. On it's own it emits " "no reports, but adds valuable information to the analysis when " "enabled.">, - Documentation; + Documentation, + Hidden; def CStringNullArg : Checker<"NullArg">, HelpText<"Check for null pointers being passed as arguments to C string " @@ -390,7 +394,8 @@ def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">, "false"> ]>, Dependencies<[CStringModeling]>, - Documentation; + Documentation, + Hidden; def MallocChecker: Checker<"Malloc">, HelpText<"Check for memory leaks, double free, and use-after-free problems. " @@ -462,11 +467,13 @@ def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">, def CXXSelfAssignmentChecker : Checker<"SelfAssignment">, HelpText<"Checks C++ copy and move assignment operators for self assignment">, - Documentation; + Documentation, + Hidden; def SmartPtrModeling: Checker<"SmartPtr">, HelpText<"Model behavior of C++ smart pointers">, - Documentation; + Documentation, + Hidden; def MoveChecker: Checker<"Move">, HelpText<"Find use-after-move bugs in C++">, @@ -559,7 +566,8 @@ def EnumCastOutOfRangeChecker : Checker<"EnumCastOutOfRange">, def IteratorModeling : Checker<"IteratorModeling">, HelpText<"Models iterators of C++ containers">, - Documentation; + Documentation, + Hidden; def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">, HelpText<"Check for use of invalidated iterators">, @@ -588,7 +596,8 @@ let ParentPackage = Valist in { def ValistBase : Checker<"ValistBase">, HelpText<"Gathers information about va_lists.">, - Documentation; + Documentation, + Hidden; def UninitializedChecker : Checker<"Uninitialized">, HelpText<"Check for usages of uninitialized (or already released) va_lists.">, @@ -655,7 +664,8 @@ let ParentPackage = InsecureAPI in { def SecuritySyntaxChecker : Checker<"SecuritySyntaxChecker">, HelpText<"Base of various security function related checkers">, - Documentation; + Documentation, + Hidden; def bcmp : Checker<"bcmp">, HelpText<"Warn on uses of the 'bcmp' function">, @@ -787,7 +797,8 @@ let ParentPackage = Cocoa in { def RetainCountBase : Checker<"RetainCountBase">, HelpText<"Common base of various retain count related checkers">, - Documentation; + Documentation, + Hidden; } // end "osx.cocoa" @@ -795,7 +806,8 @@ let ParentPackage = OSX in { def NSOrCFErrorDerefChecker : Checker<"NSOrCFErrorDerefChecker">, HelpText<"Implementation checker for NSErrorChecker and CFErrorChecker">, - Documentation; + Documentation, + Hidden; def NumberObjectConversionChecker : Checker<"NumberObjectConversion">, HelpText<"Check for erroneous conversions of objects representing numbers " @@ -962,7 +974,8 @@ def IvarInvalidationModeling : Checker<"IvarInvalidationModeling">, HelpText<"Gathers information for annotation driven invalidation checking " "for classes that contains a method annotated with " "'objc_instance_variable_invalidator'">, - Documentation; + Documentation, + Hidden; def InstanceVariableInvalidation : Checker<"InstanceVariableInvalidation">, HelpText<"Check that the invalidatable instance variables are invalidated in " diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h index b8d9213..63b7cd3 100644 --- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -197,6 +197,7 @@ public: unsigned DisableAllChecks : 1; unsigned ShowCheckerHelp : 1; + unsigned ShowCheckerHelpHidden : 1; unsigned ShowEnabledCheckerList : 1; unsigned ShowConfigOptionsList : 1; unsigned ShouldEmitErrorsOnInvalidConfigValue : 1; diff --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h index 8550baa..c6cb8ac 100644 --- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -137,6 +137,7 @@ public: StringRef Desc; StringRef DocumentationUri; CmdLineOptionList CmdLineOptions; + bool IsHidden = false; StateFromCmdLine State = StateFromCmdLine::State_Unspecified; ConstCheckerInfoList Dependencies; @@ -150,9 +151,10 @@ public: } CheckerInfo(InitializationFunction Fn, ShouldRegisterFunction sfn, - StringRef Name, StringRef Desc, StringRef DocsUri) + StringRef Name, StringRef Desc, StringRef DocsUri, + bool IsHidden) : Initialize(Fn), ShouldRegister(sfn), FullName(Name), Desc(Desc), - DocumentationUri(DocsUri) {} + DocumentationUri(DocsUri), IsHidden(IsHidden) {} // Used for lower_bound. explicit CheckerInfo(StringRef FullName) : FullName(FullName) {} @@ -190,16 +192,19 @@ public: /// Adds a checker to the registry. Use this non-templated overload when your /// checker requires custom initialization. void addChecker(InitializationFunction Fn, ShouldRegisterFunction sfn, - StringRef FullName, StringRef Desc, StringRef DocsUri); + StringRef FullName, StringRef Desc, StringRef DocsUri, + bool IsHidden); /// Adds a checker to the registry. Use this templated overload when your /// checker does not require any custom initialization. template - void addChecker(StringRef FullName, StringRef Desc, StringRef DocsUri) { + void addChecker(StringRef FullName, StringRef Desc, StringRef DocsUri, + bool IsHidden = false) { // Avoid MSVC's Compiler Error C2276: // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx addChecker(&CheckerRegistry::initializeManager, - &CheckerRegistry::returnTrue, FullName, Desc, DocsUri); + &CheckerRegistry::returnTrue, FullName, Desc, DocsUri, + IsHidden); } /// Makes the checker with the full name \p fullName depends on the checker diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index cc7d5b6..12846b1 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -285,6 +285,7 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args, } Opts.ShowCheckerHelp = Args.hasArg(OPT_analyzer_checker_help); + Opts.ShowCheckerHelpHidden = Args.hasArg(OPT_analyzer_checker_help_hidden); Opts.ShowConfigOptionsList = Args.hasArg(OPT_analyzer_config_help); Opts.ShowEnabledCheckerList = Args.hasArg(OPT_analyzer_list_enabled_checkers); Opts.ShouldEmitErrorsOnInvalidConfigValue = diff --git a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp index c226838..f77a865 100644 --- a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp +++ b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp @@ -234,28 +234,30 @@ bool ExecuteCompilerInvocation(CompilerInstance *Clang) { } #if CLANG_ENABLE_STATIC_ANALYZER - // Honor -analyzer-checker-help. - // This should happen AFTER plugins have been loaded! - if (Clang->getAnalyzerOpts()->ShowCheckerHelp) { + // These should happen AFTER plugins have been loaded! + + AnalyzerOptions &AnOpts = *Clang->getAnalyzerOpts(); + // Honor -analyzer-checker-help and -analyzer-checker-help-hidden. + if (AnOpts.ShowCheckerHelp || AnOpts.ShowCheckerHelpHidden) { ento::printCheckerHelp(llvm::outs(), Clang->getFrontendOpts().Plugins, - *Clang->getAnalyzerOpts(), + AnOpts, Clang->getDiagnostics(), Clang->getLangOpts()); return true; } // Honor -analyzer-list-enabled-checkers. - if (Clang->getAnalyzerOpts()->ShowEnabledCheckerList) { + if (AnOpts.ShowEnabledCheckerList) { ento::printEnabledCheckerList(llvm::outs(), Clang->getFrontendOpts().Plugins, - *Clang->getAnalyzerOpts(), + AnOpts, Clang->getDiagnostics(), Clang->getLangOpts()); } // Honor -analyzer-config-help. - if (Clang->getAnalyzerOpts()->ShowConfigOptionsList) { + if (AnOpts.ShowConfigOptionsList) { ento::printAnalyzerConfigList(llvm::outs()); return true; } diff --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp index ee670fb..893c721 100644 --- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -33,7 +33,7 @@ std::vector AnalyzerOptions::getRegisteredCheckers(bool IncludeExperimental /* = false */) { static const StringRef StaticAnalyzerChecks[] = { #define GET_CHECKERS -#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI) \ +#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) \ FULLNAME, #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp index 226d5a9..a8f529b 100644 --- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp @@ -256,7 +256,7 @@ static json::Object createResult(const PathDiagnostic &Diag, json::Array &Files, static StringRef getRuleDescription(StringRef CheckName) { return llvm::StringSwitch(CheckName) #define GET_CHECKERS -#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI) \ +#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) \ .Case(FULLNAME, HELPTEXT) #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER @@ -267,7 +267,7 @@ static StringRef getRuleDescription(StringRef CheckName) { static StringRef getRuleHelpURIStr(StringRef CheckName) { return llvm::StringSwitch(CheckName) #define GET_CHECKERS -#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI) \ +#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) \ .Case(FULLNAME, DOC_URI) #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER diff --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp index f1ee454..4267d8a 100644 --- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -115,9 +115,9 @@ CheckerRegistry::CheckerRegistry( // Register builtin checkers. #define GET_CHECKERS -#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI) \ +#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) \ addChecker(register##CLASS, shouldRegister##CLASS, FULLNAME, HELPTEXT, \ - DOC_URI); + DOC_URI, IS_HIDDEN); #define GET_PACKAGES #define PACKAGE(FULLNAME) addPackage(FULLNAME); @@ -350,8 +350,9 @@ void CheckerRegistry::addPackageOption(StringRef OptionType, void CheckerRegistry::addChecker(InitializationFunction Rfn, ShouldRegisterFunction Sfn, StringRef Name, - StringRef Desc, StringRef DocsUri) { - Checkers.emplace_back(Rfn, Sfn, Name, Desc, DocsUri); + StringRef Desc, StringRef DocsUri, + bool IsHidden) { + Checkers.emplace_back(Rfn, Sfn, Name, Desc, DocsUri, IsHidden); // Record the presence of the checker in its packages. StringRef PackageName, LeafName; @@ -421,6 +422,9 @@ void CheckerRegistry::printCheckerWithDescList(raw_ostream &Out, const size_t InitialPad = 2; for (const auto &Checker : Checkers) { + if (!AnOpts.ShowCheckerHelpHidden && Checker.IsHidden) + continue; + Out.indent(InitialPad) << Checker.FullName; int Pad = OptionFieldWidth - Checker.FullName.size(); diff --git a/clang/test/Analysis/show-checker-list.c b/clang/test/Analysis/show-checker-list.c new file mode 100644 index 0000000..83ed6e4 --- /dev/null +++ b/clang/test/Analysis/show-checker-list.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -analyzer-checker-help \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK + +// RUN: %clang_cc1 -analyzer-checker-help-hidden \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-HIDDEN + +// CHECK: core.DivideZero +// CHECK-HIDDEN: core.DivideZero + +// CHECK-NOT: unix.DynamicMemoryModeling +// CHECK-HIDDEN: unix.DynamicMemoryModeling diff --git a/clang/utils/TableGen/ClangSACheckersEmitter.cpp b/clang/utils/TableGen/ClangSACheckersEmitter.cpp index 03c3b3b..428a5c8 100644 --- a/clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ b/clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -110,6 +110,16 @@ static std::string getCheckerOptionType(const Record &R) { return ""; } +static bool isHidden(const Record *R) { + if (R->getValueAsBit("Hidden")) + return true; + // Not declared as hidden, check the parent package if it is hidden. + if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage"))) + return isHidden(DI->getDef()); + + return false; +} + static void printChecker(llvm::raw_ostream &OS, const Record &R) { OS << "CHECKER(" << "\""; OS.write_escaped(getCheckerFullName(&R)) << "\", "; @@ -118,7 +128,14 @@ static void printChecker(llvm::raw_ostream &OS, const Record &R) { OS.write_escaped(getStringValue(R, "HelpText")) << "\", "; OS << "\""; OS.write_escaped(getCheckerDocs(R)); - OS << "\""; + OS << "\", "; + + if (!isHidden(&R)) + OS << "false"; + else + OS << "true"; + + OS << ")\n"; } namespace clang { @@ -206,7 +223,6 @@ void EmitClangSACheckers(RecordKeeper &Records, raw_ostream &OS) { "\n"; for (const Record *checker : checkers) { printChecker(OS, *checker); - OS << ")\n"; } OS << "\n" "#endif // GET_CHECKERS\n" -- 2.7.4