Szelethus updated this revision to Diff 190993.
Szelethus added a comment.
Rebase.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59195/new/
https://reviews.llvm.org/D59195
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
lib/StaticAnalyzer/Checkers/CloneChecker.cpp
lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
lib/StaticAnalyzer/Checkers/MoveChecker.cpp
lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
test/Analysis/invalid-checker-option.c
unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
Index: unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
===================================================================
--- unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
+++ unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
@@ -51,25 +51,24 @@
// CheckerTwo one has Option specified as true. It should read true regardless
// of search mode.
CheckerOneMock CheckerOne;
- EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option", false));
+ EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option"));
// The package option is overridden with a checker option.
- EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option", false,
- true));
+ EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option", true));
// The Outer package option is overridden by the Inner package option. No
// package option is specified.
- EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option2", false,
- true));
- // No package option is specified and search in packages is turned off. The
- // default value should be returned.
- EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerOne, "Option2", false));
EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option2", true));
+ // No package option is specified and search in packages is turned off. We
+ // should assert here, but we can't test that.
+ //Opts.getCheckerBooleanOption(&CheckerOne, "Option2");
+ //Opts.getCheckerBooleanOption(&CheckerOne, "Option2");
- // Checker true has no option specified. It should get the default value when
- // search in parents turned off and false when search in parents turned on.
+ // Checker true has no option specified. It should get false when search in
+ // parents turned on.
CheckerTwoMock CheckerTwo;
- EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", false));
- EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", true));
- EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", true, true));
+ EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", true));
+ // In any other case, we should assert, that we cannot test unfortunately.
+ //Opts.getCheckerBooleanOption(&CheckerTwo, "Option");
+ //Opts.getCheckerBooleanOption(&CheckerTwo, "Option");
}
TEST(StaticAnalyzerOptions, StringOptions) {
@@ -84,16 +83,14 @@
CheckerOneMock CheckerOne;
EXPECT_TRUE("StringValue" ==
- Opts.getCheckerStringOption(&CheckerOne, "Option", "DefaultValue"));
- EXPECT_TRUE("DefaultValue" ==
- Opts.getCheckerStringOption(&CheckerOne, "Option2", "DefaultValue"));
+ Opts.getCheckerStringOption(&CheckerOne, "Option"));
}
TEST(StaticAnalyzerOptions, SubCheckerOptions) {
AnalyzerOptions Opts;
Opts.Config["Outer.Inner.CheckerOne:Option"] = "StringValue";
EXPECT_TRUE("StringValue" == Opts.getCheckerStringOption(
- "Outer.Inner.CheckerOne", "Option", "DefaultValue"));
+ "Outer.Inner.CheckerOne", "Option"));
}
} // end namespace ento
Index: test/Analysis/invalid-checker-option.c
===================================================================
--- test/Analysis/invalid-checker-option.c
+++ test/Analysis/invalid-checker-option.c
@@ -15,6 +15,27 @@
// CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=debug.ConfigDumper \
+// RUN: -analyzer-checker=debug.AnalysisOrder \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config debug.AnalysisOrder:*=yesplease \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-CORRECTED-BOOL-VALUE
+
+// CHECK-CORRECTED-BOOL-VALUE: debug.AnalysisOrder:* = false
+//
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=debug.ConfigDumper \
+// RUN: -analyzer-checker=optin.performance.Padding \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config optin.performance.Padding:AllowedPad=surpriseme \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-CORRECTED-INT-VALUE
+
+// CHECK-CORRECTED-INT-VALUE: optin.performance.Padding:AllowedPad = 24
+
+
// Every other error should be avoidable in compatiblity mode.
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -326,7 +326,9 @@
return;
// Insertion failed, the user supplied this package/checker option on the
- // command line. If the supplied value is invalid, we'll emit an error.
+ // command line. If the supplied value is invalid, we'll restore the option
+ // to it's default value, and if we're in non-compatibility mode, we'll also
+ // emit an error.
StringRef SuppliedValue = It.first->getValue();
@@ -336,6 +338,8 @@
Diags.Report(diag::err_analyzer_checker_option_invalid_input)
<< FullOption << "a boolean value";
}
+
+ It.first->setValue(Option.DefaultValStr);
}
return;
}
@@ -348,6 +352,8 @@
Diags.Report(diag::err_analyzer_checker_option_invalid_input)
<< FullOption << "an integer value";
}
+
+ It.first->setValue(Option.DefaultValStr);
}
return;
}
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -103,8 +103,7 @@
StringRef AnalyzerOptions::getCheckerStringOption(StringRef CheckerName,
StringRef OptionName,
- StringRef DefaultVal,
- bool SearchInParents ) const {
+ bool SearchInParents) const {
assert(!CheckerName.empty() &&
"Empty checker name! Make sure the checker object (including it's "
"bases!) if fully initialized before calling this function!");
@@ -117,62 +116,66 @@
return StringRef(I->getValue());
size_t Pos = CheckerName.rfind('.');
if (Pos == StringRef::npos)
- return DefaultVal;
+ break;
+
CheckerName = CheckerName.substr(0, Pos);
} while (!CheckerName.empty() && SearchInParents);
- return DefaultVal;
+
+ llvm_unreachable("Unknown checker option! Did you call getChecker*Option "
+ "with incorrect parameters? User input must've been "
+ "verified by CheckerRegistry.");
+
+ return "";
}
StringRef AnalyzerOptions::getCheckerStringOption(const ento::CheckerBase *C,
StringRef OptionName,
- StringRef DefaultVal,
- bool SearchInParents ) const {
+ bool SearchInParents) const {
return getCheckerStringOption(
- C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+ C->getTagDescription(), OptionName, SearchInParents);
}
bool AnalyzerOptions::getCheckerBooleanOption(StringRef CheckerName,
StringRef OptionName,
- bool DefaultVal,
- bool SearchInParents ) const {
- // FIXME: We should emit a warning here if the value is something other than
- // "true", "false", or the empty string (meaning the default value),
- // but the AnalyzerOptions doesn't have access to a diagnostic engine.
- return llvm::StringSwitch<bool>(
+ bool SearchInParents) const {
+ auto Ret = llvm::StringSwitch<llvm::Optional<bool>>(
getCheckerStringOption(CheckerName, OptionName,
- DefaultVal ? "true" : "false",
SearchInParents))
.Case("true", true)
.Case("false", false)
- .Default(DefaultVal);
+ .Default(None);
+
+ assert(Ret &&
+ "This option should be either 'true' or 'false', and should've been "
+ "validated by CheckerRegisrty!");
+
+ return *Ret;
}
bool AnalyzerOptions::getCheckerBooleanOption(const ento::CheckerBase *C,
StringRef OptionName,
- bool DefaultVal,
- bool SearchInParents ) const {
+ bool SearchInParents) const {
return getCheckerBooleanOption(
- C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+ C->getTagDescription(), OptionName, SearchInParents);
}
int AnalyzerOptions::getCheckerIntegerOption(StringRef CheckerName,
StringRef OptionName,
- int DefaultVal,
- bool SearchInParents ) const {
- int Ret = DefaultVal;
+ bool SearchInParents) const {
+ int Ret = 0;
bool HasFailed = getCheckerStringOption(CheckerName, OptionName,
- std::to_string(DefaultVal),
SearchInParents)
.getAsInteger(0, Ret);
- assert(!HasFailed && "analyzer-config option should be numeric");
+ assert(!HasFailed &&
+ "This option should be numeric, and should've been validated by "
+ "CheckerRegisrty!");
(void)HasFailed;
return Ret;
}
int AnalyzerOptions::getCheckerIntegerOption(const ento::CheckerBase *C,
StringRef OptionName,
- int DefaultVal,
- bool SearchInParents ) const {
+ bool SearchInParents) const {
return getCheckerIntegerOption(
- C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+ C->getTagDescription(), OptionName, SearchInParents);
}
Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -279,8 +279,7 @@
VirtualCallChecker *checker = mgr.registerChecker<VirtualCallChecker>();
checker->IsPureOnly =
- mgr.getAnalyzerOptions().getCheckerBooleanOption(
- checker, "PureOnly", false);
+ mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "PureOnly");
}
bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -611,18 +611,15 @@
AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
UninitObjCheckerOptions &ChOpts = Chk->Opts;
- ChOpts.IsPedantic =
- AnOpts.getCheckerBooleanOption(Chk, "Pedantic", /*DefaultVal*/ false);
+ ChOpts.IsPedantic = AnOpts.getCheckerBooleanOption(Chk, "Pedantic");
ChOpts.ShouldConvertNotesToWarnings = AnOpts.getCheckerBooleanOption(
- Chk, "NotesAsWarnings", /*DefaultVal*/ false);
+ Chk, "NotesAsWarnings");
ChOpts.CheckPointeeInitialization = AnOpts.getCheckerBooleanOption(
- Chk, "CheckPointeeInitialization", /*DefaultVal*/ false);
+ Chk, "CheckPointeeInitialization");
ChOpts.IgnoredRecordsWithFieldPattern =
- AnOpts.getCheckerStringOption(Chk, "IgnoreRecordsWithField",
- /*DefaultVal*/ "\"\"");
+ AnOpts.getCheckerStringOption(Chk, "IgnoreRecordsWithField");
ChOpts.IgnoreGuardedFields =
- AnOpts.getCheckerBooleanOption(Chk, "IgnoreGuardedFields",
- /*DefaultVal*/ false);
+ AnOpts.getCheckerBooleanOption(Chk, "IgnoreGuardedFields");
std::string ErrorMsg;
if (!llvm::Regex(ChOpts.IgnoredRecordsWithFieldPattern).isValid(ErrorMsg))
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -348,7 +348,7 @@
void ento::registerPaddingChecker(CheckerManager &Mgr) {
auto *Checker = Mgr.registerChecker<PaddingChecker>();
Checker->AllowedPad = Mgr.getAnalyzerOptions()
- .getCheckerIntegerOption(Checker, "AllowedPad", 24);
+ .getCheckerIntegerOption(Checker, "AllowedPad");
if (Checker->AllowedPad < 0)
Mgr.reportInvalidCheckerOptionValue(
Checker, "AllowedPad", "a non-negative value");
Index: lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
@@ -346,7 +346,7 @@
NumberObjectConversionChecker *Chk =
Mgr.registerChecker<NumberObjectConversionChecker>();
Chk->Pedantic =
- Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic", false);
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic");
}
bool ento::shouldRegisterNumberObjectConversionChecker(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -1208,7 +1208,7 @@
checker->NoDiagnoseCallsToSystemHeaders = \
checker->NoDiagnoseCallsToSystemHeaders || \
mgr.getAnalyzerOptions().getCheckerBooleanOption( \
- checker, "NoDiagnoseCallsToSystemHeaders", false, true); \
+ checker, "NoDiagnoseCallsToSystemHeaders", true); \
} \
\
bool ento::shouldRegister##name##Checker(const LangOptions &LO) { \
Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -740,8 +740,7 @@
void ento::registerMoveChecker(CheckerManager &mgr) {
MoveChecker *chk = mgr.registerChecker<MoveChecker>();
chk->setAggressiveness(
- mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn",
- "KnownsAndLocals"), mgr);
+ mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn"), mgr);
}
bool ento::shouldRegisterMoveChecker(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -82,10 +82,10 @@
mgr.registerChecker<MmapWriteExecChecker>();
Mwec->ProtExecOv =
mgr.getAnalyzerOptions()
- .getCheckerIntegerOption(Mwec, "MmapProtExec", 0x04);
+ .getCheckerIntegerOption(Mwec, "MmapProtExec");
Mwec->ProtReadOv =
mgr.getAnalyzerOptions()
- .getCheckerIntegerOption(Mwec, "MmapProtRead", 0x01);
+ .getCheckerIntegerOption(Mwec, "MmapProtRead");
}
bool ento::shouldRegisterMmapWriteExecChecker(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3098,7 +3098,7 @@
void ento::registerDynamicMemoryModeling(CheckerManager &mgr) {
auto *checker = mgr.registerChecker<MallocChecker>();
checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
- checker, "Optimistic", false);
+ checker, "Optimistic");
}
bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
+++ lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
@@ -1398,7 +1398,7 @@
mgr.registerChecker<NonLocalizedStringChecker>();
checker->IsAggressive =
mgr.getAnalyzerOptions().getCheckerBooleanOption(
- checker, "AggressiveReport", false);
+ checker, "AggressiveReport");
}
bool ento::shouldRegisterNonLocalizedStringChecker(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -195,17 +195,17 @@
auto *Checker = Mgr.registerChecker<CloneChecker>();
Checker->MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
- Checker, "MinimumCloneComplexity", 50);
+ Checker, "MinimumCloneComplexity");
if (Checker->MinComplexity < 0)
Mgr.reportInvalidCheckerOptionValue(
Checker, "MinimumCloneComplexity", "a non-negative value");
Checker->ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
- Checker, "ReportNormalClones", true);
+ Checker, "ReportNormalClones");
Checker->IgnoredFilesPattern = Mgr.getAnalyzerOptions()
- .getCheckerStringOption(Checker, "IgnoredFilesPattern", "");
+ .getCheckerStringOption(Checker, "IgnoredFilesPattern");
}
bool ento::shouldRegisterCloneChecker(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -44,8 +44,8 @@
check::LiveSymbols> {
bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
- return Opts.getCheckerBooleanOption(this, "*", false) ||
- Opts.getCheckerBooleanOption(this, CallbackName, false);
+ return Opts.getCheckerBooleanOption(this, "*") ||
+ Opts.getCheckerBooleanOption(this, CallbackName);
}
bool isCallbackEnabled(CheckerContext &C, StringRef CallbackName) const {
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===================================================================
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -279,18 +279,14 @@
/// Checker options are retrieved in the following format:
/// `-analyzer-config CheckerName:OptionName=Value.
/// @param [in] OptionName Name for option to retrieve.
- /// @param [in] DefaultVal Default value returned if no such option was
- /// specified.
/// @param [in] SearchInParents If set to true and the searched option was not
/// specified for the given checker the options for the parent packages will
/// be searched as well. The inner packages take precedence over the outer
/// ones.
bool getCheckerBooleanOption(StringRef CheckerName, StringRef OptionName,
- bool DefaultVal,
bool SearchInParents = false) const;
bool getCheckerBooleanOption(const ento::CheckerBase *C, StringRef OptionName,
- bool DefaultVal,
bool SearchInParents = false) const;
/// Interprets an option's string value as an integer value.
@@ -303,18 +299,14 @@
/// Checker options are retrieved in the following format:
/// `-analyzer-config CheckerName:OptionName=Value.
/// @param [in] OptionName Name for option to retrieve.
- /// @param [in] DefaultVal Default value returned if no such option was
- /// specified.
/// @param [in] SearchInParents If set to true and the searched option was not
/// specified for the given checker the options for the parent packages will
/// be searched as well. The inner packages take precedence over the outer
/// ones.
int getCheckerIntegerOption(StringRef CheckerName, StringRef OptionName,
- int DefaultVal,
bool SearchInParents = false) const;
int getCheckerIntegerOption(const ento::CheckerBase *C, StringRef OptionName,
- int DefaultVal,
bool SearchInParents = false) const;
/// Query an option's string value.
@@ -327,18 +319,15 @@
/// Checker options are retrieved in the following format:
/// `-analyzer-config CheckerName:OptionName=Value.
/// @param [in] OptionName Name for option to retrieve.
- /// @param [in] DefaultVal Default value returned if no such option was
- /// specified.
/// @param [in] SearchInParents If set to true and the searched option was not
/// specified for the given checker the options for the parent packages will
/// be searched as well. The inner packages take precedence over the outer
/// ones.
StringRef getCheckerStringOption(StringRef CheckerName, StringRef OptionName,
- StringRef DefaultVal,
bool SearchInParents = false) const;
StringRef getCheckerStringOption(const ento::CheckerBase *C,
- StringRef OptionName, StringRef DefaultVal,
+ StringRef OptionName,
bool SearchInParents = false) const;
/// Retrieves and sets the UserMode. This is a high-level option,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
