[analyzer] Conversion to CheckerFamily: DereferenceChecker

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -206,21 +206,15 @@ def CallAndMessageChecker : Checker<"CallAndMessage">,
   Documentation<HasDocumentation>,
   Dependencies<[CallAndMessageModeling]>;
 
-def DereferenceModeling : Checker<"DereferenceModeling">,
-  HelpText<"General support for dereference related checkers">,
-  Documentation<NotDocumented>,
-  Hidden;
-
 def FixedAddressDereferenceChecker
     : Checker<"FixedAddressDereference">,
       HelpText<"Check for dereferences of fixed addresses">,
-      Documentation<HasDocumentation>,
-      Dependencies<[DereferenceModeling]>;
+      Documentation<HasDocumentation>;
 
-def NullDereferenceChecker : Checker<"NullDereference">,
-  HelpText<"Check for dereferences of null pointers">,
-  Documentation<HasDocumentation>,
-  Dependencies<[DereferenceModeling]>;
+def NullDereferenceChecker
+    : Checker<"NullDereference">,
+      HelpText<"Check for dereferences of null pointers">,
+      Documentation<HasDocumentation>;
 
 def NonNullParamChecker : Checker<"NonNullParamChecker">,
   HelpText<"Check for null pointers passed as arguments to a function whose "

clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp

@@ -25,18 +25,22 @@ using namespace clang;
 using namespace ento;
 
 namespace {
+
+class DerefBugType : public BugType {
+  StringRef ArrayMsg, FieldMsg;
+
+public:
+  DerefBugType(CheckerFrontend *FE, StringRef Desc, const char *RMsg,
+               const char *FMsg = nullptr)
+      : BugType(FE, Desc), ArrayMsg(RMsg), FieldMsg(FMsg ? FMsg : RMsg) {}
+  StringRef getArrayMsg() const { return ArrayMsg; }
+  StringRef getFieldMsg() const { return FieldMsg; }
+};
+
 class DereferenceChecker
-    : public Checker< check::Location,
-                      check::Bind,
-                      EventDispatcher<ImplicitNullDerefEvent> > {
-  enum DerefKind {
-    NullPointer,
-    UndefinedPointerValue,
-    AddressOfLabel,
-    FixedAddress,
-  };
-
-  void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S,
+    : public CheckerFamily<check::Location, check::Bind,
+                           EventDispatcher<ImplicitNullDerefEvent>> {
+  void reportBug(const DerefBugType &BT, ProgramStateRef State, const Stmt *S,
                  CheckerContext &C) const;
 
   bool suppressReport(CheckerContext &C, const Expr *E) const;
@@ -52,13 +56,23 @@ class DereferenceChecker
                              const LocationContext *LCtx,
                              bool loadedFrom = false);
 
-  bool CheckNullDereference = false;
-  bool CheckFixedDereference = false;
-
-  std::unique_ptr<BugType> BT_Null;
-  std::unique_ptr<BugType> BT_Undef;
-  std::unique_ptr<BugType> BT_Label;
-  std::unique_ptr<BugType> BT_FixedAddress;
+  CheckerFrontend NullDerefChecker, FixedDerefChecker;
+  const DerefBugType NullBug{&NullDerefChecker, "Dereference of null pointer",
+                             "a null pointer dereference",
+                             "a dereference of a null pointer"};
+  const DerefBugType UndefBug{&NullDerefChecker,
+                              "Dereference of undefined pointer value",
+                              "an undefined pointer dereference",
+                              "a dereference of an undefined pointer value"};
+  const DerefBugType LabelBug{&NullDerefChecker,
+                              "Dereference of the address of a label",
+                              "an undefined pointer dereference",
+                              "a dereference of an address of a label"};
+  const DerefBugType FixedAddressBug{&FixedDerefChecker,
+                                     "Dereference of a fixed address",
+                                     "a dereference of a fixed address"};
+
+  StringRef getDebugTag() const override { return "DereferenceChecker"; }
 };
 } // end anonymous namespace
 
@@ -158,115 +172,87 @@ static bool isDeclRefExprToReference(const Expr *E) {
   return false;
 }
 
-void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State,
-                                   const Stmt *S, CheckerContext &C) const {
-  const BugType *BT = nullptr;
-  llvm::StringRef DerefStr1;
-  llvm::StringRef DerefStr2;
-  switch (K) {
-  case DerefKind::NullPointer:
-    if (!CheckNullDereference) {
-      C.addSink();
-      return;
-    }
-    BT = BT_Null.get();
-    DerefStr1 = " results in a null pointer dereference";
-    DerefStr2 = " results in a dereference of a null pointer";
-    break;
-  case DerefKind::UndefinedPointerValue:
-    if (!CheckNullDereference) {
-      C.addSink();
+void DereferenceChecker::reportBug(const DerefBugType &BT,
+                                   ProgramStateRef State, const Stmt *S,
+                                   CheckerContext &C) const {
+  if (&BT == &FixedAddressBug) {
+    if (!FixedDerefChecker.isEnabled())
+      // Deliberately don't add a sink node if check is disabled.
+      // This situation may be valid in special cases.
       return;
-    }
-    BT = BT_Undef.get();
-    DerefStr1 = " results in an undefined pointer dereference";
-    DerefStr2 = " results in a dereference of an undefined pointer value";
-    break;
-  case DerefKind::AddressOfLabel:
-    if (!CheckNullDereference) {
+  } else {
+    if (!NullDerefChecker.isEnabled()) {
       C.addSink();
       return;
     }
-    BT = BT_Label.get();
-    DerefStr1 = " results in an undefined pointer dereference";
-    DerefStr2 = " results in a dereference of an address of a label";
-    break;
-  case DerefKind::FixedAddress:
-    // Deliberately don't add a sink node if check is disabled.
-    // This situation may be valid in special cases.
-    if (!CheckFixedDereference)
-      return;
-
-    BT = BT_FixedAddress.get();
-    DerefStr1 = " results in a dereference of a fixed address";
-    DerefStr2 = " results in a dereference of a fixed address";
-    break;
-  };
+  }
 
   // Generate an error node.
   ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
     return;
 
-  SmallString<100> buf;
-  llvm::raw_svector_ostream os(buf);
+  SmallString<100> Buf;
+  llvm::raw_svector_ostream Out(Buf);
 
   SmallVector<SourceRange, 2> Ranges;
 
   switch (S->getStmtClass()) {
   case Stmt::ArraySubscriptExprClass: {
-    os << "Array access";
+    Out << "Array access";
     const ArraySubscriptExpr *AE = cast<ArraySubscriptExpr>(S);
-    AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
-                   State.get(), N->getLocationContext());
-    os << DerefStr1;
+    AddDerefSource(Out, Ranges, AE->getBase()->IgnoreParenCasts(), State.get(),
+                   N->getLocationContext());
+    Out << " results in " << BT.getArrayMsg();
     break;
   }
   case Stmt::ArraySectionExprClass: {
-    os << "Array access";
+    Out << "Array access";
     const ArraySectionExpr *AE = cast<ArraySectionExpr>(S);
-    AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
-                   State.get(), N->getLocationContext());
-    os << DerefStr1;
+    AddDerefSource(Out, Ranges, AE->getBase()->IgnoreParenCasts(), State.get(),
+                   N->getLocationContext());
+    Out << " results in " << BT.getArrayMsg();
     break;
   }
   case Stmt::UnaryOperatorClass: {
-    os << BT->getDescription();
+    Out << BT.getDescription();
     const UnaryOperator *U = cast<UnaryOperator>(S);
-    AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
-                   State.get(), N->getLocationContext(), true);
+    AddDerefSource(Out, Ranges, U->getSubExpr()->IgnoreParens(), State.get(),
+                   N->getLocationContext(), true);
     break;
   }
   case Stmt::MemberExprClass: {
     const MemberExpr *M = cast<MemberExpr>(S);
     if (M->isArrow() || isDeclRefExprToReference(M->getBase())) {
-      os << "Access to field '" << M->getMemberNameInfo() << "'" << DerefStr2;
-      AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
-                     State.get(), N->getLocationContext(), true);
+      Out << "Access to field '" << M->getMemberNameInfo() << "' results in "
+          << BT.getFieldMsg();
+      AddDerefSource(Out, Ranges, M->getBase()->IgnoreParenCasts(), State.get(),
+                     N->getLocationContext(), true);
     }
     break;
   }
   case Stmt::ObjCIvarRefExprClass: {
     const ObjCIvarRefExpr *IV = cast<ObjCIvarRefExpr>(S);
-    os << "Access to instance variable '" << *IV->getDecl() << "'" << DerefStr2;
-    AddDerefSource(os, Ranges, IV->getBase()->IgnoreParenCasts(),
-                   State.get(), N->getLocationContext(), true);
+    Out << "Access to instance variable '" << *IV->getDecl() << "' results in "
+        << BT.getFieldMsg();
+    AddDerefSource(Out, Ranges, IV->getBase()->IgnoreParenCasts(), State.get(),
+                   N->getLocationContext(), true);
     break;
   }
   default:
     break;
   }
 
-  auto report = std::make_unique<PathSensitiveBugReport>(
-      *BT, buf.empty() ? BT->getDescription() : buf.str(), N);
+  auto BR = std::make_unique<PathSensitiveBugReport>(
+      BT, Buf.empty() ? BT.getDescription() : Buf.str(), N);
 
-  bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
+  bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *BR);
 
   for (SmallVectorImpl<SourceRange>::iterator
        I = Ranges.begin(), E = Ranges.end(); I!=E; ++I)
-    report->addRange(*I);
+    BR->addRange(*I);
 
-  C.emitReport(std::move(report));
+  C.emitReport(std::move(BR));
 }
 
 void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
@@ -275,7 +261,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
   if (l.isUndef()) {
     const Expr *DerefExpr = getDereferenceExpr(S);
     if (!suppressReport(C, DerefExpr))
-      reportBug(DerefKind::UndefinedPointerValue, C.getState(), DerefExpr, C);
+      reportBug(UndefBug, C.getState(), DerefExpr, C);
     return;
   }
 
@@ -296,7 +282,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
       // we call an "explicit" null dereference.
       const Expr *expr = getDereferenceExpr(S);
       if (!suppressReport(C, expr)) {
-        reportBug(DerefKind::NullPointer, nullState, expr, C);
+        reportBug(NullBug, nullState, expr, C);
         return;
       }
     }
@@ -314,7 +300,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
   if (location.isConstant()) {
     const Expr *DerefExpr = getDereferenceExpr(S, isLoad);
     if (!suppressReport(C, DerefExpr))
-      reportBug(DerefKind::FixedAddress, notNullState, DerefExpr, C);
+      reportBug(FixedAddressBug, notNullState, DerefExpr, C);
     return;
   }
 
@@ -330,7 +316,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
 
   // One should never write to label addresses.
   if (auto Label = L.getAs<loc::GotoLabel>()) {
-    reportBug(DerefKind::AddressOfLabel, C.getState(), S, C);
+    reportBug(LabelBug, C.getState(), S, C);
     return;
   }
 
@@ -351,7 +337,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
     if (!StNonNull) {
       const Expr *expr = getDereferenceExpr(S, /*IsBind=*/true);
       if (!suppressReport(C, expr)) {
-        reportBug(DerefKind::NullPointer, StNull, expr, C);
+        reportBug(NullBug, StNull, expr, C);
         return;
       }
     }
@@ -369,7 +355,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
   if (V.isConstant()) {
     const Expr *DerefExpr = getDereferenceExpr(S, true);
     if (!suppressReport(C, DerefExpr))
-      reportBug(DerefKind::FixedAddress, State, DerefExpr, C);
+      reportBug(FixedAddressBug, State, DerefExpr, C);
     return;
   }
 
@@ -392,38 +378,16 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
   C.addTransition(State, this);
 }
 
-void ento::registerDereferenceModeling(CheckerManager &Mgr) {
-  Mgr.registerChecker<DereferenceChecker>();
-}
-
-bool ento::shouldRegisterDereferenceModeling(const CheckerManager &) {
-  return true;
-}
-
 void ento::registerNullDereferenceChecker(CheckerManager &Mgr) {
-  auto *Chk = Mgr.getChecker<DereferenceChecker>();
-  Chk->CheckNullDereference = true;
-  Chk->BT_Null.reset(new BugType(Mgr.getCurrentCheckerName(),
-                                 "Dereference of null pointer",
-                                 categories::LogicError));
-  Chk->BT_Undef.reset(new BugType(Mgr.getCurrentCheckerName(),
-                                  "Dereference of undefined pointer value",
-                                  categories::LogicError));
-  Chk->BT_Label.reset(new BugType(Mgr.getCurrentCheckerName(),
-                                  "Dereference of the address of a label",
-                                  categories::LogicError));
+  Mgr.getChecker<DereferenceChecker>()->NullDerefChecker.enable(Mgr);
 }
 
 bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &) {
   return true;
 }
 
 void ento::registerFixedAddressDereferenceChecker(CheckerManager &Mgr) {
-  auto *Chk = Mgr.getChecker<DereferenceChecker>();
-  Chk->CheckFixedDereference = true;
-  Chk->BT_FixedAddress.reset(new BugType(Mgr.getCurrentCheckerName(),
-                                         "Dereference of a fixed address",
-                                         categories::LogicError));
+  Mgr.getChecker<DereferenceChecker>()->FixedDerefChecker.enable(Mgr);
 }
 
 bool ento::shouldRegisterFixedAddressDereferenceChecker(

clang/test/Analysis/analyzer-enabled-checkers.c

@@ -14,7 +14,6 @@
 // CHECK-NEXT: core.BitwiseShift
 // CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: core.CallAndMessage
-// CHECK-NEXT: core.DereferenceModeling
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
 // CHECK-NEXT: core.FixedAddressDereference

clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c

@@ -22,7 +22,6 @@
 // CHECK-NEXT: core.BitwiseShift
 // CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: core.CallAndMessage
-// CHECK-NEXT: core.DereferenceModeling
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
 // CHECK-NEXT: core.FixedAddressDereference