[analyzer] Checker: check::BeginAnalysis

[Charusso]

Summary

This callback is called when the analysis starts. It could be used to
construct the checker's fields which are related to the Preprocessor.

void CoolFunctionChecker::checkBeginAnalysis(CheckerContext &C) const {
  Preprocessor &PP = C.getPreprocessor();
  UseMyCoolLib = PP.isMacroDefined("__MY_COOL_LIB__");
}

[Charusso]

I felt that it will be easier, eh.

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

118

I think we need to merge the BeginFunction into the BeginWorklist.

[Szelethus]

YES PLEASE. Debug checkers that only dump from check::EndAnalysis won't rely on the analysis not actually crashing. Which is ironically exactly when we want to use them.

clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp

199

Hmmm, what use case do you have in mind? Do we actually want this to happen? By the time checkers are constructed, the AST is already built, alongside all the other data structures that will be inspected by the analyzer. Since checkers are constructed once for each clang invocation (not for every BeginAnalysis -- EndAnalysis cycle), I would imagine that the checker registry functions are more appropriate for such initializations.

I'm only aware of one non-debug checker that uses EndAnalysis (maybe RetailCount?), that clears some sort of a locally stored map that it shouldn't really have anyways (since checkers are supposed to be stateless), and I struggle to think of a realistic, supported use case for this callback other than debug checkers.

199

Turns out deadcode.UnreachableCode uses EndAnalysis quite cleverly as well.

[Charusso]

• Do not restrict what you supposed to do with the callback.

[Charusso]

In D69745#1732348, @Szelethus wrote:

YES PLEASE. Debug checkers that only dump from check::EndAnalysis won't rely on the analysis not actually crashing. Which is ironically exactly when we want to use them.

Thanks for the idea! I am not that cool with debug checkers and I think you are the first user who made a creative decision how to use this. My use case is the Preprocessor only if I do not touch the checker-registry based construction.

clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp

199

Yes, good point. It is made for the Preprocessor mainly which is very path-sensitive, but that is not that good idea to construct e.g. the BugType multiple times. So that, I do not want to restrict creativity and I have removed that line.

[NoQ]

I think we need to merge the BeginFunction into the BeginWorklist.

Basically this, but the other way round: your BeginAnalysis is BeginFunction with extra steps (namely, checking C.inTopFrame()).

Backstory: I wanted to add BeginAnalysis a few years ago because i wanted to mark argc and argv as tainted when analyzing main(). But then i noticed that we already have BeginFunction so i decided not to add it. No, i didn't mark them as tainted yet :/

YES PLEASE. Debug checkers that only dump from check::EndAnalysis won't rely on the analysis not actually crashing.

There are currently two such checkers: debug.ViewExplodedGraph and debug.Stats. It doesn't make sense to move them to BeginAnalysis though, because the data that they're dumping isn't available yet. If the checker dumps data that isn't collected during path sensitive analysis, it should subscribe to ASTCodeBody instead. This way activating the checker won't trigger path-sensitive analysis to occur.

[Charusso]

In D69745#1732739, @NoQ wrote:

Basically this, but the other way round: your BeginAnalysis is BeginFunction with extra steps (namely, checking C.inTopFrame()).

Backstory: I wanted to add BeginAnalysis a few years ago because i wanted to mark argc and argv as tainted when analyzing main(). But then i noticed that we already have BeginFunction so i decided not to add it. No, i didn't mark them as tainted yet :/

Hm, that is a cool workaround. I think every UnknownSpaceRegion stuff is tainted, and that is a strong mark here.

[NoQ]

In D69745#1732744, @Charusso wrote:

I think every UnknownSpaceRegion stuff is tainted, and that is a strong mark here.

Mmm, no, that's definitely not true. Basically, if you analyze a method of a class as your top frame, you have your whole class reside in UnknownSpaceRegion, being represented as SymRegion{reg_$0<this>}. It doesn't mean that there are no contracts that constrain possible values of fields within the class.

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

 public:
   template <typename CHECKER>
   static void _register(CHECKER *checker, CheckerManager &mgr) {
     mgr._registerForBind(
            CheckerManager::CheckBindFunc(checker, _checkBind<CHECKER>));
   }
 };
 
+class BeginAnalysis {
+  template <typename CHECKER>
+  static void _checkBeginAnalysis(void *checker, CheckerContext &C) {
+    ((const CHECKER *)checker)->checkBeginAnalysis(C);
+  }
+
+public:
+  template <typename CHECKER>
+  static void _register(CHECKER *checker, CheckerManager &mgr) {
+    mgr._registerForBeginAnalysis(CheckerManager::CheckBeginAnalysisFunc(
+        checker, _checkBeginAnalysis<CHECKER>));
+  }
+};
+
 class EndAnalysis {
   template <typename CHECKER>
   static void _checkEndAnalysis(void *checker, ExplodedGraph &G,
                                 BugReporter &BR, ExprEngine &Eng) {
     ((const CHECKER *)checker)->checkEndAnalysis(G, BR, Eng);
   }
 
 public:

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

//===----------------------------------------------------------------------===//
 
   /// Run checkers for binding of a value to a location.
   void runCheckersForBind(ExplodedNodeSet &Dst,
                           const ExplodedNodeSet &Src,
                           SVal location, SVal val,
                           const Stmt *S, ExprEngine &Eng,
                           const ProgramPoint &PP);
 
+  /// Run checkers for begin of analysis.
+  void runCheckersForBeginAnalysis(ExplodedNodeSet &Dst, const BlockEdge &L,
+                                   ExplodedNode *Pred, ExprEngine &Eng);
+
   /// Run checkers for end of analysis.
   void runCheckersForEndAnalysis(ExplodedGraph &G, BugReporter &BR,
                                  ExprEngine &Eng);
 
   /// Run checkers on beginning of function.
   void runCheckersForBeginFunction(ExplodedNodeSet &Dst,
                                    const BlockEdge &L,
                                    ExplodedNode *Pred,
//===----------------------------------------------------------------------===//
   using CheckLocationFunc =
       CheckerFn<void (const SVal &location, bool isLoad, const Stmt *S,
                       CheckerContext &)>;
 
   using CheckBindFunc =
       CheckerFn<void (const SVal &location, const SVal &val, const Stmt *S,
                       CheckerContext &)>;
 
+  using CheckBeginAnalysisFunc = CheckerFn<void(CheckerContext &)>;
+
   using CheckEndAnalysisFunc =
       CheckerFn<void (ExplodedGraph &, BugReporter &, ExprEngine &)>;
 
   using CheckBeginFunctionFunc = CheckerFn<void (CheckerContext &)>;
 
   using CheckEndFunctionFunc =
       CheckerFn<void (const ReturnStmt *, CheckerContext &)>;
 
//===----------------------------------------------------------------------===//
 
   void _registerForPreCall(CheckCallFunc checkfn);
   void _registerForPostCall(CheckCallFunc checkfn);
 
   void _registerForLocation(CheckLocationFunc checkfn);
 
   void _registerForBind(CheckBindFunc checkfn);
 
+  void _registerForBeginAnalysis(CheckBeginAnalysisFunc checkfn);
   void _registerForEndAnalysis(CheckEndAnalysisFunc checkfn);
 
   void _registerForBeginFunction(CheckBeginFunctionFunc checkfn);
   void _registerForEndFunction(CheckEndFunctionFunc checkfn);
 
   void _registerForBranchCondition(CheckBranchConditionFunc checkfn);
 
   void _registerForNewAllocator(CheckNewAllocatorFunc checkfn);
private:
 
   std::vector<CheckCallFunc> PreCallCheckers;
   std::vector<CheckCallFunc> PostCallCheckers;
 
   std::vector<CheckLocationFunc> LocationCheckers;
 
   std::vector<CheckBindFunc> BindCheckers;
 
+  std::vector<CheckBeginAnalysisFunc> BeginAnalysisCheckers;
   std::vector<CheckEndAnalysisFunc> EndAnalysisCheckers;
 
   std::vector<CheckBeginFunctionFunc> BeginFunctionCheckers;
   std::vector<CheckEndFunctionFunc> EndFunctionCheckers;
 
   std::vector<CheckBranchConditionFunc> BranchConditionCheckers;
 
   std::vector<CheckNewAllocatorFunc> NewAllocatorCheckers;

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

CheckerContext(NodeBuilder &builder,
     assert(Pred->getState() &&
            "We should not call the checkers on an empty state.");
   }
 
   AnalysisManager &getAnalysisManager() {
     return Eng.getAnalysisManager();
   }
 
+  CheckerManager &getCheckerManager() { return Eng.getCheckerManager(); }
+
   ConstraintManager &getConstraintManager() {
     return Eng.getConstraintManager();
   }
 
   StoreManager &getStoreManager() {
     return Eng.getStoreManager();
   }
 

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

public:
   /// Generate the entry node of the callee.
   void processCallEnter(NodeBuilderContext& BC, CallEnter CE,
                         ExplodedNode *Pred) override;
 
   /// Generate the sequence of nodes that simulate the call exit and the post
   /// visit for CallExpr.
   void processCallExit(ExplodedNode *Pred) override;
 
+  /// Called by CoreEngine when the analysis worklist starts.
+  void processBeginWorklist(NodeBuilderContext &NBC, ExplodedNode *Pred,
+                            ExplodedNodeSet &Dst, const BlockEdge &L) override;
+
   /// Called by CoreEngine when the analysis worklist has terminated.
   void processEndWorklist() override;
 
   /// evalAssume - Callback function invoked by the ConstraintManager when
   ///  making assumptions about state values.
   ProgramStateRef processAssume(ProgramStateRef state, SVal cond,
                                 bool assumption) override;
 

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

notifyCheckersOfPointerEscape(ProgramStateRef State,
                            const CallEvent *Call,
                            RegionAndSymbolInvalidationTraits &HTraits) = 0;
 
   /// printJson - Called by ProgramStateManager to print checker-specific data.
   virtual void printJson(raw_ostream &Out, ProgramStateRef State,
                          const LocationContext *LCtx, const char *NL,
                          unsigned int Space, bool IsDot) const = 0;
 
+  /// Called by CoreEngine when the analysis worklist starts.
+  virtual void processBeginWorklist(NodeBuilderContext &NBC, ExplodedNode *Pred,
+                                    ExplodedNodeSet &Dst,
+                                    const BlockEdge &L) = 0;
+
   /// Called by CoreEngine when the analysis worklist is either empty or the
-  //  maximum number of analysis steps have been reached.
+  /// maximum number of analysis steps have been reached.
   virtual void processEndWorklist() = 0;
 };
 
 } // end GR namespace
 
 } // end clang namespace
 
 #endif

clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp

class CheckerDocumentation : public Checker< check::PreStmt<ReturnStmt>,
                                        check::PostCall,
                                        check::BranchCondition,
                                        check::NewAllocator,
                                        check::Location,
                                        check::Bind,
                                        check::DeadSymbols,
                                        check::BeginFunction,
                                        check::EndFunction,
+                                       check::BeginAnalysis,
                                        check::EndAnalysis,
                                        check::EndOfTranslationUnit,
                                        eval::Call,
                                        eval::Assume,
                                        check::LiveSymbols,
                                        check::RegionChanges,
                                        check::PointerEscape,
                                        check::ConstPointerEscape,
public:
 
   /// Called when the analyzer core reaches the end of a
   /// function being analyzed regardless of whether it is analyzed at the top
   /// level or is inlined.
   ///
   /// check::EndFunction
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &Ctx) const {}
 
+  /// Called when the analysis starts.
+  ///
+  /// This callback should be used to construct the checker's fields.
+  ///
+  /// check::BeginAnalysis
+  void checkBeginAnalysis(const Decl *D, BugReporter &BR);
+
   /// Called after all the paths in the ExplodedGraph reach end of path
   /// - the symbolic execution graph is fully explored.
   ///
   /// This callback should be used in cases when a checker needs to have a
   /// global view of the information generated on all paths. For example, to
   /// compare execution summary/result several paths.
   /// See IdempotentOperationChecker for a usage example.
   ///

clang/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
-class TraversalDumper : public Checker< check::BranchCondition,
-                                        check::BeginFunction,
-                                        check::EndFunction > {
+class TraversalDumper
+    : public Checker<check::BeginAnalysis, check::EndAnalysis,
+                     check::BranchCondition, check::BeginFunction,
+                     check::EndFunction> {
 public:
+  void checkBeginAnalysis(CheckerContext &C) const;
+  void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR,
+                        ExprEngine &Eng) const;
   void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const;
   void checkBeginFunction(CheckerContext &C) const;
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
 };
 }
 
 void TraversalDumper::checkBranchCondition(const Stmt *Condition,
                                            CheckerContext &C) const {
   // Special-case Objective-C's for-in loop, which uses the entire loop as its
   // condition. We just print the collection expression.
   const Stmt *Parent = dyn_cast<ObjCForCollectionStmt>(Condition);
   if (!Parent) {
     const ParentMap &Parents = C.getLocationContext()->getParentMap();
     Parent = Parents.getParent(Condition);
   }
 
   // It is mildly evil to print directly to llvm::outs() rather than emitting
   // warnings, but this ensures things do not get filtered out by the rest of
   // the static analyzer machinery.
   SourceLocation Loc = Parent->getBeginLoc();
   llvm::outs() << C.getSourceManager().getSpellingLineNumber(Loc) << " "
                << Parent->getStmtClassName() << "\n";
 }
 
+void TraversalDumper::checkBeginAnalysis(CheckerContext &) const {
+  llvm::outs() << "--BEGIN ANALYSIS--\n";
+}
+
+void TraversalDumper::checkEndAnalysis(ExplodedGraph &G, BugReporter &BR,
+                                       ExprEngine &Eng) const {
+  llvm::outs() << "--END ANALYSIS--\n";
+}
+
 void TraversalDumper::checkBeginFunction(CheckerContext &C) const {
   llvm::outs() << "--BEGIN FUNCTION--\n";
 }
 
 void TraversalDumper::checkEndFunction(const ReturnStmt *RS,
                                        CheckerContext &C) const {
   llvm::outs() << "--END FUNCTION--\n";
 }

clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

void CheckerManager::runCheckersForBind(ExplodedNodeSet &Dst,
                                         const ExplodedNodeSet &Src,
                                         SVal location, SVal val,
                                         const Stmt *S, ExprEngine &Eng,
                                         const ProgramPoint &PP) {
   CheckBindContext C(BindCheckers, location, val, S, Eng, PP);
   expandGraphWithCheckers(C, Dst, Src);
 }
 
+namespace {
+struct CheckBeginAnalysisContext {
+  using CheckersTy = std::vector<CheckerManager::CheckBeginAnalysisFunc>;
+
+  const CheckersTy &Checkers;
+  ExprEngine &Eng;
+  const ProgramPoint &PP;
+
+  CheckBeginAnalysisContext(const CheckersTy &Checkers, ExprEngine &Eng,
+                            const ProgramPoint &PP)
+      : Checkers(Checkers), Eng(Eng), PP(PP) {}
+
+  CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); }
+  CheckersTy::const_iterator checkers_end() { return Checkers.end(); }
+
+  void runChecker(CheckerManager::CheckBeginAnalysisFunc checkFn,
+                  NodeBuilder &Bldr, ExplodedNode *Pred) {
+    const ProgramPoint &L = PP.withTag(checkFn.Checker);
+    CheckerContext C(Bldr, Eng, Pred, L);
+
+    checkFn(C);
+  }
+};
+} // namespace
+
+void CheckerManager::runCheckersForBeginAnalysis(ExplodedNodeSet &Dst,
+                                                 const BlockEdge &L,
+                                                 ExplodedNode *Pred,
+                                                 ExprEngine &Eng) {
+  ExplodedNodeSet Src;
+  Src.insert(Pred);
+  CheckBeginAnalysisContext C(BeginAnalysisCheckers, Eng, L);
+  expandGraphWithCheckers(C, Dst, Src);
+}
+
 void CheckerManager::runCheckersForEndAnalysis(ExplodedGraph &G,
                                                BugReporter &BR,
                                                ExprEngine &Eng) {
   for (const auto EndAnalysisChecker : EndAnalysisCheckers)
     EndAnalysisChecker(G, BR, Eng);
 }
 
 namespace {

 void CheckerManager::_registerForLocation(CheckLocationFunc checkfn) {
   LocationCheckers.push_back(checkfn);
 }
 
 void CheckerManager::_registerForBind(CheckBindFunc checkfn) {
   BindCheckers.push_back(checkfn);
 }
 
+void CheckerManager::_registerForBeginAnalysis(CheckBeginAnalysisFunc checkfn) {
+  BeginAnalysisCheckers.push_back(checkfn);
+}
+
 void CheckerManager::_registerForEndAnalysis(CheckEndAnalysisFunc checkfn) {
   EndAnalysisCheckers.push_back(checkfn);
 }
 
 void CheckerManager::_registerForBeginFunction(CheckBeginFunctionFunc checkfn) {
   BeginFunctionCheckers.push_back(checkfn);
 }
 

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

if (G.num_roots() == 0) { // Initialize the analysis by constructing
 
     bool IsNew;
     ExplodedNode *Node = G.getNode(StartLoc, InitState, false, &IsNew);
     assert(IsNew);
     G.addRoot(Node);
 
     NodeBuilderContext BuilderCtx(*this, StartLoc.getDst(), Node);
     ExplodedNodeSet DstBegin;
+
+    SubEng.processBeginWorklist(BuilderCtx, Node, DstBegin, StartLoc);
     SubEng.processBeginOfFunction(BuilderCtx, Node, DstBegin, StartLoc);
 
     enqueue(DstBegin);
   }
 
   // Check if we have a steps limit
   bool UnlimitedSteps = Steps == 0;
   // Cap our pre-reservation in the event that the user specifies
   // a very large number of maximum steps.

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

void ExprEngine::printJson(raw_ostream &Out, ProgramStateRef State,
   } else {
     Out << "null," << NL;
   }
 
   getCheckerManager().runCheckersForPrintStateJson(Out, State, NL, Space,
                                                    IsDot);
 }
 
+void ExprEngine::processBeginWorklist(NodeBuilderContext &NBC,
+                                      ExplodedNode *Pred, ExplodedNodeSet &Dst,
+                                      const BlockEdge &L) {
+  SaveAndRestore<const NodeBuilderContext *> NodeContextRAII(currBldrCtx, &NBC);
+  getCheckerManager().runCheckersForBeginAnalysis(Dst, L, Pred, *this);
+}
+
 void ExprEngine::processEndWorklist() {
   getCheckerManager().runCheckersForEndAnalysis(G, BR, *this);
 }
 
 void ExprEngine::processCFGElement(const CFGElement E, ExplodedNode *Pred,
                                    unsigned StmtIdx, NodeBuilderContext *Ctx) {
   PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
   currStmtIdx = StmtIdx;

clang/test/Analysis/traversal-begin-end-function.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.DumpTraversal %s | FileCheck %s
 
 void inline_callee(int i);
 
-// CHECK: --BEGIN FUNCTION--
 void inline_caller() {
-  // CHECK: --BEGIN FUNCTION--
-  // CHECK: --BEGIN FUNCTION--
-  // CHECK: --BEGIN FUNCTION--
   inline_callee(3);
-  // CHECK: --END FUNCTION--
-  // CHECK: --END FUNCTION--
-  // CHECK: --END FUNCTION--
 }
-// CHECK: --END FUNCTION--
 
 void inline_callee(int i) {
   if (i <= 1)
     return;
 
   inline_callee(i - 1);
 }
+
+// CHECK:      --BEGIN ANALYSIS--
+// CHECK-NEXT: --BEGIN FUNCTION--
+// CHECK-NEXT: --BEGIN FUNCTION--
+// CHECK-NEXT: IfStmt
+// CHECK-NEXT: --BEGIN FUNCTION--
+// CHECK-NEXT: IfStmt
+// CHECK-NEXT: --BEGIN FUNCTION--
+// CHECK-NEXT: IfStmt
+// CHECK-NEXT: --END FUNCTION--
+// CHECK-NEXT: --END FUNCTION--
+// CHECK-NEXT: --END FUNCTION--
+// CHECK-NEXT: --END FUNCTION--
+// CHECK-NEXT: --END ANALYSIS--