r355704 - [analyzer] Emit an error rather than assert on invalid checker option input
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 8 08:00:42 PST 2019
Author: szelethus
Date: Fri Mar 8 08:00:42 2019
New Revision: 355704
URL: http://llvm.org/viewvc/llvm-project?rev=355704&view=rev
Log:
[analyzer] Emit an error rather than assert on invalid checker option input
Asserting on invalid input isn't very nice, hence the patch to emit an error
instead.
This is the first of many patches to overhaul the way we handle checker options.
Differential Revision: https://reviews.llvm.org/D57850
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
cfe/trunk/test/Analysis/copypaste/suspicious-clones.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
cfe/trunk/test/Analysis/outofbound.c
cfe/trunk/test/Analysis/padding_c.c
cfe/trunk/test/Analysis/undef-buffers.c
cfe/trunk/test/Analysis/use-after-move.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Mar 8 08:00:42 2019
@@ -303,6 +303,8 @@ def err_analyzer_config_multiple_values
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_invalid_input : Error<
+ "invalid input for checker option '%0', that expects %1">;
def err_drv_invalid_hvx_length : Error<
"-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h Fri Mar 8 08:00:42 2019
@@ -136,6 +136,12 @@ public:
AnalyzerOptions &getAnalyzerOptions() { return AOptions; }
ASTContext &getASTContext() { return Context; }
+ /// Emits an error through a DiagnosticsEngine about an invalid user supplied
+ /// checker option value.
+ void reportInvalidCheckerOptionValue(const CheckerBase *C,
+ StringRef OptionName,
+ StringRef ExpectedValueDesc);
+
using CheckerRef = CheckerBase *;
using CheckerTag = const void *;
using CheckerDtor = CheckerFn<void ()>;
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Fri Mar 8 08:00:42 2019
@@ -1086,14 +1086,10 @@ bool ObjCDeallocChecker::isNibLoadedIvar
}
void ento::registerObjCDeallocChecker(CheckerManager &Mgr) {
- const LangOptions &LangOpts = Mgr.getLangOpts();
- // These checker only makes sense under MRR.
- if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount)
- return;
-
Mgr.registerChecker<ObjCDeallocChecker>();
}
bool ento::shouldRegisterObjCDeallocChecker(const LangOptions &LO) {
- return true;
+ // These checker only makes sense under MRR.
+ return LO.getGC() != LangOptions::GCOnly && !LO.ObjCAutoRefCount;
}
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp Fri Mar 8 08:00:42 2019
@@ -27,6 +27,13 @@ using namespace ento;
namespace {
class CloneChecker
: public Checker<check::ASTCodeBody, check::EndOfTranslationUnit> {
+public:
+ // Checker options.
+ int MinComplexity;
+ bool ReportNormalClones;
+ StringRef IgnoredFilesPattern;
+
+private:
mutable CloneDetector Detector;
mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
@@ -62,19 +69,6 @@ void CloneChecker::checkEndOfTranslation
// At this point, every statement in the translation unit has been analyzed by
// the CloneDetector. The only thing left to do is to report the found clones.
- int MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
- this, "MinimumCloneComplexity", 50);
- assert(MinComplexity >= 0);
-
- bool ReportSuspiciousClones = Mgr.getAnalyzerOptions()
- .getCheckerBooleanOption(this, "ReportSuspiciousClones", true);
-
- bool ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
- this, "ReportNormalClones", true);
-
- StringRef IgnoredFilesPattern = Mgr.getAnalyzerOptions()
- .getCheckerStringOption(this, "IgnoredFilesPattern", "");
-
// Let the CloneDetector create a list of clones from all the analyzed
// statements. We don't filter for matching variable patterns at this point
// because reportSuspiciousClones() wants to search them for errors.
@@ -86,8 +80,7 @@ void CloneChecker::checkEndOfTranslation
MinComplexityConstraint(MinComplexity),
RecursiveCloneTypeIIVerifyConstraint(), OnlyLargestCloneConstraint());
- if (ReportSuspiciousClones)
- reportSuspiciousClones(BR, Mgr, AllCloneGroups);
+ reportSuspiciousClones(BR, Mgr, AllCloneGroups);
// We are done for this translation unit unless we also need to report normal
// clones.
@@ -199,7 +192,20 @@ void CloneChecker::reportSuspiciousClone
//===----------------------------------------------------------------------===//
void ento::registerCloneChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<CloneChecker>();
+ auto *Checker = Mgr.registerChecker<CloneChecker>();
+
+ Checker->MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
+ Checker, "MinimumCloneComplexity", 50);
+
+ if (Checker->MinComplexity < 0)
+ Mgr.reportInvalidCheckerOptionValue(
+ Checker, "MinimumCloneComplexity", "a non-negative value");
+
+ Checker->ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ Checker, "ReportNormalClones", true);
+
+ Checker->IgnoredFilesPattern = Mgr.getAnalyzerOptions()
+ .getCheckerStringOption(Checker, "IgnoredFilesPattern", "");
}
bool ento::shouldRegisterCloneChecker(const LangOptions &LO) {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp Fri Mar 8 08:00:42 2019
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "clang/AST/ExprCXX.h"
+#include "clang/Driver/DriverDiagnostic.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -186,13 +187,17 @@ private:
AggressivenessKind Aggressiveness;
public:
- void setAggressiveness(StringRef Str) {
+ void setAggressiveness(StringRef Str, CheckerManager &Mgr) {
Aggressiveness =
llvm::StringSwitch<AggressivenessKind>(Str)
.Case("KnownsOnly", AK_KnownsOnly)
.Case("KnownsAndLocals", AK_KnownsAndLocals)
.Case("All", AK_All)
- .Default(AK_KnownsAndLocals); // A sane default.
+ .Default(AK_Invalid);
+
+ if (Aggressiveness == AK_Invalid)
+ Mgr.reportInvalidCheckerOptionValue(this, "WarnOn",
+ "either \"KnownsOnly\", \"KnownsAndLocals\" or \"All\" string value");
};
private:
@@ -735,7 +740,8 @@ void MoveChecker::printState(raw_ostream
void ento::registerMoveChecker(CheckerManager &mgr) {
MoveChecker *chk = mgr.registerChecker<MoveChecker>();
chk->setAggressiveness(
- mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn", ""));
+ mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn",
+ "KnownsAndLocals"), mgr);
}
bool ento::shouldRegisterMoveChecker(const LangOptions &LO) {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp Fri Mar 8 08:00:42 2019
@@ -16,6 +16,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/RecordLayout.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Driver/DriverDiagnostic.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -348,8 +349,9 @@ void ento::registerPaddingChecker(Checke
auto *Checker = Mgr.registerChecker<PaddingChecker>();
Checker->AllowedPad = Mgr.getAnalyzerOptions()
.getCheckerIntegerOption(Checker, "AllowedPad", 24);
- assert(Checker->AllowedPad >= 0 &&
- "AllowedPad option should be non-negative");
+ if (Checker->AllowedPad < 0)
+ Mgr.reportInvalidCheckerOptionValue(
+ Checker, "AllowedPad", "a non-negative value");
}
bool ento::shouldRegisterPaddingChecker(const LangOptions &LO) {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Fri Mar 8 08:00:42 2019
@@ -20,6 +20,7 @@
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "UninitializedObject.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Driver/DriverDiagnostic.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
@@ -618,10 +619,16 @@ void ento::registerUninitializedObjectCh
Chk, "CheckPointeeInitialization", /*DefaultVal*/ false);
ChOpts.IgnoredRecordsWithFieldPattern =
AnOpts.getCheckerStringOption(Chk, "IgnoreRecordsWithField",
- /*DefaultVal*/ "");
+ /*DefaultVal*/ "\"\"");
ChOpts.IgnoreGuardedFields =
AnOpts.getCheckerBooleanOption(Chk, "IgnoreGuardedFields",
/*DefaultVal*/ false);
+
+ std::string ErrorMsg;
+ if (!llvm::Regex(ChOpts.IgnoredRecordsWithFieldPattern).isValid(ErrorMsg))
+ Mgr.reportInvalidCheckerOptionValue(Chk, "IgnoreRecordsWithField",
+ "a valid regex, building failed with error message "
+ "\"" + ErrorMsg + "\"");
}
bool ento::shouldRegisterUninitializedObjectChecker(const LangOptions &LO) {
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp Fri Mar 8 08:00:42 2019
@@ -15,6 +15,7 @@
#include "clang/AST/Stmt.h"
#include "clang/Analysis/ProgramPoint.h"
#include "clang/Basic/LLVM.h"
+#include "clang/Driver/DriverDiagnostic.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
@@ -58,6 +59,15 @@ void CheckerManager::finishedCheckerRegi
#endif
}
+void CheckerManager::reportInvalidCheckerOptionValue(
+ const CheckerBase *C, StringRef OptionName, StringRef ExpectedValueDesc) {
+
+ Context.getDiagnostics()
+ .Report(diag::err_analyzer_checker_option_invalid_input)
+ << (llvm::Twine() + C->getTagDescription() + ":" + OptionName).str()
+ << ExpectedValueDesc;
+}
+
//===----------------------------------------------------------------------===//
// Functions for running checkers for AST traversing..
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/test/Analysis/copypaste/suspicious-clones.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/copypaste/suspicious-clones.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/copypaste/suspicious-clones.cpp (original)
+++ cfe/trunk/test/Analysis/copypaste/suspicious-clones.cpp Fri Mar 8 08:00:42 2019
@@ -1,4 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:ReportSuspiciousClones=true -analyzer-config alpha.clone.CloneChecker:ReportNormalClones=false -analyzer-config alpha.clone.CloneChecker:MinimumCloneComplexity=10 -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=alpha.clone.CloneChecker \
+// RUN: -analyzer-config alpha.clone.CloneChecker:ReportNormalClones=false \
+// RUN: -analyzer-config alpha.clone.CloneChecker:MinimumCloneComplexity=10
// Tests finding a suspicious clone that references local variables.
Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp Fri Mar 8 08:00:42 2019
@@ -3,6 +3,20 @@
// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind" \
// RUN: -std=c++11 -verify %s
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=alpha.cplusplus.UninitializedObject \
+// RUN: -analyzer-config \
+// RUN: alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="([)]" \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-UNINIT-INVALID-REGEX
+
+// CHECK-UNINIT-INVALID-REGEX: (frontend): invalid input for checker option
+// CHECK-UNINIT-INVALID-REGEX-SAME: 'alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField',
+// CHECK-UNINIT-INVALID-REGEX-SAME: that expects a valid regex, building failed
+// CHECK-UNINIT-INVALID-REGEX-SAME: with error message "parentheses not
+// CHECK-UNINIT-INVALID-REGEX-SAME: balanced"
+
+
// expected-no-diagnostics
// Both type and name contains "kind".
Modified: cfe/trunk/test/Analysis/outofbound.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/outofbound.c?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/outofbound.c (original)
+++ cfe/trunk/test/Analysis/outofbound.c Fri Mar 8 08:00:42 2019
@@ -2,7 +2,7 @@
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=unix \
// RUN: -analyzer-checker=alpha.security.ArrayBound \
-// RUN: -analyzer-config unix:Optimistic=true
+// RUN: -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
Modified: cfe/trunk/test/Analysis/padding_c.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/padding_c.c?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/padding_c.c (original)
+++ cfe/trunk/test/Analysis/padding_c.c Fri Mar 8 08:00:42 2019
@@ -1,4 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=optin.performance \
+// RUN: -analyzer-config optin.performance.Padding:AllowedPad=2
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=optin.performance.Padding \
+// RUN: -analyzer-config optin.performance.Padding:AllowedPad=-10 \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-PAD-NEGATIVE-VALUE
+
+// CHECK-PAD-NEGATIVE-VALUE: (frontend): invalid input for checker option
+// CHECK-PAD-NEGATIVE-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-PAD-NEGATIVE-VALUE-SAME: expects a non-negative value
#if __has_include(<stdalign.h>)
#include <stdalign.h>
Modified: cfe/trunk/test/Analysis/undef-buffers.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/undef-buffers.c?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/undef-buffers.c (original)
+++ cfe/trunk/test/Analysis/undef-buffers.c Fri Mar 8 08:00:42 2019
@@ -2,7 +2,7 @@
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=unix \
// RUN: -analyzer-checker=core.uninitialized \
-// RUN: -analyzer-config unix:Optimistic=true
+// RUN: -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
Modified: cfe/trunk/test/Analysis/use-after-move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/use-after-move.cpp?rev=355704&r1=355703&r2=355704&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/use-after-move.cpp (original)
+++ cfe/trunk/test/Analysis/use-after-move.cpp Fri Mar 8 08:00:42 2019
@@ -27,6 +27,17 @@
// RUN: -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
// RUN: -analyzer-checker debug.ExprInspection
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.Move \
+// RUN: -analyzer-config cplusplus.Move:WarnOn="a bunch of things" \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-MOVE-INVALID-VALUE
+
+// CHECK-MOVE-INVALID-VALUE: (frontend): invalid input for checker option
+// CHECK-MOVE-INVALID-VALUE-SAME: 'cplusplus.Move:WarnOn', that expects either
+// CHECK-MOVE-INVALID-VALUE-SAME: "KnownsOnly", "KnownsAndLocals" or "All"
+// CHECK-MOVE-INVALID-VALUE-SAME: string value
+
#include "Inputs/system-header-simulator-cxx.h"
void clang_analyzer_warnIfReached();
More information about the cfe-commits
mailing list