Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs,
baloghadamsoftware.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, szepet, whisperity.
Herald added a reviewer: teemperor.
Herald added a project: clang.
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, and I admit that it's a kind of "things that didn't fit anywhere else"
patch.
Repository:
rC Clang
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
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: 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/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/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"; please supply a valid value
+
+
// expected-no-diagnostics
// Both type and name contains "kind".
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: 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,18 @@
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.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+ << (llvm::Twine() + Chk->getTagDescription()
+ + ":IgnoreRecordsWithField").str()
+ << "a valid regex, building failed with error message \"" + ErrorMsg
+ + "\"; please supply a valid";
}
bool ento::shouldRegisterUninitializedObjectChecker(const LangOptions &LO) {
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,10 @@
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.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+ << (llvm::Twine() + Checker->getTagDescription() + ":AllowedPad").str()
+ << "a non-negative";
}
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,18 @@
AggressivenessKind Aggressiveness;
public:
- void setAggressiveness(StringRef Str) {
+ void setAggressiveness(StringRef Str, DiagnosticsEngine &Diags) {
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)
+ Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+ << (llvm::Twine() + getTagDescription() + ":WarnOn").str()
+ << "either \"KnownsOnly\", \"KnownsAndLocals\" or \"All\" string";
};
private:
@@ -735,7 +741,9 @@
void ento::registerMoveChecker(CheckerManager &mgr) {
MoveChecker *chk = mgr.registerChecker<MoveChecker>();
chk->setAggressiveness(
- mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn", ""));
+ mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn",
+ "KnownsAndLocals"),
+ mgr.getDiagnostics());
}
bool ento::shouldRegisterMoveChecker(const LangOptions &LO) {
Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -15,6 +15,7 @@
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/Analysis/CloneDetection.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/Driver/DriverDiagnostic.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -27,6 +28,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 +70,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 +81,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 +193,22 @@
//===----------------------------------------------------------------------===//
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.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+ << (llvm::Twine() + Checker->getTagDescription()
+ + ":MinimumCloneComplexity").str()
+ << "a non-negative";
+
+ 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/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: include/clang/StaticAnalyzer/Core/CheckerManager.h
===================================================================
--- include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -135,6 +135,7 @@
const LangOptions &getLangOpts() const { return LangOpts; }
AnalyzerOptions &getAnalyzerOptions() { return AOptions; }
ASTContext &getASTContext() { return Context; }
+ DiagnosticsEngine &getDiagnostics() { return Context.getDiagnostics(); }
using CheckerRef = CheckerBase *;
using CheckerTag = const void *;
Index: include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -301,6 +301,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 value">;
def err_drv_invalid_hvx_length : Error<
"-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
