From 79affb7b94f06e462c3a4998f84b0a43af79471e Mon Sep 17 00:00:00 2001
From: Zhongxing Xu <xuzhongxing@gmail.com>
Date: Wed, 2 Sep 2009 08:10:35 +0000
Subject: [PATCH] Refactor bad callee check into a Checker. Now bad callee is
 checked as a PreVisit to the CallExpr.

llvm-svn: 80771
---
 clang/lib/Analysis/BugReporterVisitors.cpp    |  3 +-
 clang/lib/Analysis/GRExprEngine.cpp           | 15 +-----
 .../Analysis/GRExprEngineInternalChecks.cpp   | 46 +++++++++++++------
 3 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/clang/lib/Analysis/BugReporterVisitors.cpp b/clang/lib/Analysis/BugReporterVisitors.cpp
index 5c31066eff4b..fce31e70f0b2 100644
--- a/clang/lib/Analysis/BugReporterVisitors.cpp
+++ b/clang/lib/Analysis/BugReporterVisitors.cpp
@@ -63,7 +63,8 @@ clang::bugreporter::GetDenomExpr(const ExplodedNode *N) {
 
 const Stmt*
 clang::bugreporter::GetCalleeExpr(const ExplodedNode *N) {
-  const Stmt *S = N->getLocationAs<PostStmt>()->getStmt();
+  // Callee is checked as a PreVisit to the CallExpr.
+  const Stmt *S = N->getLocationAs<PreStmt>()->getStmt();
   if (const CallExpr *CE = dyn_cast<CallExpr>(S))
     return CE->getCallee();
   return NULL;
diff --git a/clang/lib/Analysis/GRExprEngine.cpp b/clang/lib/Analysis/GRExprEngine.cpp
index d0b6bbf0956c..a6b580b865ac 100644
--- a/clang/lib/Analysis/GRExprEngine.cpp
+++ b/clang/lib/Analysis/GRExprEngine.cpp
@@ -1493,20 +1493,7 @@ void GRExprEngine::VisitCallRec(CallExpr* CE, ExplodedNode* Pred,
 
     // FIXME: Add support for symbolic function calls (calls involving
     //  function pointer values that are symbolic).
-    
-    // Check for undefined control-flow or calls to NULL.
-    
-    if (L.isUndef() || isa<loc::ConcreteInt>(L)) {      
-      ExplodedNode* N = Builder->generateNode(CE, state, *DI);
-      
-      if (N) {
-        N->markAsSink();
-        BadCalls.insert(N);
-      }
-      
-      continue;
-    }
-    
+
     // Check for the "noreturn" attribute.
     
     SaveAndRestore<bool> OldSink(Builder->BuildSinks);
diff --git a/clang/lib/Analysis/GRExprEngineInternalChecks.cpp b/clang/lib/Analysis/GRExprEngineInternalChecks.cpp
index 91b7cf3db6f7..4c6371c44c7f 100644
--- a/clang/lib/Analysis/GRExprEngineInternalChecks.cpp
+++ b/clang/lib/Analysis/GRExprEngineInternalChecks.cpp
@@ -70,7 +70,7 @@ public:
 
   const std::string &getDescription() const { return desc; }
   
-  virtual void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) = 0;
+  virtual void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {}
 
   void FlushReports(BugReporter& BR) { FlushReportsImpl(BR, Eng); }
   
@@ -220,14 +220,10 @@ public:
   
 class VISIBILITY_HIDDEN BadCall : public BuiltinBug {
 public:
-  BadCall(GRExprEngine *eng)
+  BadCall(GRExprEngine *eng = 0)
   : BuiltinBug(eng, "Invalid function call",
         "Called function pointer is a null or undefined pointer value") {}
   
-  void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
-    Emit(BR, Eng.bad_calls_begin(), Eng.bad_calls_end());
-  }
-  
   void registerInitialVisitors(BugReporterContext& BRC,
                                const ExplodedNode* N,
                                BuiltinBugReport *R) {
@@ -252,18 +248,12 @@ public:
 
 class VISIBILITY_HIDDEN BadArg : public BuiltinBug {
 public:  
-  BadArg() : BuiltinBug(0, "Uninitialized argument",
-                    "Pass-by-value argument in function call is undefined.") {}
-
-  BadArg(GRExprEngine* eng) : BuiltinBug(eng,"Uninitialized argument",  
+  BadArg(GRExprEngine* eng=0) : BuiltinBug(eng,"Uninitialized argument",  
     "Pass-by-value argument in function call is undefined.") {}
 
   BadArg(GRExprEngine* eng, const char* d)
     : BuiltinBug(eng,"Uninitialized argument", d) {}
   
-  void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
-  }
-
   void registerInitialVisitors(BugReporterContext& BRC,
                                const ExplodedNode* N,
                                BuiltinBugReport *R) {
@@ -662,6 +652,34 @@ void CheckUndefinedArg::PreVisitCallExpr(CheckerContext &C, const CallExpr *CE){
   }
 }
 
+class VISIBILITY_HIDDEN CheckBadCall : public CheckerVisitor<CheckBadCall> {
+  BadCall *BT;
+
+public:
+  CheckBadCall() : BT(0) {}
+  ~CheckBadCall() {}
+
+  const void *getTag() {
+    static int x = 0;
+    return &x;
+  }
+
+  void PreVisitCallExpr(CheckerContext &C, const CallExpr *CE);
+};
+
+void CheckBadCall::PreVisitCallExpr(CheckerContext &C, const CallExpr *CE) {
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  SVal L = C.getState()->getSVal(Callee);
+
+  if (L.isUndef() || isa<loc::ConcreteInt>(L)) {
+    if (ExplodedNode *N = C.generateNode(CE, C.getState(), true)) {
+      if (!BT)
+        BT = new BadCall();
+      C.EmitReport(new BuiltinBugReport(*BT, BT->getDescription().c_str(), N));
+    }
+  }
+}
+
 }
 //===----------------------------------------------------------------------===//
 // Check registration.
@@ -678,7 +696,6 @@ void GRExprEngine::RegisterInternalChecks() {
   BR.Register(new UndefBranch(this));
   BR.Register(new DivZero(this));
   BR.Register(new UndefResult(this));
-  BR.Register(new BadCall(this));
   BR.Register(new RetStack(this));
   BR.Register(new RetUndef(this));
   BR.Register(new BadMsgExprArg(this));
@@ -695,4 +712,5 @@ void GRExprEngine::RegisterInternalChecks() {
   // object.
   registerCheck(new CheckAttrNonNull());
   registerCheck(new CheckUndefinedArg());
+  registerCheck(new CheckBadCall());
 }