Szelethus updated this revision to Diff 190023.
Szelethus added a comment.
Herald added subscribers: Charusso, jdoerfert.
Refactored the entire patch, NFC compared to the earlier diff. This was needed
as in a followup revision, much of the code would be rewritten.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57860/new/
https://reviews.llvm.org/D57860
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
test/Analysis/invalid-checker-option.c
Index: test/Analysis/invalid-checker-option.c
===================================================================
--- test/Analysis/invalid-checker-option.c
+++ test/Analysis/invalid-checker-option.c
@@ -14,6 +14,53 @@
// CHECK-NON-EXISTENT-CHECKER: (frontend): no analyzer checkers or packages
// CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
+
+// Every other error should be avoidable in compatiblity mode.
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config debug.AnalysisOrder:Everything=false \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'debug.AnalysisOrder'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Everything'
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config debug.AnalysisOrder:Everything=false
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config debug.AnalysisOrder:*=nothankyou \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'debug.AnalysisOrder:*', that expects a
+// CHECK-INVALID-BOOL-VALUE-SAME: boolean value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config debug.AnalysisOrder:*=nothankyou
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config optin.performance.Padding:AllowedPad=surpriseme \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-INT-VALUE
+
+// CHECK-INVALID-INT-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-INT-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-INVALID-INT-VALUE-SAME: expects an integer value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config optin.performance.Padding:AllowedPad=surpriseme
+
// expected-no-diagnostics
int main() {}
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -314,6 +314,51 @@
CheckerIt->Dependencies.emplace_back(&*DependencyIt);
}
+/// Insert the checker/package option to AnalyzerOptions' config table, and
+/// validate it, if the user supplied it on the command line.
+static void insertAndValidate(StringRef OptionType, StringRef FullName,
+ StringRef OptionName, StringRef DefaultValue,
+ AnalyzerOptions &AnOpts,
+ DiagnosticsEngine &Diags) {
+
+ std::string FullOption =
+ (llvm::Twine() + FullName + ":" + OptionName).str();
+
+ auto It = AnOpts.Config.insert({FullOption, DefaultValue});
+
+ // Insertation was successful -- CmdLineOption's constructor will validate
+ // whether values received from plugins or TableGen files are correct.
+ if (It.second)
+ return;
+
+ // Insertation failed, the user supplied this package/checker option on the
+ // command line. If the supplied value is invalid, we'll emit an error.
+
+ StringRef SuppliedValue = It.first->getValue();
+
+ if (OptionType == "bool") {
+ if (SuppliedValue != "true" && SuppliedValue != "false") {
+ if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
+ Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+ << FullOption << "a boolean value";
+ }
+ }
+ return;
+ }
+
+ if (OptionType == "int") {
+ int Tmp;
+ bool HasFailed = SuppliedValue.getAsInteger(0, Tmp);
+ if (HasFailed) {
+ if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
+ Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+ << FullOption << "an integer value";
+ }
+ }
+ return;
+ }
+}
+
void CheckerRegistry::addCheckerOption(StringRef OptionType,
StringRef CheckerFullName,
StringRef OptionName,
@@ -328,8 +373,8 @@
CheckerIt->CmdLineOptions.emplace_back(
OptionType, OptionName, DefaultValStr, Description);
- AnOpts.Config.insert({(Twine() + CheckerFullName + ":" + OptionName).str(),
- DefaultValStr});
+ insertAndValidate(
+ OptionType, CheckerFullName, OptionName, DefaultValStr, AnOpts, Diags);
}
void CheckerRegistry::addPackage(StringRef FullName) {
@@ -350,8 +395,8 @@
PackageIt->CmdLineOptions.emplace_back(
OptionType, OptionName, DefaultValStr, Description);
- AnOpts.Config.insert({(Twine() + FullName + ":" + OptionName).str(),
- DefaultValStr});
+ insertAndValidate(
+ OptionType, FullName, OptionName, DefaultValStr, AnOpts, Diags);
}
void CheckerRegistry::initializeManager(CheckerManager &checkerMgr) const {
@@ -365,22 +410,52 @@
}
}
+static void isOptionContainedIn(
+ const CheckerRegistry::CmdLineOptionList &OptionList,
+ StringRef SuppliedChecker, StringRef SuppliedOption,
+ const AnalyzerOptions &AnOpts, DiagnosticsEngine &Diags) {
+
+ if (!AnOpts.ShouldEmitErrorsOnInvalidConfigValue)
+ return;
+
+ using CmdLineOption = CheckerRegistry::CmdLineOption;
+
+ auto SameOptName = [SuppliedOption](const CmdLineOption &Opt) {
+ return Opt.OptionName == SuppliedOption;
+ };
+
+ auto OptionIt = llvm::find_if(OptionList, SameOptName);
+
+ if (OptionIt == OptionList.end()) {
+ Diags.Report(diag::err_analyzer_checker_option_unknown)
+ << SuppliedChecker << SuppliedOption;
+ return;
+ }
+}
+
void CheckerRegistry::validateCheckerOptions() const {
- for (const auto &config : AnOpts.Config) {
- size_t pos = config.getKey().find(':');
- if (pos == StringRef::npos)
+ for (const auto &Config : AnOpts.Config) {
+ size_t Pos = Config.getKey().find(':');
+ if (Pos == StringRef::npos)
continue;
- StringRef SuppliedChecker(config.first().substr(0, pos));
+ StringRef SuppliedChecker(Config.first().substr(0, Pos));
+ StringRef SuppliedOption(Config.first().drop_front(Pos + 1));
// We can't get away with binaryFind here, as it uses lower_bound.
auto CheckerIt = llvm::find(Checkers, CheckerInfo(SuppliedChecker));
- if (CheckerIt != Checkers.end())
+ if (CheckerIt != Checkers.end()) {
+ isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedChecker,
+ SuppliedOption, AnOpts, Diags);
continue;
+ }
auto PackageIt = llvm::find(Packages, PackageInfo(SuppliedChecker));
- if (PackageIt != Packages.end())
+ if (PackageIt != Packages.end()) {
+ isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedChecker,
+ SuppliedOption, AnOpts, Diags);
continue;
+ }
Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker;
}
Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -107,6 +107,17 @@
assert((OptionType == "bool" || OptionType == "string" ||
OptionType == "int") && "Unknown command line option type!");
+
+ assert((OptionType != "bool" ||
+ (DefaultValStr == "true" || DefaultValStr == "false")) &&
+ "Invalid value for boolean command line option! Maybe incorrect "
+ "parameters to the addCheckerOption or addPackageOption method?");
+
+ int Tmp;
+ assert((OptionType != "int" || !DefaultValStr.getAsInteger(0, Tmp)) &&
+ "Invalid value for integer command line option! Maybe incorrect "
+ "parameters to the addCheckerOption or addPackageOption method?");
+ (void)Tmp;
}
};
Index: include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -303,6 +303,8 @@
def err_analyzer_config_invalid_input : Error<
"invalid input for analyzer-config option '%0', that expects %1 value">;
def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">;
+def err_analyzer_checker_option_unknown : Error<
+ "checker '%0' has no option called '%1'">;
def err_analyzer_checker_option_invalid_input : Error<
"invalid input for checker option '%0', that expects %1">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
