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, 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: lib/StaticAnalyzer/Checkers/CloneChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/CloneChecker.cpp +++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp @@ -195,17 +195,17 @@ auto *Checker = Mgr.registerChecker(); 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/LocalizationChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -1398,7 +1398,7 @@ mgr.registerChecker(); checker->IsAggressive = mgr.getAnalyzerOptions().getCheckerBooleanOption( - checker, "AggressiveReport", false); + checker, "AggressiveReport"); } bool ento::shouldRegisterNonLocalizedStringChecker(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(); checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption( - checker, "Optimistic", false); + checker, "Optimistic"); } bool ento::shouldRegisterDynamicMemoryModeling(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(); 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/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(); 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/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/NumberObjectConversionChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp +++ lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp @@ -346,7 +346,7 @@ NumberObjectConversionChecker *Chk = Mgr.registerChecker(); Chk->Pedantic = - Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic", false); + Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic"); } bool ento::shouldRegisterNumberObjectConversionChecker(const LangOptions &LO) { 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(); 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/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/VirtualCallChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -279,8 +279,7 @@ VirtualCallChecker *checker = mgr.registerChecker(); checker->IsPureOnly = - mgr.getAnalyzerOptions().getCheckerBooleanOption( - checker, "PureOnly", false); + mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "PureOnly"); } bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) { 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 SearchInParents) const { + auto Ret = llvm::StringSwitch>( 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/Frontend/CheckerRegistry.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -332,7 +332,9 @@ 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. + // 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(); @@ -342,6 +344,8 @@ Diags.Report(diag::err_analyzer_checker_option_invalid_input) << FullOption << "a boolean value"; } + + It.first->setValue(DefaultValue); } return; } @@ -354,6 +358,8 @@ Diags.Report(diag::err_analyzer_checker_option_invalid_input) << FullOption << "an integer value"; } + + It.first->setValue(DefaultValue); } return; } 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: 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