Expose the name of the checker producing each diagnostic message.

Summary:
In clang-tidy we'd like to know the name of the checker producing each
diagnostic message. PathDiagnostic has BugType and Category fields, which are
both arbitrary human-readable strings, but we need to know the exact name of the
checker in the form that can be used in the CheckersControlList option to
enable/disable the specific checker.

This patch adds the CheckName field to the CheckerBase class, and sets it in
the CheckerManager::registerChecker() method, which gets them from the
CheckerRegistry.

Checkers that implement multiple checks have to store the names of each check
in the respective registerXXXChecker method.

Reviewers: jordan_rose, krememek

Reviewed By: jordan_rose

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D2557

llvm-svn: 201186

Author: alexfh

clang/examples/analyzer-plugin/MainCallChecker.cpp

@@ -35,7 +35,7 @@ void MainCallChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const
       return;
 
     if (!BT)
-      BT.reset(new BugType("call to main", "example analyzer plugin"));
+      BT.reset(new BugType(this, "call to main", "example analyzer plugin"));
 
     BugReport *report = new BugReport(*BT, BT->getName(), N);
     report->addRange(Callee->getSourceRange());

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

@@ -19,6 +19,7 @@
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
@@ -463,7 +464,12 @@ class BugReporter {
   /// reports.
   void emitReport(BugReport *R);
 
-  void EmitBasicReport(const Decl *DeclWithIssue,
+  void EmitBasicReport(const Decl *DeclWithIssue, const CheckerBase *Checker,
+                       StringRef BugName, StringRef BugCategory,
+                       StringRef BugStr, PathDiagnosticLocation Loc,
+                       ArrayRef<SourceRange> Ranges = None);
+
+  void EmitBasicReport(const Decl *DeclWithIssue, CheckName CheckName,
                        StringRef BugName, StringRef BugCategory,
                        StringRef BugStr, PathDiagnosticLocation Loc,
                        ArrayRef<SourceRange> Ranges = None);
@@ -473,7 +479,8 @@ class BugReporter {
 
   /// \brief Returns a BugType that is associated with the given name and
   /// category.
-  BugType *getBugTypeForName(StringRef name, StringRef category);
+  BugType *getBugTypeForName(CheckName CheckName, StringRef name,
+                             StringRef category);
 };
 
 // FIXME: Get rid of GRBugReporter.  It's the wrong abstraction.

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h

@@ -16,6 +16,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
 #include "llvm/ADT/FoldingSet.h"
 #include <string>
 
@@ -29,20 +30,25 @@ class ExprEngine;
 
 class BugType {
 private:
+  const CheckName CheckName;
   const std::string Name;
   const std::string Category;
   bool SuppressonSink;
 
   virtual void anchor();
 public:
-  BugType(StringRef name, StringRef cat)
-    : Name(name), Category(cat), SuppressonSink(false) {}
+  BugType(class CheckName Check, StringRef name, StringRef cat)
+      : CheckName(Check), Name(name), Category(cat), SuppressonSink(false) {}
+  BugType(const CheckerBase *checker, StringRef name, StringRef cat)
+      : CheckName(checker->getCheckName()), Name(name), Category(cat),
+        SuppressonSink(false) {}
   virtual ~BugType() {}
 
   // FIXME: Should these be made strings as well?
   StringRef getName() const { return Name; }
   StringRef getCategory() const { return Category; }
-  
+  StringRef getCheckName() const { return CheckName.getName(); }
+
   /// isSuppressOnSink - Returns true if bug reports associated with this bug
   ///  type should be suppressed if the end node of the report is post-dominated
   ///  by a sink node.
@@ -56,12 +62,17 @@ class BuiltinBug : public BugType {
   const std::string desc;
   virtual void anchor();
 public:
-  BuiltinBug(const char *name, const char *description)
-    : BugType(name, categories::LogicError), desc(description) {}
-  
-  BuiltinBug(const char *name)
-    : BugType(name, categories::LogicError), desc(name) {}
-  
+  BuiltinBug(class CheckName CheckName, const char *name,
+             const char *description)
+      : BugType(CheckName, name, categories::LogicError), desc(description) {}
+
+  BuiltinBug(const CheckerBase *checker, const char *name,
+             const char *description)
+      : BugType(checker, name, categories::LogicError), desc(description) {}
+
+  BuiltinBug(const CheckerBase *checker, const char *name)
+      : BugType(checker, name, categories::LogicError), desc(name) {}
+
   StringRef getDescription() const { return desc; }
 };
 

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

@@ -710,6 +710,7 @@ class PathDiagnosticMacroPiece : public PathDiagnosticSpotPiece {
 ///  diagnostic.  It represents an ordered-collection of PathDiagnosticPieces,
 ///  each which represent the pieces of the path.
 class PathDiagnostic : public llvm::FoldingSetNode {
+  std::string CheckName;
   const Decl *DeclWithIssue;
   std::string BugType;
   std::string VerboseDesc;
@@ -730,8 +731,8 @@ class PathDiagnostic : public llvm::FoldingSetNode {
 
   PathDiagnostic() LLVM_DELETED_FUNCTION;
 public:
-  PathDiagnostic(const Decl *DeclWithIssue, StringRef bugtype,
-                 StringRef verboseDesc, StringRef shortDesc,
+  PathDiagnostic(StringRef CheckName, const Decl *DeclWithIssue,
+                 StringRef bugtype, StringRef verboseDesc, StringRef shortDesc,
                  StringRef category, PathDiagnosticLocation LocationToUnique,
                  const Decl *DeclToUnique);
 
@@ -788,6 +789,7 @@ class PathDiagnostic : public llvm::FoldingSetNode {
   StringRef getShortDescription() const {
     return ShortDesc.empty() ? VerboseDesc : ShortDesc;
   }
+  StringRef getCheckName() const { return CheckName; }
   StringRef getBugType() const { return BugType; }
   StringRef getCategory() const { return Category; }
 

clang/include/clang/StaticAnalyzer/Core/Checker.h

@@ -453,14 +453,18 @@ class Call {
 } // end eval namespace
 
 class CheckerBase : public ProgramPointTag {
+  CheckName Name;
+  friend class ::clang::ento::CheckerManager;
+
 public:
   StringRef getTagDescription() const;
+  CheckName getCheckName() const;
 
   /// See CheckerManager::runCheckersForPrintState.
   virtual void printState(raw_ostream &Out, ProgramStateRef State,
                           const char *NL, const char *Sep) const { }
 };
-  
+
 template <typename CHECK1, typename CHECK2=check::_VoidCheck,
           typename CHECK3=check::_VoidCheck, typename CHECK4=check::_VoidCheck,
           typename CHECK5=check::_VoidCheck, typename CHECK6=check::_VoidCheck,

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

@@ -29,6 +29,7 @@ namespace clang {
 
 namespace ento {
   class CheckerBase;
+  class CheckerRegistry;
   class ExprEngine;
   class AnalysisManager;
   class BugReporter;
@@ -131,9 +132,26 @@ enum PointerEscapeKind {
   PSK_EscapeOther
 };
 
+// This wrapper is used to ensure that only StringRefs originating from the
+// CheckerRegistry are used as check names. We want to make sure all check
+// name strings have a lifetime that keeps them alive at least until the path
+// diagnostics have been processed.
+class CheckName {
+  StringRef Name;
+  friend class ::clang::ento::CheckerRegistry;
+  explicit CheckName(StringRef Name) : Name(Name) {}
+
+public:
+  CheckName() {}
+  CheckName(const CheckName &Other) : Name(Other.Name) {}
+  StringRef getName() const { return Name; }
+};
+
 class CheckerManager {
   const LangOptions LangOpts;
   AnalyzerOptionsRef AOptions;
+  CheckName CurrentCheckName;
+
 public:
   CheckerManager(const LangOptions &langOpts,
                  AnalyzerOptionsRef AOptions)
@@ -142,6 +160,9 @@ class CheckerManager {
 
   ~CheckerManager();
 
+  void setCurrentCheckName(CheckName name) { CurrentCheckName = name; }
+  CheckName getCurrentCheckName() const { return CurrentCheckName; }
+
   bool hasPathSensitiveCheckers() const;
 
   void finishedCheckerRegistration();
@@ -168,6 +189,7 @@ class CheckerManager {
       return static_cast<CHECKER *>(ref); // already registered.
 
     CHECKER *checker = new CHECKER();
+    checker->Name = CurrentCheckName;
     CheckerDtors.push_back(CheckerDtor(checker, destruct<CHECKER>));
     CHECKER::_register(checker, *this);
     ref = checker;
@@ -182,6 +204,7 @@ class CheckerManager {
       return static_cast<CHECKER *>(ref); // already registered.
 
     CHECKER *checker = new CHECKER(AOpts);
+    checker->Name = CurrentCheckName;
     CheckerDtors.push_back(CheckerDtor(checker, destruct<CHECKER>));
     CHECKER::_register(checker, *this);
     ref = checker;

clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp

@@ -112,7 +112,7 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G,
       << " | Empty WorkList: "
       << (Eng.hasEmptyWorkList() ? "yes" : "no");
 
-  B.EmitBasicReport(D, "Analyzer Statistics", "Internal Statistics",
+  B.EmitBasicReport(D, this, "Analyzer Statistics", "Internal Statistics",
                     output.str(), PathDiagnosticLocation(D, SM));
 
   // Emit warning for each block we bailed out on.
@@ -129,7 +129,7 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G,
       outputI << "(" << NameOfRootFunction << ")" <<
                  ": The analyzer generated a sink at this point";
       B.EmitBasicReport(
-          D, "Sink Point", "Internal Statistics", outputI.str(),
+          D, this, "Sink Point", "Internal Statistics", outputI.str(),
           PathDiagnosticLocation::createBegin(CS->getStmt(), SM, LC));
     }
   }

clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp

@@ -66,8 +66,9 @@ void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS,
       return;
 
     if (!BT)
-      BT.reset(new BuiltinBug("Out-of-bound array access",
-                       "Access out-of-bound array element (buffer overflow)"));
+      BT.reset(new BuiltinBug(
+          this, "Out-of-bound array access",
+          "Access out-of-bound array element (buffer overflow)"));
 
     // FIXME: It would be nice to eventually make this diagnostic more clear,
     // e.g., by referencing the original declaration or by saying *why* this

clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

@@ -187,7 +187,7 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext,
     return;
 
   if (!BT)
-    BT.reset(new BuiltinBug("Out-of-bound access"));
+    BT.reset(new BuiltinBug(this, "Out-of-bound access"));
 
   // FIXME: This diagnostics are preliminary.  We should get far better
   // diagnostics for explaining buffer overruns.
@@ -311,7 +311,6 @@ RegionRawOffsetV2 RegionRawOffsetV2::computeOffset(ProgramStateRef state,
   return RegionRawOffsetV2();
 }
 
-
 void ento::registerArrayBoundCheckerV2(CheckerManager &mgr) {
   mgr.registerChecker<ArrayBoundCheckerV2>();
 }

clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

@@ -39,7 +39,8 @@ using namespace ento;
 namespace {
 class APIMisuse : public BugType {
 public:
-  APIMisuse(const char* name) : BugType(name, "API Misuse (Apple)") {}
+  APIMisuse(const CheckerBase *checker, const char *name)
+      : BugType(checker, name, "API Misuse (Apple)") {}
 };
 } // end anonymous namespace
 
@@ -191,7 +192,7 @@ void NilArgChecker::generateBugReport(ExplodedNode *N,
                                       const Expr *E,
                                       CheckerContext &C) const {
   if (!BT)
-    BT.reset(new APIMisuse("nil argument"));
+    BT.reset(new APIMisuse(this, "nil argument"));
 
   BugReport *R = new BugReport(*BT, Msg, N);
   R->addRange(Range);
@@ -483,8 +484,8 @@ void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE,
       << " bits of the input integer will be lost.";
 
     if (!BT)
-      BT.reset(new APIMisuse("Bad use of CFNumberCreate"));
-    
+      BT.reset(new APIMisuse(this, "Bad use of CFNumberCreate"));
+
     BugReport *report = new BugReport(*BT, os.str(), N);
     report->addRange(CE->getArg(2)->getSourceRange());
     C.emitReport(report);
@@ -522,8 +523,8 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE,
     Retain = &Ctx.Idents.get("CFRetain");
     Release = &Ctx.Idents.get("CFRelease");
     MakeCollectable = &Ctx.Idents.get("CFMakeCollectable");
-    BT.reset(
-      new APIMisuse("null passed to CFRetain/CFRelease/CFMakeCollectable"));
+    BT.reset(new APIMisuse(
+        this, "null passed to CFRetain/CFRelease/CFMakeCollectable"));
   }
 
   // Check if we called CFRetain/CFRelease/CFMakeCollectable.
@@ -600,9 +601,9 @@ void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
                                               CheckerContext &C) const {
 
   if (!BT) {
-    BT.reset(new APIMisuse("message incorrectly sent to class instead of class "
-                           "instance"));
-  
+    BT.reset(new APIMisuse(
+        this, "message incorrectly sent to class instead of class instance"));
+
     ASTContext &Ctx = C.getASTContext();
     releaseS = GetNullarySelector("release", Ctx);
     retainS = GetNullarySelector("retain", Ctx);
@@ -708,7 +709,8 @@ VariadicMethodTypeChecker::isVariadicMessage(const ObjCMethodCall &msg) const {
 void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
                                                     CheckerContext &C) const {
   if (!BT) {
-    BT.reset(new APIMisuse("Arguments passed to variadic method aren't all "
+    BT.reset(new APIMisuse(this,
+                           "Arguments passed to variadic method aren't all "
                            "Objective-C pointer types"));
 
     ASTContext &Ctx = C.getASTContext();
@@ -1263,6 +1265,7 @@ void ento::registerObjCLoopChecker(CheckerManager &mgr) {
   mgr.registerChecker<ObjCLoopChecker>();
 }
 
-void ento::registerObjCNonNilReturnValueChecker(CheckerManager &mgr) {
+void
+ento::registerObjCNonNilReturnValueChecker(CheckerManager &mgr) {
   mgr.registerChecker<ObjCNonNilReturnValueChecker>();
 }