[analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

This patch effectively fixes the almost decade old checker naming issue.
The solution is to assert when CheckerManager::getChecker is called on an
unregistered checker, and assert when CheckerManager::registerChecker is called
on a checker that is already registered.

Differential Revision: https://reviews.llvm.org/D55429


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@352292 91177308-0d34-0410-b5e6-96231b3b80d8

18 files changed, 58 insertions, 42 deletions

diff --git a/include/clang/StaticAnalyzer/Core/CheckerManager.h b/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 01621c8740..b4de26d792 100644
--- a/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -141,7 +141,7 @@ public:
   using CheckerDtor = CheckerFn<void ()>;
 
 //===----------------------------------------------------------------------===//
-// registerChecker
+// Checker registration.
 //===----------------------------------------------------------------------===//
 
   /// Used to register checkers.
@@ -153,8 +153,7 @@ public:
   CHECKER *registerChecker(AT &&... Args) {
     CheckerTag tag = getTag<CHECKER>();
     CheckerRef &ref = CheckerTags[tag];
-    if (ref)
-      return static_cast<CHECKER *>(ref); // already registered.
+    assert(!ref && "Checker already registered, use getChecker!");
 
     CHECKER *checker = new CHECKER(std::forward<AT>(Args)...);
     checker->Name = CurrentCheckName;
@@ -164,8 +163,17 @@ public:
     return checker;
   }
 
+  template <typename CHECKER>
+  CHECKER *getChecker() {
+    CheckerTag tag = getTag<CHECKER>();
+    assert(CheckerTags.count(tag) != 0 &&
+           "Requested checker is not registered! Maybe you should add it as a "
+           "dependency in Checkers.td?");
+    return static_cast<CHECKER *>(CheckerTags[tag]);
+  }
+
 //===----------------------------------------------------------------------===//
-// Functions for running checkers for AST traversing..
+// Functions for running checkers for AST traversing.
 //===----------------------------------------------------------------------===//
 
   /// Run checkers handling Decls.
diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index d37b2c57f5..fe11e54883 100644
--- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2485,7 +2485,7 @@ bool ento::shouldRegisterCStringModeling(const LangOptions &LO) {
 
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
-    CStringChecker *checker = mgr.registerChecker<CStringChecker>();           \
+    CStringChecker *checker = mgr.getChecker<CStringChecker>();                \
     checker->Filter.Check##name = true;                                        \
     checker->Filter.CheckName##name = mgr.getCurrentCheckName();               \
   }                                                                            \
diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 4992ece132..5a7eba0760 100644
--- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -609,8 +609,7 @@ bool ento::shouldRegisterCallAndMessageChecker(const LangOptions &LO) {
 }
 
 void ento::registerCallAndMessageUnInitRefArg(CheckerManager &mgr) {
-  CallAndMessageChecker *Checker =
-      mgr.registerChecker<CallAndMessageChecker>();
+  CallAndMessageChecker *Checker = mgr.getChecker<CallAndMessageChecker>();
   Checker->Check_CallAndMessageUnInitRefArg = true;
   Checker->CheckName_CallAndMessageUnInitRefArg = mgr.getCurrentCheckName();
 }
diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 383005aa44..e297f8cdf2 100644
--- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -915,8 +915,7 @@ bool ento::shouldRegisterSecuritySyntaxChecker(const LangOptions &LO) {
 
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
-    SecuritySyntaxChecker *checker =                                           \
-        mgr.registerChecker<SecuritySyntaxChecker>();                          \
+    SecuritySyntaxChecker *checker =  mgr.getChecker<SecuritySyntaxChecker>(); \
     checker->filter.check_##name = true;                                       \
     checker->filter.checkName_##name = mgr.getCurrentCheckName();              \
   }                                                                            \
diff --git a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
index dc6388da28..0058f3d388 100644
--- a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
+++ b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
@@ -226,7 +226,7 @@ bool ento::shouldRegisterDirectIvarAssignment(const LangOptions &LO) {
 
 void ento::registerDirectIvarAssignmentForAnnotatedFunctions(
     CheckerManager &mgr) {
-  mgr.registerChecker<DirectIvarAssignment>()->ShouldSkipMethod = &AttrFilter;
+  mgr.getChecker<DirectIvarAssignment>()->ShouldSkipMethod = &AttrFilter;
 }
 
 bool ento::shouldRegisterDirectIvarAssignmentForAnnotatedFunctions(
diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
index 2e473fb147..1862ffc79d 100644
--- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
+++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
@@ -987,8 +987,7 @@ DynamicTypePropagation::GenericsBugVisitor::VisitNode(const ExplodedNode *N,
 
 /// Register checkers.
 void ento::registerObjCGenericsChecker(CheckerManager &mgr) {
-  DynamicTypePropagation *checker =
-      mgr.registerChecker<DynamicTypePropagation>();
+  DynamicTypePropagation *checker = mgr.getChecker<DynamicTypePropagation>();
   checker->CheckGenerics = true;
 }
 
diff --git a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
index fbabe33595..8649a32f05 100644
--- a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -2403,7 +2403,7 @@ bool ento::shouldRegisterIteratorModeling(const LangOptions &LO) {
 
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &Mgr) {                             \
-    auto *checker = Mgr.registerChecker<IteratorChecker>();                    \
+    auto *checker = Mgr.getChecker<IteratorChecker>();                         \
     checker->ChecksEnabled[IteratorChecker::CK_##name] = true;                 \
     checker->CheckNames[IteratorChecker::CK_##name] =                          \
         Mgr.getCurrentCheckName();                                             \
diff --git a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
index 77ec825798..2b75f3acc9 100644
--- a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
@@ -746,7 +746,7 @@ bool ento::shouldRegisterIvarInvalidationModeling(const LangOptions &LO) {
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
     IvarInvalidationChecker *checker =                                         \
-        mgr.registerChecker<IvarInvalidationChecker>();                        \
+        mgr.getChecker<IvarInvalidationChecker>();                             \
     checker->Filter.check_##name = true;                                       \
     checker->Filter.checkName_##name = mgr.getCurrentCheckName();              \
   }                                                                            \
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 016aa636f1..7ab67213d5 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3089,16 +3089,16 @@ markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
 // Intended to be used in InnerPointerChecker to register the part of
 // MallocChecker connected to it.
 void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) {
-    MallocChecker *checker = mgr.registerChecker<MallocChecker>();
-    checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
-                                                  "Optimistic", false, checker);
-    checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true;
-    checker->CheckNames[MallocChecker::CK_InnerPointerChecker] =
-        mgr.getCurrentCheckName();
+  MallocChecker *checker = mgr.getChecker<MallocChecker>();
+  checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true;
+  checker->CheckNames[MallocChecker::CK_InnerPointerChecker] =
+      mgr.getCurrentCheckName();
 }
 
 void ento::registerDynamicMemoryModeling(CheckerManager &mgr) {
-  mgr.registerChecker<MallocChecker>();
+  auto *checker = mgr.registerChecker<MallocChecker>();
+  checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
+                                                  "Optimistic", false, checker);
 }
 
 bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) {
@@ -3107,9 +3107,7 @@ bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) {
 
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
-    MallocChecker *checker = mgr.registerChecker<MallocChecker>();             \
-    checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(  \
-        "Optimistic", false, checker);                                         \
+    MallocChecker *checker = mgr.getChecker<MallocChecker>();                  \
     checker->ChecksEnabled[MallocChecker::CK_##name] = true;                   \
     checker->CheckNames[MallocChecker::CK_##name] = mgr.getCurrentCheckName(); \
   }                                                                            \
diff --git a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
index 3520b12933..5cec012258 100644
--- a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -317,8 +317,7 @@ bool ento::shouldRegisterNSOrCFErrorDerefChecker(const LangOptions &LO) {
 
 void ento::registerNSErrorChecker(CheckerManager &mgr) {
   mgr.registerChecker<NSErrorMethodChecker>();
-  NSOrCFErrorDerefChecker *checker =
-      mgr.registerChecker<NSOrCFErrorDerefChecker>();
+  NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>();
   checker->ShouldCheckNSError = true;
 }
 
@@ -328,8 +327,7 @@ bool ento::shouldRegisterNSErrorChecker(const LangOptions &LO) {
 
 void ento::registerCFErrorChecker(CheckerManager &mgr) {
   mgr.registerChecker<CFErrorFunctionChecker>();
-  NSOrCFErrorDerefChecker *checker =
-      mgr.registerChecker<NSOrCFErrorDerefChecker>();
+  NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>();
   checker->ShouldCheckCFError = true;
 }
 
diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 7d86f67090..c2afb9fab7 100644
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -1201,7 +1201,7 @@ bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) {
 
 #define REGISTER_CHECKER(name, trackingRequired)                               \
   void ento::register##name##Checker(CheckerManager &mgr) {                    \
-    NullabilityChecker *checker = mgr.registerChecker<NullabilityChecker>();   \
+    NullabilityChecker *checker = mgr.getChecker<NullabilityChecker>();        \
     checker->Filter.Check##name = true;                                        \
     checker->Filter.CheckName##name = mgr.getCurrentCheckName();               \
     checker->NeedTracking = checker->NeedTracking || trackingRequired;         \
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 5d8e7a83e3..c3d191429e 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1464,7 +1464,7 @@ bool ento::shouldRegisterRetainCountBase(const LangOptions &LO) {
 }
 
 void ento::registerRetainCountChecker(CheckerManager &Mgr) {
-  auto *Chk = Mgr.registerChecker<RetainCountChecker>();
+  auto *Chk = Mgr.getChecker<RetainCountChecker>();
   Chk->TrackObjCAndCFObjects = true;
 }
 
@@ -1484,7 +1484,7 @@ static bool hasPrevCheckOSObjectOptionDisabled(AnalyzerOptions &Options) {
 }
 
 void ento::registerOSObjectRetainCountChecker(CheckerManager &Mgr) {
-  auto *Chk = Mgr.registerChecker<RetainCountChecker>();
+  auto *Chk = Mgr.getChecker<RetainCountChecker>();
   if (!hasPrevCheckOSObjectOptionDisabled(Mgr.getAnalyzerOptions()))
     Chk->TrackOSObjects = true;
 }
diff --git a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index f6bb7375d1..b93bed5c30 100644
--- a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -367,11 +367,11 @@ bool ento::shouldRegisterStackAddrEscapeBase(const LangOptions &LO) {
   return true;
 }
 
-#define REGISTER_CHECKER(name) \
-  void ento::register##name(CheckerManager &Mgr) { \
-    StackAddrEscapeChecker *Chk = \
-        Mgr.registerChecker<StackAddrEscapeChecker>(); \
-    Chk->ChecksEnabled[StackAddrEscapeChecker::CK_##name] = true; \
+#define REGISTER_CHECKER(name)                                                 \
+  void ento::register##name(CheckerManager &Mgr) {                             \
+    StackAddrEscapeChecker *Chk =                                              \
+        Mgr.getChecker<StackAddrEscapeChecker>();                              \
+    Chk->ChecksEnabled[StackAddrEscapeChecker::CK_##name] = true;              \
   }                                                                            \
                                                                                \
   bool ento::shouldRegister##name(const LangOptions &LO) {                     \
diff --git a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
index d3eb506312..13ad3d98e8 100644
--- a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -409,7 +409,7 @@ bool ento::shouldRegisterValistBase(const LangOptions &LO) {
 
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name##Checker(CheckerManager &mgr) {                    \
-    ValistChecker *checker = mgr.registerChecker<ValistChecker>();             \
+    ValistChecker *checker = mgr.getChecker<ValistChecker>();                  \
     checker->ChecksEnabled[ValistChecker::CK_##name] = true;                   \
     checker->CheckNames[ValistChecker::CK_##name] = mgr.getCurrentCheckName(); \
   }                                                                            \
diff --git a/test/Analysis/analyzer-checker-config.c b/test/Analysis/analyzer-checker-config.c
index 34e339963f..d28b2ff665 100644
--- a/test/Analysis/analyzer-checker-config.c
+++ b/test/Analysis/analyzer-checker-config.c
@@ -4,7 +4,7 @@
 // RUN: not %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -analyzer-config ..:Optimistic=true 2>&1 | FileCheck %s
 // RUN: not %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -analyzer-config unix.:Optimistic=true 2>&1 | FileCheck %s
 // RUN: not %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -analyzer-config unrelated:Optimistic=true 2>&1 | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -analyzer-config unix.Malloc:Optimistic=true
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 
 // Just to test clang is working.
 # foo
diff --git a/test/Analysis/free.c b/test/Analysis/free.c
index acdb2820d3..0d29bacf27 100644
--- a/test/Analysis/free.c
+++ b/test/Analysis/free.c
@@ -1,5 +1,11 @@
-// RUN: %clang_analyze_cc1 -analyzer-store=region -analyzer-checker=core,unix.Malloc -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-store=region -analyzer-checker=core,unix.Malloc -fblocks -verify -analyzer-config unix.Malloc:Optimistic=true %s
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc
+//
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 typedef __typeof(sizeof(int)) size_t;
 void free(void *);
 void *alloca(size_t);
diff --git a/test/Analysis/outofbound.c b/test/Analysis/outofbound.c
index 35672c0c44..c4af1fbb90 100644
--- a/test/Analysis/outofbound.c
+++ b/test/Analysis/outofbound.c
@@ -1,4 +1,8 @@
-// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,unix,alpha.security.ArrayBound -analyzer-store=region -verify -analyzer-config unix:Optimistic=true %s
+// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-store=region -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix \
+// RUN:   -analyzer-checker=alpha.security.ArrayBound \
+// RUN:   -analyzer-config unix:Optimistic=true
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
diff --git a/test/Analysis/undef-buffers.c b/test/Analysis/undef-buffers.c
index d5802fb8c6..10a46c64f6 100644
--- a/test/Analysis/undef-buffers.c
+++ b/test/Analysis/undef-buffers.c
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,core.uninitialized -analyzer-store=region -verify -analyzer-config unix:Optimistic=true %s
+// RUN: %clang_analyze_cc1 -analyzer-store=region -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix \
+// RUN:   -analyzer-checker=core.uninitialized \
+// RUN:   -analyzer-config unix:Optimistic=true
+
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);