This revision was automatically updated to reflect the committed changes.
Closed by commit rC355704: [analyzer] Emit an error rather than assert on
invalid checker option input (authored by Szelethus, committed by ).
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57850/new/
https://reviews.llvm.org/D57850
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/StaticAnalyzer/Core/CheckerManager.h
lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
lib/StaticAnalyzer/Checkers/CloneChecker.cpp
lib/StaticAnalyzer/Checkers/MoveChecker.cpp
lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
lib/StaticAnalyzer/Core/CheckerManager.cpp
test/Analysis/copypaste/suspicious-clones.cpp
test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
test/Analysis/outofbound.c
test/Analysis/padding_c.c
test/Analysis/undef-buffers.c
test/Analysis/use-after-move.cpp
Index: include/clang/StaticAnalyzer/Core/CheckerManager.h
===================================================================
--- include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -136,6 +136,12 @@
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 ()>;
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_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">;
Index: test/Analysis/undef-buffers.c
===================================================================
--- test/Analysis/undef-buffers.c
+++ test/Analysis/undef-buffers.c
@@ -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);
Index: test/Analysis/padding_c.c
===================================================================
--- test/Analysis/padding_c.c
+++ test/Analysis/padding_c.c
@@ -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>
Index: test/Analysis/outofbound.c
===================================================================
--- test/Analysis/outofbound.c
+++ test/Analysis/outofbound.c
@@ -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);
Index: test/Analysis/copypaste/suspicious-clones.cpp
===================================================================
--- test/Analysis/copypaste/suspicious-clones.cpp
+++ test/Analysis/copypaste/suspicious-clones.cpp
@@ -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.
Index: test/Analysis/use-after-move.cpp
===================================================================
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -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();
Index: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
===================================================================
--- test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
+++ test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
@@ -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".
Index: lib/StaticAnalyzer/Core/CheckerManager.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -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 @@
#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..
//===----------------------------------------------------------------------===//
Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -1086,14 +1086,10 @@
}
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;
}
Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -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 @@
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) {
Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -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 @@
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 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) {
Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -27,6 +27,13 @@
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 @@
// 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 @@
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 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) {
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -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 @@
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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
