Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, baloghadamsoftware. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity. Herald added a project: clang.
Here we go guys! This patch is I believe the most important since I started working on this `AnalyzerOptions` refactoring effort. It's been a long standing issue that checker options were deep hidden within the implementation, not even necessarily on the bottom of the checker files, in some cases they were totally undocumented, and the possibility of user error due to them being so long was enormous, while at the same time giving no warnings or errors at all. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options, add them to both `CheckerRegistry` and `AnalyzerOptions` while at the same time exposing the same functionality to non-generated statically linked and plugin checkers. In detail: - Add checker and package options to the TableGen files - Added a new class called `CmdLineOption`, and both `Package` and `Checker` recieved a `list<CmdLineOption>` field. - Added every existing checker option to `Checkers.td`. I'm a little unsure about some, so take a look I guess! - `AnalyzerOptions` now inserts every checker option with their default value to it's `ConfigTable` (insertion fails if the user specified that option already) - The `CheckerRegistry` class - Received some comments to most of it's inline classes - Received the `CmdLineOption` and `PackageInfo` inline classes, a list of `CmdLineOption` was added to `CheckerInfo` and `PackageInfo` - Moved the `addDependency` function out-of-line - Renamed the cryptic `printHelp` and `printList` functions - Added `addCheckerOption` and `addPackageOption` - Added a new field called `Packages`, used in `addPackageOptions`, filled up in `addPackage` - `validateCheckerOptions` was refactored, but actual checker option validation will come in a later patch I know this patch is huge, and if it's a big burden, I'll look for ways to cut some pieces out, but I definitely didn't want to delay showing the overall direction any longer. Repository: rC Clang https://reviews.llvm.org/D57855 Files: include/clang/Basic/DiagnosticCommonKinds.td include/clang/StaticAnalyzer/Checkers/CheckerBase.td include/clang/StaticAnalyzer/Checkers/Checkers.td include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h lib/Frontend/CompilerInvocation.cpp lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp test/Analysis/analyzer-config.c test/Analysis/disable-all-checks.c test/Analysis/invalid-checker-option.c utils/TableGen/ClangSACheckersEmitter.cpp
Index: utils/TableGen/ClangSACheckersEmitter.cpp =================================================================== --- utils/TableGen/ClangSACheckersEmitter.cpp +++ utils/TableGen/ClangSACheckersEmitter.cpp @@ -90,6 +90,23 @@ .str(); } +static std::string getCheckerOptionType(const Record &R) { + if (BitsInit *BI = R.getValueAsBitsInit("Type")) { + switch(getValueFromBitsInit(BI, R)) { + case 0: + return "int"; + case 1: + return "string"; + case 2: + return "bool"; + } + } + PrintFatalError(R.getLoc(), + "unable to parse command line option type for " + + getCheckerFullName(&R)); + return ""; +} + static void printChecker(llvm::raw_ostream &OS, const Record &R) { OS << "CHECKER(" << "\""; OS.write_escaped(getCheckerFullName(&R)) << "\", "; @@ -134,6 +151,45 @@ OS << "#endif // GET_PACKAGES\n" "\n"; + // Emit a package option. + // + // PACKAGE_OPTION(OPTIONTYPE, PACKAGENAME, OPTIONNAME, DESCRIPTION, DEFAULT) + // - OPTIONTYPE: Type of the option, whether it's integer or boolean etc. + // This is important for validating user input. Note that + // it's a string, rather than an actual type: since we can + // load checkers runtime, we can't use template hackery for + // sorting this out compile-time. + // - PACKAGENAME: Name of the package. + // - OPTIONNAME: Name of the option. + // - DESCRIPTION + // - DEFAULT: The default value for this option. + // + // The full option can be specified in the command like like this: + // -analyzer-config PACKAGENAME:OPTIONNAME=VALUE + OS << "\n" + "#ifdef GET_PACKAGE_OPTIONS\n"; + for (const Record *package : packages) { + + if (package->isValueUnset("PackageOptions")) + continue; + + std::vector<Record *> PackageOptions = package + ->getValueAsListOfDefs("PackageOptions"); + for (Record *PackageOpt : PackageOptions) { + OS << "PACKAGE_OPTION(\""; + OS.write_escaped(getCheckerOptionType(*PackageOpt)) << "\", \""; + OS.write_escaped(getPackageFullName(package)) << "\", "; + OS << '\"' << getStringValue(*PackageOpt, "CmdFlag") << "\", "; + OS << '\"'; + OS.write_escaped(getStringValue(*PackageOpt, "Desc")) << "\", "; + OS << '\"'; + OS.write_escaped(getStringValue(*PackageOpt, "DefaultVal")) << "\""; + OS << ")\n"; + } + } + OS << "#endif // GET_PACKAGE_OPTIONS\n" + "\n"; + // Emit checkers. // // CHECKER(FULLNAME, CLASS, HELPTEXT) @@ -176,5 +232,46 @@ } OS << "\n" "#endif // GET_CHECKER_DEPENDENCIES\n"; + + // Emit a package option. + // + // CHECKER_OPTION(OPTIONTYPE, CHECKERNAME, OPTIONNAME, DESCRIPTION, DEFAULT) + // - OPTIONTYPE: Type of the option, whether it's integer or boolean etc. + // This is important for validating user input. Note that + // it's a string, rather than an actual type: since we can + // load checkers runtime, we can't use template hackery for + // sorting this out compile-time. + // - CHECKERNAME: Name of the package. + // - OPTIONNAME: Name of the option. + // - DESCRIPTION + // - DEFAULT: The default value for this option. + // + // The full option can be specified in the command like like this: + // -analyzer-config CHECKERNAME:OPTIONNAME=VALUE + OS << "\n" + "#ifdef GET_CHECKER_OPTIONS\n"; + for (const Record *checker : checkers) { + + if (checker->isValueUnset("CheckerOptions")) + continue; + + + std::vector<Record *> CheckerOptions = checker + ->getValueAsListOfDefs("CheckerOptions"); + for (Record *CheckerOpt : CheckerOptions) { + OS << "CHECKER_OPTION(\""; + OS << getCheckerOptionType(*CheckerOpt) << "\", \""; + OS.write_escaped(getCheckerFullName(checker)) << "\", "; + OS << '\"' << getStringValue(*CheckerOpt, "CmdFlag") << "\", "; + OS << '\"'; + OS.write_escaped(getStringValue(*CheckerOpt, "Desc")) << "\", "; + OS << '\"'; + OS.write_escaped(getStringValue(*CheckerOpt, "DefaultVal")) << "\""; + OS << ")"; + OS << '\n'; + } + } + OS << "#endif // GET_CHECKER_OPTIONS\n" + "\n"; } } // end namespace clang Index: test/Analysis/invalid-checker-option.c =================================================================== --- /dev/null +++ test/Analysis/invalid-checker-option.c @@ -0,0 +1,19 @@ +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config RetainOneTwoThree:CheckOSObject=false \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER + +// Note that non-existent packages and checkers were always reported. + +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config-compatibility-mode=true \ +// RUN: -analyzer-config RetainOneTwoThree:CheckOSObject=false \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER + +// CHECK-NON-EXISTENT-CHECKER: (frontend): no analyzer checkers or packages +// CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree' + +// expected-no-diagnostics + +int main() {} Index: test/Analysis/disable-all-checks.c =================================================================== --- test/Analysis/disable-all-checks.c +++ test/Analysis/disable-all-checks.c @@ -12,7 +12,7 @@ // // expected-no-diagnostics -// CHECK: no analyzer checkers are associated with 'non.existant.Checker' +// CHECK: no analyzer checkers or packages are associated with 'non.existant.Checker' // CHECK: use -analyzer-disable-all-checks to disable all static analyzer checkers int buggy() { int x = 0; Index: test/Analysis/analyzer-config.c =================================================================== --- test/Analysis/analyzer-config.c +++ test/Analysis/analyzer-config.c @@ -3,6 +3,16 @@ // CHECK: [config] // CHECK-NEXT: aggressive-binary-operation-simplification = false +// CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = "" +// CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50 +// CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true +// CHECK-NEXT: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization = false +// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreGuardedFields = false +// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField = "" +// CHECK-NEXT: alpha.cplusplus.UninitializedObject:NotesAsWarnings = false +// CHECK-NEXT: alpha.cplusplus.UninitializedObject:Pedantic = false +// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04 +// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false // CHECK-NEXT: c++-allocator-inlining = true // CHECK-NEXT: c++-container-inlining = false @@ -18,9 +28,26 @@ // CHECK-NEXT: cfg-rich-constructors = true // CHECK-NEXT: cfg-scopes = false // CHECK-NEXT: cfg-temporary-dtors = true +// CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals // CHECK-NEXT: crosscheck-with-z3 = false // CHECK-NEXT: ctu-dir = "" // CHECK-NEXT: ctu-index-name = externalDefMap.txt +// CHECK-NEXT: debug.AnalysisOrder:* = false +// CHECK-NEXT: debug.AnalysisOrder:Bind = false +// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false +// CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false +// CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false +// CHECK-NEXT: debug.AnalysisOrder:PostCall = false +// CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false +// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false +// CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false +// CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = false +// CHECK-NEXT: debug.AnalysisOrder:PreCall = false +// CHECK-NEXT: debug.AnalysisOrder:PreStmtArraySubscriptExpr = false +// CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXNewExpr = false +// CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false +// CHECK-NEXT: debug.AnalysisOrder:PreStmtOffsetOfExpr = false +// CHECK-NEXT: debug.AnalysisOrder:RegionChanges = false // CHECK-NEXT: display-ctu-progress = false // CHECK-NEXT: eagerly-assume = true // CHECK-NEXT: elide-constructors = true @@ -40,7 +67,16 @@ // CHECK-NEXT: mode = deep // CHECK-NEXT: model-path = "" // CHECK-NEXT: notes-as-events = false +// CHECK-NEXT: nullability:NoDiagnoseCallsToSystemHeaders = false +// CHECK-NEXT: nullability:Optimistic = false // CHECK-NEXT: objc-inlining = true +// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false +// CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false +// CHECK-NEXT: optin.performance.Padding:AllowedPad = 24 +// CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false +// CHECK-NEXT: osx.cocoa.RetainCount:CheckOSObject = true +// CHECK-NEXT: osx.cocoa.RetainCount:TrackNSCFStartParam = false +// CHECK-NEXT: osx.cocoa.RetainCount:leak-diagnostics-reference-allocation = false // CHECK-NEXT: prune-paths = true // CHECK-NEXT: region-store-small-struct-limit = 2 // CHECK-NEXT: report-in-main-source-file = false @@ -49,7 +85,8 @@ // CHECK-NEXT: suppress-c++-stdlib = true // CHECK-NEXT: suppress-inlined-defensive-checks = true // CHECK-NEXT: suppress-null-return-paths = true +// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: widen-loops = false // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 49 +// CHECK-NEXT: num-entries = 86 Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -9,6 +9,7 @@ #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/LLVM.h" +#include "clang/Driver/DriverDiagnostic.h" #include "clang/Frontend/FrontendDiagnostic.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" @@ -18,6 +19,7 @@ #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/DynamicLibrary.h" +#include "llvm/Support/FormattedStream.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" #include <algorithm> @@ -43,6 +45,11 @@ return a.FullName < b.FullName; } +static bool packageNameLT(const CheckerRegistry::PackageInfo &a, + const CheckerRegistry::PackageInfo &b) { + return a.FullName < b.FullName; +} + static constexpr char PackageSeparator = '.'; static bool isInPackage(const CheckerRegistry::CheckerInfo &checker, @@ -83,9 +90,9 @@ // checker. size_t size = 1; llvm::StringMap<size_t>::const_iterator packageSize = - Packages.find(CmdLineArg); + PackageSizes.find(CmdLineArg); - if (packageSize != Packages.end()) + if (packageSize != PackageSizes.end()) size = packageSize->getValue(); return { it, it + size }; @@ -103,9 +110,16 @@ #define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI) \ addChecker(register##CLASS, shouldRegister##CLASS, FULLNAME, HELPTEXT, \ DOC_URI); + +#define GET_PACKAGES +#define PACKAGE(FULLNAME) \ + addPackage(FULLNAME); + #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER #undef GET_CHECKERS +#undef PACKAGE +#undef GET_PACKAGES // Register checkers from plugins. for (ArrayRef<std::string>::iterator i = plugins.begin(), e = plugins.end(); @@ -150,15 +164,28 @@ // Would it be better to name it '~experimental' or something else // that's ASCIIbetically last? llvm::sort(Checkers, checkerNameLT); + llvm::sort(Packages, packageNameLT); #define GET_CHECKER_DEPENDENCIES #define CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY) \ addDependency(FULLNAME, DEPENDENCY); +#define GET_CHECKER_OPTIONS +#define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL) \ + addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC); + +#define GET_PACKAGE_OPTIONS +#define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL) \ + addPackageOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC); + #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER_DEPENDENCY #undef GET_CHECKER_DEPENDENCIES +#undef CHECKER_OPTION +#undef GET_CHECKER_OPTIONS +#undef PACKAGE_OPTION +#undef GET_PACKAGE_OPTIONS // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the // command line. @@ -255,11 +282,78 @@ StringRef packageName, leafName; std::tie(packageName, leafName) = Name.rsplit(PackageSeparator); while (!leafName.empty()) { - Packages[packageName] += 1; + PackageSizes[packageName] += 1; std::tie(packageName, leafName) = packageName.rsplit(PackageSeparator); } } +namespace { +template <class CheckerOrPackageInfo> +struct SameFullName { + StringRef RequestedName; + + SameFullName(StringRef RequestedName) : RequestedName(RequestedName) {} + + bool operator()(const CheckerOrPackageInfo &CheckerOrPackage) { + return CheckerOrPackage.FullName == RequestedName; + } +}; + +using SameCheckerName = SameFullName<CheckerRegistry::CheckerInfo>; +using SamePackageName = SameFullName<CheckerRegistry::PackageInfo>; +} // end of anonymous namespace + +void CheckerRegistry::addDependency(StringRef fullName, StringRef dependency) { + auto CheckerIt = llvm::find_if(Checkers, SameCheckerName(fullName)); + assert(CheckerIt != Checkers.end() && + "Failed to find the checker while attempting to set up it's " + "dependencies!"); + + auto DependencyIt = llvm::find_if(Checkers, SameCheckerName(dependency)); + assert(DependencyIt != Checkers.end() && + "Failed to find the dependency of a checker!"); + + CheckerIt->Dependencies.push_back(&*DependencyIt); +} + +void CheckerRegistry::addCheckerOption(StringRef OptionType, + StringRef CheckerFullName, + StringRef OptionName, + StringRef DefaultValStr, + StringRef Description) { + + auto CheckerIt = llvm::find_if(Checkers, SameCheckerName(CheckerFullName)); + assert(CheckerIt != Checkers.end() && + "Failed to find the checker while attempting to add a command line " + "option to it!"); + + CheckerIt->CmdLineOptions.push_back( + {OptionType, OptionName, DefaultValStr, Description}); + + AnOpts.registerCheckerOption(CheckerFullName, OptionName, DefaultValStr); +} + +void CheckerRegistry::addPackage(StringRef FullName) { + Packages.push_back(FullName); +} + +void CheckerRegistry::addPackageOption(StringRef OptionType, + StringRef CheckerFullName, + StringRef OptionName, + StringRef DefaultValStr, + StringRef Description) { + + auto PackageIt = llvm::find_if(Packages, SamePackageName(CheckerFullName)); + assert(PackageIt != Packages.end() && + "Failed to find the package while attempting to add a command line " + "option to it!"); + + PackageIt->CmdLineOptions.push_back( + {OptionType, OptionName, DefaultValStr, Description}); + + AnOpts.registerPackageOption(CheckerFullName, OptionName, DefaultValStr); +} + void CheckerRegistry::initializeManager(CheckerManager &checkerMgr) const { // Collect checkers enabled by the options. CheckerInfoSet enabledCheckers = getEnabledCheckers(); @@ -277,21 +371,23 @@ if (pos == StringRef::npos) continue; - bool hasChecker = false; - StringRef checkerName = config.getKey().substr(0, pos); - for (const auto &checker : Checkers) { - if (checker.FullName.startswith(checkerName) && - (checker.FullName.size() == pos || checker.FullName[pos] == '.')) { - hasChecker = true; - break; - } - } - if (!hasChecker) - Diags.Report(diag::err_unknown_analyzer_checker) << checkerName; + StringRef SuppliedChecker(config.first().substr(0, pos)); + + auto CheckerIt = + llvm::find_if(Checkers, SameCheckerName(SuppliedChecker)); + if (CheckerIt != Checkers.end()) + continue; + + auto PackageIt = + llvm::find_if(Packages, SamePackageName(SuppliedChecker)); + if (PackageIt != Packages.end()) + continue; + + Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker; } } -void CheckerRegistry::printHelp(raw_ostream &out, +void CheckerRegistry::printCheckerWithDescList(raw_ostream &out, size_t maxNameChars) const { // FIXME: Print available packages. @@ -324,7 +420,7 @@ } } -void CheckerRegistry::printList(raw_ostream &out) const { +void CheckerRegistry::printEnabledCheckerList(raw_ostream &out) const { // Collect checkers enabled by the options. CheckerInfoSet enabledCheckers = getEnabledCheckers(); Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp +++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp @@ -50,7 +50,8 @@ out << "OVERVIEW: Clang Static Analyzer Checkers List\n\n"; out << "USAGE: -analyzer-checker <CHECKER or PACKAGE,...>\n\n"; - CheckerRegistry(plugins, diags, anopts, langOpts).printHelp(out); + CheckerRegistry(plugins, diags, anopts, langOpts) + .printCheckerWithDescList(out); } void ento::printEnabledCheckerList(raw_ostream &out, @@ -60,7 +61,8 @@ const LangOptions &langOpts) { out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n"; - CheckerRegistry(plugins, diags, anopts, langOpts).printList(out); + CheckerRegistry(plugins, diags, anopts, langOpts) + .printEnabledCheckerList(out); } void ento::printAnalyzerConfigList(raw_ostream &out) { Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp =================================================================== --- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -101,6 +101,20 @@ return *K >= Param; } +void AnalyzerOptions::registerCheckerOption(StringRef CheckerFullName, + StringRef OptionName, + StringRef DefaultVal) { + Config.insert({(Twine() + CheckerFullName + ":" + OptionName).str(), + DefaultVal}); +} + +void AnalyzerOptions::registerPackageOption(StringRef CheckerFullName, + StringRef OptionName, + StringRef DefaultVal) { + Config.insert({(Twine() + CheckerFullName + ":" + OptionName).str(), + DefaultVal}); +} + StringRef AnalyzerOptions::getCheckerStringOption(StringRef CheckerName, StringRef OptionName, StringRef DefaultVal, @@ -110,14 +124,17 @@ "bases!) if fully initialized before calling this function!"); ConfigTable::const_iterator E = Config.end(); + do { ConfigTable::const_iterator I = Config.find((Twine(CheckerName) + ":" + OptionName).str()); if (I != E) return StringRef(I->getValue()); + size_t Pos = CheckerName.rfind('.'); if (Pos == StringRef::npos) return DefaultVal; + CheckerName = CheckerName.substr(0, Pos); } while (!CheckerName.empty() && SearchInParents); return DefaultVal; @@ -163,7 +180,7 @@ bool HasFailed = getCheckerStringOption(CheckerName, OptionName, std::to_string(DefaultVal), SearchInParents) - .getAsInteger(10, Ret); + .getAsInteger(0, Ret); assert(!HasFailed && "analyzer-config option should be numeric"); (void)HasFailed; return Ret; Index: lib/Frontend/CompilerInvocation.cpp =================================================================== --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -422,7 +422,7 @@ OptionField = DefaultVal; bool HasFailed = getStringOption(Config, Name, std::to_string(DefaultVal)) - .getAsInteger(10, OptionField); + .getAsInteger(0, OptionField); if (Diags && HasFailed) Diags->Report(diag::err_analyzer_config_invalid_input) << Name << "an unsigned"; Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h =================================================================== --- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -92,6 +92,26 @@ using InitializationFunction = void (*)(CheckerManager &); using ShouldRegisterFunction = bool (*)(const LangOptions &); + /// Specifies a command line option. It may either belong to a checker or a + /// package. + struct CmdLineOption { + StringRef OptionType; + StringRef OptionName; + StringRef DefaultValStr; + StringRef Description; + + CmdLineOption(StringRef OptionType, StringRef OptionName, + StringRef DefaultValStr, StringRef Description) + : OptionType(OptionType), OptionName(OptionName), + DefaultValStr(DefaultValStr), Description(Description) { + + assert((OptionType == "bool" || OptionType == "string" || + OptionType == "int") && "Unknown command line option type!"); + } + }; + + using CmdLineOptionList = llvm::SmallVector<CmdLineOption, 0>; + struct CheckerInfo; using CheckerInfoList = std::vector<CheckerInfo>; @@ -99,6 +119,8 @@ using ConstCheckerInfoList = llvm::SmallVector<const CheckerInfo *, 0>; using CheckerInfoSet = llvm::SetVector<const CheckerInfo *>; + /// Specifies a checker. Note that this isn't what we call a checker object, + /// it merely contains everything required to create one. struct CheckerInfo { enum class StateFromCmdLine { // This checker wasn't explicitly enabled or disabled. @@ -114,6 +136,7 @@ StringRef FullName; StringRef Desc; StringRef DocumentationUri; + CmdLineOptionList CmdLineOptions; StateFromCmdLine State = StateFromCmdLine::State_Unspecified; ConstCheckerInfoList Dependencies; @@ -134,6 +157,17 @@ using StateFromCmdLine = CheckerInfo::StateFromCmdLine; + /// Specifies a package. Each package option is implicitly an option for all + /// checkers within the package. + struct PackageInfo { + StringRef FullName; + CmdLineOptionList CmdLineOptions; + + PackageInfo(StringRef FullName) : FullName(FullName) {} + }; + + using PackageInfoList = llvm::SmallVector<PackageInfo, 0>; + private: template <typename T> static void initializeManager(CheckerManager &mgr) { @@ -164,25 +198,40 @@ /// Makes the checker with the full name \p fullName depends on the checker /// called \p dependency. - void addDependency(StringRef fullName, StringRef dependency) { - auto CheckerThatNeedsDeps = - [&fullName](const CheckerInfo &Chk) { return Chk.FullName == fullName; }; - auto Dependency = - [&dependency](const CheckerInfo &Chk) { - return Chk.FullName == dependency; - }; - - auto CheckerIt = llvm::find_if(Checkers, CheckerThatNeedsDeps); - assert(CheckerIt != Checkers.end() && - "Failed to find the checker while attempting to set up it's " - "dependencies!"); - - auto DependencyIt = llvm::find_if(Checkers, Dependency); - assert(DependencyIt != Checkers.end() && - "Failed to find the dependency of a checker!"); - - CheckerIt->Dependencies.push_back(&*DependencyIt); - } + void addDependency(StringRef fullName, StringRef dependency); + + /// Registers an option to a given checker. A checker option will always have + /// the following format: + /// CheckerFullName:OptionName=Value + /// And can be specified from the command line like this: + /// -analyzer-config CheckerFullName:OptionName=Value + /// + /// Options for unknown checkers, or unknown options for a given checker, or + /// invalid value types for that given option are reported as an error in + /// non-compatibility mode. + void addCheckerOption(StringRef OptionType, + StringRef CheckerFullName, + StringRef OptionName, + StringRef DefaultValStr, + StringRef Description); + + /// Adds a package to the registry. + void addPackage(StringRef FullName); + + /// Registers an option to a given package. A package option will always have + /// the following format: + /// PackageFullName:OptionName=Value + /// And can be specified from the command line like this: + /// -analyzer-config PackageFullName:OptionName=Value + /// + /// Options for unknown packages, or unknown options for a given package, or + /// invalid value types for that given option are reported as an error in + /// non-compatibility mode. + void addPackageOption(StringRef OptionType, + StringRef PackageFullName, + StringRef OptionName, + StringRef DefaultValStr, + StringRef Description); // FIXME: This *really* should be added to the frontend flag descriptions. /// Initializes a CheckerManager by calling the initialization functions for @@ -196,8 +245,9 @@ /// Prints the name and description of all checkers in this registry. /// This output is not intended to be machine-parseable. - void printHelp(raw_ostream &out, size_t maxNameChars = 30) const; - void printList(raw_ostream &out) const; + void printCheckerWithDescList(raw_ostream &out, + size_t maxNameChars = 30) const; + void printEnabledCheckerList(raw_ostream &out) const; private: /// Collect all enabled checkers. The returned container preserves the order @@ -211,7 +261,10 @@ CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg); CheckerInfoList Checkers; - llvm::StringMap<size_t> Packages; + PackageInfoList Packages; + /// Used for couting how many checkers belong to a certain package in the + /// \c Checkers field. For convenience purposes. + llvm::StringMap<size_t> PackageSizes; DiagnosticsEngine &Diags; AnalyzerOptions &AnOpts; @@ -219,7 +272,6 @@ }; } // namespace ento - } // namespace clang #endif // LLVM_CLANG_STATICANALYZER_CORE_CHECKERREGISTRY_H Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h =================================================================== --- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -172,6 +172,7 @@ /// A key-value table of use-specified configuration values. // TODO: This shouldn't be public. ConfigTable Config; + AnalysisStores AnalysisStoreOpt = RegionStoreModel; AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel; AnalysisDiagClients AnalysisDiagOpt = PD_HTML; @@ -268,6 +269,18 @@ llvm::sort(AnalyzerConfigCmdFlags); } + /// Inserts a checker option with the specified default value. If the user + /// already specified this option when invoking the analyzer, nothing happens. + void registerCheckerOption(StringRef CheckerFullName, + StringRef OptionName, + StringRef DefaultValStr); + + /// Inserts a package option with the specified default value. If the user + /// already specified this option when invoking the analyzer, nothing happens. + void registerPackageOption(StringRef CheckerFullName, + StringRef OptionName, + StringRef DefaultValStr); + /// Interprets an option's string value as a boolean. The "true" string is /// interpreted as true and the "false" string is interpreted as false. /// Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -42,7 +42,18 @@ // development, but unwanted for developers who target only a single platform. def PortabilityOptIn : Package<"portability">, ParentPackage<OptIn>; -def Nullability : Package<"nullability">; +def Nullability : Package<"nullability">, + PackageOptions<[ + CmdLineOption<Boolean, + "Optimistic", + "In set to false, the checker assumes that it does not know " + "which functions might free the memory.", + "false">, + CmdLineOption<Boolean, + "NoDiagnoseCallsToSystemHeaders", + "", + "false"> + ]>; def Cplusplus : Package<"cplusplus">; def CplusplusAlpha : Package<"cplusplus">, ParentPackage<Alpha>; @@ -369,6 +380,13 @@ HelpText<"The base of several malloc() related checkers. On it's own it " "emits no reports, but adds valuable information to the analysis " "when enabled.">, + CheckerOptions<[ + CmdLineOption<Boolean, + "Optimistic", + "In set to false, the checker assumes that it does not know " + "which functions might free the memory.", + "false"> + ]>, Dependencies<[CStringModeling]>, Documentation<NotDocumented>; @@ -445,7 +463,28 @@ Documentation<NotDocumented>; def MoveChecker: Checker<"Move">, - HelpText<"Find use-after-move bugs in C++">, + HelpText<"Find use-after-move bugs in C++">, + CheckerOptions<[ + CmdLineOption<String, + "WarnOn", + "In non-aggressive mode, only warn on use-after-move of " + "local variables (or local rvalue references) and of STL " + "objects. The former is possible because local variables (or " + "local rvalue references) are not tempting their user to " + "re-use the storage. The latter is possible because STL " + "objects are known to end up in a valid but unspecified " + "state after the move and their state-reset methods are also " + "known, which allows us to predict precisely when " + "use-after-move is invalid. Some STL objects are known to " + "conform to additional contracts after move, so they are not " + "tracked. However, smart pointers specifically are tracked " + "because we can perform extra checking over them. In " + "aggressive mode, warn on any use-after-move because the " + "user has intentionally asked us to completely eliminate " + "use-after-move in his code. Values: \"KnownsOnly\", " + "\"KnownsAndLocals\", \"All\".", + "KnownsAndLocals"> + ]>, Documentation<HasDocumentation>; } // end: "cplusplus" @@ -454,6 +493,12 @@ def VirtualCallChecker : Checker<"VirtualCall">, HelpText<"Check virtual function calls during construction or destruction">, + CheckerOptions<[ + CmdLineOption<Boolean, + "PureOnly", + "Whether to only report calls to pure virtual methods.", + "false"> + ]>, Documentation<HasDocumentation>; } // end: "optin.cplusplus" @@ -490,7 +535,40 @@ Documentation<HasAlphaDocumentation>; def UninitializedObjectChecker: Checker<"UninitializedObject">, - HelpText<"Reports uninitialized fields after object construction">, + HelpText<"Reports uninitialized fields after object construction">, + CheckerOptions<[ + CmdLineOption<Boolean, + "Pedantic", + "If set to false, the checker won't emit warnings " + "for objects that don't have at least one initialized " + "field.", + "false">, + CmdLineOption<Boolean, + "NotesAsWarnings", + "If set to true, the checker will emit a warning " + "for each uninitalized field, as opposed to emitting one " + "warning per constructor call, and listing the uninitialized " + "fields that belongs to it in notes.", + "false">, + CmdLineOption<Boolean, + "CheckPointeeInitialization", + "If set to false, the checker will not analyze " + "the pointee of pointer/reference fields, and will only " + "check whether the object itself is initialized.", + "false">, + CmdLineOption<String, + "IgnoreRecordsWithField", + "If supplied, the checker will not analyze " + "structures that have a field with a name or type name that " + "matches the given pattern.", + "\"\"">, + CmdLineOption<Boolean, + "IgnoreGuardedFields", + "If set to true, the checker will analyze _syntactically_ " + "whether the found uninitialized object is used without a " + "preceding assert call. Defaults to false.", + "false"> + ]>, Documentation<HasAlphaDocumentation>; } // end: "alpha.cplusplus" @@ -551,6 +629,13 @@ def PaddingChecker : Checker<"Padding">, HelpText<"Check for excessively padded structs.">, + CheckerOptions<[ + CmdLineOption<Integer, + "AllowedPad", + "Reports are only generated if the excessive padding exceeds " + "'AllowedPad' in bytes.", + "24"> + ]>, Documentation<NotDocumented>; } // end: "padding" @@ -652,11 +737,18 @@ HelpText<"Check for overflows in the arguments to malloc()">, Documentation<HasAlphaDocumentation>; -// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, -// the defaults are correct for several common operating systems though, -// but may need to be overridden via the related analyzer-config flags. def MmapWriteExecChecker : Checker<"MmapWriteExec">, HelpText<"Warn on mmap() calls that are both writable and executable">, + CheckerOptions<[ + CmdLineOption<Integer, + "MmapProtExec", + "Specifies the value of PROT_EXEC", + "0x04">, + CmdLineOption<Integer, + "MmapProtRead", + "Specifies the value of PROT_READ", + "0x01"> + ]>, Documentation<HasAlphaDocumentation>; } // end "alpha.security" @@ -694,6 +786,14 @@ def NumberObjectConversionChecker : Checker<"NumberObjectConversion">, HelpText<"Check for erroneous conversions of objects representing numbers " "into numbers">, + CheckerOptions<[ + CmdLineOption<Boolean, + "Pedantic", + "Enables detection of more conversion patterns (which are " + "most likely more harmless, and therefore are more likely to " + "produce false positives).", + "false"> + ]>, Documentation<NotDocumented>; def MacOSXAPIChecker : Checker<"API">, @@ -780,6 +880,22 @@ def RetainCountChecker : Checker<"RetainCount">, HelpText<"Check for leaks and improper reference count management">, + CheckerOptions<[ + CmdLineOption<Boolean, + "leak-diagnostics-reference-allocation", + "Reports are only generated if the excessive padding exceeds " + "'AllowedPad' in bytes.", + "false">, + CmdLineOption<Boolean, + "CheckOSObject", + "Reports are only generated if the excessive padding exceeds " + "'AllowedPad' in bytes.", + "true">, + CmdLineOption<Boolean, + "TrackNSCFStartParam", + "", + "false"> + ]>, Dependencies<[RetainCountBase]>, Documentation<HasDocumentation>; @@ -880,6 +996,17 @@ def NonLocalizedStringChecker : Checker<"NonLocalizedStringChecker">, HelpText<"Warns about uses of non-localized NSStrings passed to UI methods " "expecting localized NSStrings">, + CheckerOptions<[ + CmdLineOption<Boolean, + "AggressiveReport", + "Marks a string being returned by any call as localized if " + "it is in LocStringFunctions (LSF) or the function is " + "annotated. Otherwise, we mark it as NonLocalized " + "(Aggressive) or NonLocalized only if it is not backed by a " + "SymRegion (Non-Aggressive), basically leaving only string " + "literals as NonLocalized.", + "false"> + ]>, Documentation<HasDocumentation>; def EmptyLocalizationContextChecker : @@ -938,6 +1065,72 @@ def AnalysisOrderChecker : Checker<"AnalysisOrder">, HelpText<"Print callbacks that are called during analysis in order">, + CheckerOptions<[ + CmdLineOption<Boolean, + "PreStmtCastExpr", + "", + "false">, + CmdLineOption<Boolean, + "PostStmtCastExpr", + "", + "false">, + CmdLineOption<Boolean, + "PreStmtArraySubscriptExpr", + "", + "false">, + CmdLineOption<Boolean, + "PostStmtArraySubscriptExpr", + "", + "false">, + CmdLineOption<Boolean, + "PreStmtCXXNewExpr", + "", + "false">, + CmdLineOption<Boolean, + "PostStmtCXXNewExpr", + "", + "false">, + CmdLineOption<Boolean, + "PreStmtOffsetOfExpr", + "", + "false">, + CmdLineOption<Boolean, + "PostStmtOffsetOfExpr", + "", + "false">, + CmdLineOption<Boolean, + "PreCall", + "", + "false">, + CmdLineOption<Boolean, + "PostCall", + "", + "false">, + CmdLineOption<Boolean, + "EndFunction", + "", + "false">, + CmdLineOption<Boolean, + "NewAllocator", + "", + "false">, + CmdLineOption<Boolean, + "Bind", + "", + "false">, + CmdLineOption<Boolean, + "LiveSymbols", + "", + "false">, + CmdLineOption<Boolean, + "RegionChanges", + "", + "false">, + CmdLineOption<Boolean, + "*", + "Enables all callbacks.", + "false"> + ]>, Documentation<NotDocumented>; def DominatorsTreeDumper : Checker<"DumpDominators">, @@ -1007,6 +1200,25 @@ def CloneChecker : Checker<"CloneChecker">, HelpText<"Reports similar pieces of code.">, + CheckerOptions<[ + CmdLineOption<Integer, + "MinimumCloneComplexity", + "Ensures that every clone has at least the given complexity. " + "Complexity is here defined as the total amount of children " + "of a statement. This constraint assumes the first statement " + "in the group is representative for all other statements in " + "the group in terms of complexity.", + "50">, + CmdLineOption<Boolean, + "ReportNormalClones", + "Report all clones, even less suspicious ones.", + "true">, + CmdLineOption<String, + "IgnoredFilesPattern", + "If supplied, the checker wont analyze files with a filename " + "that matches the given pattern.", + "\"\""> + ]>, Documentation<HasAlphaDocumentation>; } // end "clone" Index: include/clang/StaticAnalyzer/Checkers/CheckerBase.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/CheckerBase.td +++ include/clang/StaticAnalyzer/Checkers/CheckerBase.td @@ -10,14 +10,42 @@ // //===----------------------------------------------------------------------===// +/// Describes a checker or package option type. This is important for validating +/// user supplied inputs. +class CmdLineOptionTypeEnum<bits<2> val> { + bits<2> Type = val; +} +def Integer : CmdLineOptionTypeEnum<0>; +def String : CmdLineOptionTypeEnum<1>; +def Boolean : CmdLineOptionTypeEnum<2>; + +class Type<CmdLineOptionTypeEnum val> { + bits<2> Type = val.Type; +} + +/// Describes an option for a checker or a package. +class CmdLineOption<CmdLineOptionTypeEnum type, string cmdFlag, string desc, + string defaultVal> { + bits<2> Type = type.Type; + string CmdFlag = cmdFlag; + string Desc = desc; + string DefaultVal = defaultVal; +} + +/// Describes a list of package options. +class PackageOptions<list<CmdLineOption> opts> { + list<CmdLineOption> PackageOptions = opts; +} + /// Describes a package. Every checker is a part of a package, for example, /// 'NullDereference' is part of the 'core' package, hence it's full name is /// 'core.NullDereference'. /// Example: /// def Core : Package<"core">; class Package<string name> { - string PackageName = name; - Package ParentPackage; + string PackageName = name; + list<CmdLineOption> PackageOptions; + Package ParentPackage; } /// Describes a 'super' package that holds another package inside it. This is @@ -52,11 +80,17 @@ /// def DereferenceChecker : Checker<"NullDereference">, /// HelpText<"Check for dereferences of null pointers">; class Checker<string name = ""> { - string CheckerName = name; - string HelpText; - list<Checker> Dependencies; - bits<2> Documentation; - Package ParentPackage; + string CheckerName = name; + string HelpText; + list<CmdLineOption> CheckerOptions; + list<Checker> Dependencies; + bits<2> Documentation; + Package ParentPackage; +} + +/// Describes a list of checker options. +class CheckerOptions<list<CmdLineOption> opts> { + list<CmdLineOption> CheckerOptions = opts; } /// Describes dependencies in between checkers. For example, InnerPointerChecker Index: include/clang/Basic/DiagnosticCommonKinds.td =================================================================== --- include/clang/Basic/DiagnosticCommonKinds.td +++ include/clang/Basic/DiagnosticCommonKinds.td @@ -292,7 +292,7 @@ // Static Analyzer Core def err_unknown_analyzer_checker : Error< - "no analyzer checkers are associated with '%0'">; + "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">; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits