[analyzer] Introduce the check::BlockEntrance checker callback (#140924)

Tranersing the CFG blocks of a function is a fundamental operation. Many
C++ constructs can create splits in the control-flow, such as `if`,
`for`, and similar control structures or ternary expressions, gnu
conditionals, gotos, switches and possibly more.

Checkers should be able to get notifications about entering or leaving a
CFG block of interest.

Note that in the ExplodedGraph there is always a BlockEntrance
ProgramPoint right after the BlockEdge ProgramPoint. I considered naming
this callback check::BlockEdge, but then that may leave the observer of
the graph puzzled to see BlockEdge points followed more BlockEdge nodes
describing the same CFG transition. This confusion could also apply to
Bug Report Visitors too.

Because of this, I decided to hook BlockEntrance ProgramPoints instead.
The same confusion applies here, but I find this still a better place
TBH. There would only appear only one BlockEntrance ProgramPoint in the
graph if no checkers modify the state or emit a bug report. Otherwise
they modify some GDM (aka. State) thus create a new ExplodedNode with
the same BlockEntrance ProgramPoint in the graph.

CPP-6484

Author: balazs-benics-sonarsource

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

@@ -221,6 +221,22 @@ class Bind {
   }
 };
 
+class BlockEntrance {
+  template <typename CHECKER>
+  static void _checkBlockEntrance(void *Checker,
+                                  const clang::BlockEntrance &Entrance,
+                                  CheckerContext &C) {
+    ((const CHECKER *)Checker)->checkBlockEntrance(Entrance, C);
+  }
+
+public:
+  template <typename CHECKER>
+  static void _register(CHECKER *checker, CheckerManager &mgr) {
+    mgr._registerForBlockEntrance(CheckerManager::CheckBlockEntranceFunc(
+        checker, _checkBlockEntrance<CHECKER>));
+  }
+};
+
 class EndAnalysis {
   template <typename CHECKER>
   static void _checkEndAnalysis(void *checker, ExplodedGraph &G,
@@ -536,6 +552,8 @@ class CheckerBase : public CheckerFrontend, public CheckerBackend {
 template <typename... CHECKs>
 class Checker : public CheckerBase, public CHECKs... {
 public:
+  using BlockEntrance = clang::BlockEntrance;
+
   template <typename CHECKER>
   static void _register(CHECKER *Chk, CheckerManager &Mgr) {
     (CHECKs::_register(Chk, Mgr), ...);
@@ -565,6 +583,8 @@ class Checker : public CheckerBase, public CHECKs... {
 template <typename... CHECKs>
 class CheckerFamily : public CheckerBackend, public CHECKs... {
 public:
+  using BlockEntrance = clang::BlockEntrance;
+
   template <typename CHECKER>
   static void _register(CHECKER *Chk, CheckerManager &Mgr) {
     (CHECKs::_register(Chk, Mgr), ...);

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

@@ -344,6 +344,12 @@ class CheckerManager {
                           const Stmt *S, ExprEngine &Eng,
                           const ProgramPoint &PP);
 
+  /// Run checkers after taking a control flow edge.
+  void runCheckersForBlockEntrance(ExplodedNodeSet &Dst,
+                                   const ExplodedNodeSet &Src,
+                                   const BlockEntrance &Entrance,
+                                   ExprEngine &Eng) const;
+
   /// Run checkers for end of analysis.
   void runCheckersForEndAnalysis(ExplodedGraph &G, BugReporter &BR,
                                  ExprEngine &Eng);
@@ -496,6 +502,9 @@ class CheckerManager {
   using CheckBindFunc =
       CheckerFn<void(SVal location, SVal val, const Stmt *S, CheckerContext &)>;
 
+  using CheckBlockEntranceFunc =
+      CheckerFn<void(const BlockEntrance &, CheckerContext &)>;
+
   using CheckEndAnalysisFunc =
       CheckerFn<void (ExplodedGraph &, BugReporter &, ExprEngine &)>;
 
@@ -557,6 +566,8 @@ class CheckerManager {
 
   void _registerForBind(CheckBindFunc checkfn);
 
+  void _registerForBlockEntrance(CheckBlockEntranceFunc checkfn);
+
   void _registerForEndAnalysis(CheckEndAnalysisFunc checkfn);
 
   void _registerForBeginFunction(CheckBeginFunctionFunc checkfn);
@@ -663,6 +674,8 @@ class CheckerManager {
 
   std::vector<CheckBindFunc> BindCheckers;
 
+  std::vector<CheckBlockEntranceFunc> BlockEntranceCheckers;
+
   std::vector<CheckEndAnalysisFunc> EndAnalysisCheckers;
 
   std::vector<CheckBeginFunctionFunc> BeginFunctionCheckers;

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

@@ -321,6 +321,10 @@ class ExprEngine {
                                NodeBuilderWithSinks &nodeBuilder,
                                ExplodedNode *Pred);
 
+  void runCheckersForBlockEntrance(const NodeBuilderContext &BldCtx,
+                                   const BlockEntrance &Entrance,
+                                   ExplodedNode *Pred, ExplodedNodeSet &Dst);
+
   /// ProcessBranch - Called by CoreEngine. Used to generate successor nodes by
   /// processing the 'effects' of a branch condition. If the branch condition
   /// is a loop condition, IterationsCompletedInLoop is the number of completed

clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp

@@ -39,6 +39,7 @@ class CheckerDocumentation
           check::ASTDecl<FunctionDecl>,
           check::BeginFunction,
           check::Bind,
+          check::BlockEntrance,
           check::BranchCondition,
           check::ConstPointerEscape,
           check::DeadSymbols,
@@ -128,7 +129,20 @@ class CheckerDocumentation
   /// check::PostCall
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const {}
 
-  /// Pre-visit of the condition statement of a branch (such as IfStmt).
+  /// Pre-visit of the condition statement of a branch.
+  /// For example:
+  ///  - logical operators (&&, ||)
+  ///  - if, do, while, for, ranged-for statements
+  ///  - ternary operators (?:), gnu conditionals, gnu choose expressions
+  /// Interestingly, switch statements don't seem to trigger BranchCondition.
+  ///
+  /// check::BlockEntrance is a similar callback, which is strictly more
+  /// generic. Prefer check::BranchCondition to check::BlockEntrance if
+  /// pre-visiting conditional statements is enough for the checker.
+  /// Note that check::BlockEntrance is also invoked for leaving basic blocks
+  /// while entering the next.
+  ///
+  /// check::BranchCondition
   void checkBranchCondition(const Stmt *Condition, CheckerContext &Ctx) const {}
 
   /// Post-visit the C++ operator new's allocation call.
@@ -165,6 +179,29 @@ class CheckerDocumentation
   /// check::Bind
   void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &) const {}
 
+  /// Called after a CFG edge is taken within a function.
+  ///
+  /// This callback can be used to obtain information about potential branching
+  /// points or any other constructs that involve traversing a CFG edge.
+  ///
+  /// check::BranchCondition is a similar callback, which is only invoked for
+  /// pre-visiting the condition statement of a branch. Prefer that callback if
+  /// possible.
+  ///
+  /// \remark There is no CFG edge from the caller to a callee, consequently
+  /// this callback is not invoked for "inlining" a function call.
+  /// \remark Once a function call is inlined, we will start from the imaginary
+  /// "entry" basic block of that CFG. This callback will be invoked for
+  /// entering the real first basic block of the "inlined" function body from
+  /// that "entry" basic block.
+  /// \remark This callback is also invoked for entering the imaginary "exit"
+  /// basic block of the CFG when returning from a function.
+  ///
+  /// \param E The ProgramPoint that describes the transition.
+  ///
+  /// check::BlockEntrance
+  void checkBlockEntrance(const BlockEntrance &E, CheckerContext &) const {}
+
   /// Called whenever a symbol becomes dead.
   ///
   /// This callback should be used by the checkers to aggressively clean

clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

@@ -41,11 +41,11 @@ bool CheckerManager::hasPathSensitiveCheckers() const {
   return IfAnyAreNonEmpty(
       StmtCheckers, PreObjCMessageCheckers, ObjCMessageNilCheckers,
       PostObjCMessageCheckers, PreCallCheckers, PostCallCheckers,
-      LocationCheckers, BindCheckers, EndAnalysisCheckers,
-      BeginFunctionCheckers, EndFunctionCheckers, BranchConditionCheckers,
-      NewAllocatorCheckers, LiveSymbolsCheckers, DeadSymbolsCheckers,
-      RegionChangesCheckers, PointerEscapeCheckers, EvalAssumeCheckers,
-      EvalCallCheckers, EndOfTranslationUnitCheckers);
+      LocationCheckers, BindCheckers, BlockEntranceCheckers,
+      EndAnalysisCheckers, BeginFunctionCheckers, EndFunctionCheckers,
+      BranchConditionCheckers, NewAllocatorCheckers, LiveSymbolsCheckers,
+      DeadSymbolsCheckers, RegionChangesCheckers, PointerEscapeCheckers,
+      EvalAssumeCheckers, EvalCallCheckers, EndOfTranslationUnitCheckers);
 }
 
 void CheckerManager::reportInvalidCheckerOptionValue(
@@ -418,6 +418,42 @@ void CheckerManager::runCheckersForBind(ExplodedNodeSet &Dst,
   expandGraphWithCheckers(C, Dst, Src);
 }
 
+namespace {
+struct CheckBlockEntranceContext {
+  using CheckBlockEntranceFunc = CheckerManager::CheckBlockEntranceFunc;
+  using CheckersTy = std::vector<CheckBlockEntranceFunc>;
+
+  const CheckersTy &Checkers;
+  const BlockEntrance &Entrance;
+  ExprEngine &Eng;
+
+  CheckBlockEntranceContext(const CheckersTy &Checkers,
+                            const BlockEntrance &Entrance, ExprEngine &Eng)
+      : Checkers(Checkers), Entrance(Entrance), Eng(Eng) {}
+
+  auto checkers_begin() const { return Checkers.begin(); }
+  auto checkers_end() const { return Checkers.end(); }
+
+  void runChecker(CheckBlockEntranceFunc CheckFn, NodeBuilder &Bldr,
+                  ExplodedNode *Pred) {
+    llvm::TimeTraceScope TimeScope(
+        checkerScopeName("BlockEntrance", CheckFn.Checker));
+    CheckerContext C(Bldr, Eng, Pred, Entrance.withTag(CheckFn.Checker));
+    CheckFn(Entrance, C);
+  }
+};
+
+} // namespace
+
+void CheckerManager::runCheckersForBlockEntrance(ExplodedNodeSet &Dst,
+                                                 const ExplodedNodeSet &Src,
+                                                 const BlockEntrance &Entrance,
+                                                 ExprEngine &Eng) const {
+  CheckBlockEntranceContext C(BlockEntranceCheckers, Entrance, Eng);
+  llvm::TimeTraceScope TimeScope{"CheckerManager::runCheckersForBlockEntrance"};
+  expandGraphWithCheckers(C, Dst, Src);
+}
+
 void CheckerManager::runCheckersForEndAnalysis(ExplodedGraph &G,
                                                BugReporter &BR,
                                                ExprEngine &Eng) {
@@ -875,6 +911,10 @@ void CheckerManager::_registerForBind(CheckBindFunc checkfn) {
   BindCheckers.push_back(checkfn);
 }
 
+void CheckerManager::_registerForBlockEntrance(CheckBlockEntranceFunc checkfn) {
+  BlockEntranceCheckers.push_back(checkfn);
+}
+
 void CheckerManager::_registerForEndAnalysis(CheckEndAnalysisFunc checkfn) {
   EndAnalysisCheckers.push_back(checkfn);
 }

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

@@ -304,26 +304,37 @@ void CoreEngine::HandleBlockEdge(const BlockEdge &L, ExplodedNode *Pred) {
       }
     }
 
+    ExplodedNodeSet CheckerNodes;
+    BlockEntrance BE(L.getSrc(), L.getDst(), Pred->getLocationContext());
+    ExprEng.runCheckersForBlockEntrance(BuilderCtx, BE, Pred, CheckerNodes);
+
     // Process the final state transition.
-    ExprEng.processEndOfFunction(BuilderCtx, Pred, RS);
+    for (ExplodedNode *P : CheckerNodes) {
+      ExprEng.processEndOfFunction(BuilderCtx, P, RS);
+    }
 
     // This path is done. Don't enqueue any more nodes.
     return;
   }
 
   // Call into the ExprEngine to process entering the CFGBlock.
-  ExplodedNodeSet dstNodes;
   BlockEntrance BE(L.getSrc(), L.getDst(), Pred->getLocationContext());
-  NodeBuilderWithSinks nodeBuilder(Pred, dstNodes, BuilderCtx, BE);
-  ExprEng.processCFGBlockEntrance(L, nodeBuilder, Pred);
+  ExplodedNodeSet DstNodes;
+  NodeBuilderWithSinks NodeBuilder(Pred, DstNodes, BuilderCtx, BE);
+  ExprEng.processCFGBlockEntrance(L, NodeBuilder, Pred);
 
   // Auto-generate a node.
-  if (!nodeBuilder.hasGeneratedNodes()) {
-    nodeBuilder.generateNode(Pred->State, Pred);
+  if (!NodeBuilder.hasGeneratedNodes()) {
+    NodeBuilder.generateNode(Pred->State, Pred);
+  }
+
+  ExplodedNodeSet CheckerNodes;
+  for (auto *N : DstNodes) {
+    ExprEng.runCheckersForBlockEntrance(BuilderCtx, BE, N, CheckerNodes);
   }
 
   // Enqueue nodes onto the worklist.
-  enqueue(dstNodes);
+  enqueue(CheckerNodes);
 }
 
 void CoreEngine::HandleBlockEntrance(const BlockEntrance &L,

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -2617,6 +2617,19 @@ void ExprEngine::processCFGBlockEntrance(const BlockEdge &L,
   }
 }
 
+void ExprEngine::runCheckersForBlockEntrance(const NodeBuilderContext &BldCtx,
+                                             const BlockEntrance &Entrance,
+                                             ExplodedNode *Pred,
+                                             ExplodedNodeSet &Dst) {
+  llvm::PrettyStackTraceFormat CrashInfo(
+      "Processing block entrance B%d -> B%d",
+      Entrance.getPreviousBlock()->getBlockID(),
+      Entrance.getBlock()->getBlockID());
+  currBldrCtx = &BldCtx;
+  getCheckerManager().runCheckersForBlockEntrance(Dst, Pred, Entrance, *this);
+  currBldrCtx = nullptr;
+}
+
 //===----------------------------------------------------------------------===//
 // Branch processing.
 //===----------------------------------------------------------------------===//

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

@@ -711,9 +711,10 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
   }
 
   ExplodedNode *N = Pred;
-  while (!N->getLocation().getAs<BlockEntrance>()) {
+  while (!N->getLocation().getAs<BlockEdge>()) {
     ProgramPoint P = N->getLocation();
-    assert(P.getAs<PreStmt>()|| P.getAs<PreStmtPurgeDeadSymbols>());
+    assert(P.getAs<PreStmt>() || P.getAs<PreStmtPurgeDeadSymbols>() ||
+           P.getAs<BlockEntrance>());
     (void) P;
     if (N->pred_size() != 1) {
       // We failed to track back where we came from.
@@ -729,7 +730,6 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
     return;
   }
 
-  N = *N->pred_begin();
   BlockEdge BE = N->getLocation().castAs<BlockEdge>();
   SVal X;