Initial refactor of UndefBranchChecker. We still use GRBranchNodeBuilder

in the checker directly. But I don't have a better approach for now.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@89640 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 103 insertions, 41 deletions

diff --git a/include/clang/Analysis/PathSensitive/Checker.h b/include/clang/Analysis/PathSensitive/Checker.h
index b1187214ab..bfc431d07a 100644
--- a/include/clang/Analysis/PathSensitive/Checker.h
+++ b/include/clang/Analysis/PathSensitive/Checker.h
@@ -172,6 +172,10 @@ public:
                                SymbolReaper &SymReaper) {}
   virtual void EvalEndPath(GREndPathNodeBuilder &B, void *tag,
                            GRExprEngine &Eng) {}
+
+  virtual void VisitBranchCondition(GRBranchNodeBuilder &Builder,
+                                    GRExprEngine &Eng,
+                                    Stmt *Condition, void *tag) {}
 };
 } // end clang namespace
 
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 61277ba560..4f3440d88a 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -771,55 +771,49 @@ void GRExprEngine::ProcessBranch(Stmt* Condition, Stmt* Term,
                                 Condition->getLocStart(),
                                 "Error evaluating branch");
 
-  const GRState* PrevState = builder.getState();
-  SVal X = PrevState->getSVal(Condition);
-  DefinedSVal *V = NULL;
-  
-  while (true) {
-    V = dyn_cast<DefinedSVal>(&X);
-
-    if (!V) {
-      if (X.isUnknown()) {
-        if (const Expr *Ex = dyn_cast<Expr>(Condition)) {
-          if (Ex->getType()->isIntegerType()) {
-            // Try to recover some path-sensitivity.  Right now casts of symbolic
-            // integers that promote their values are currently not tracked well.
-            // If 'Condition' is such an expression, try and recover the
-            // underlying value and use that instead.
-            SVal recovered = RecoverCastedSymbol(getStateManager(),
-                                                 builder.getState(), Condition,
-                                                 getContext());
-
-            if (!recovered.isUnknown()) {
-              X = recovered;
-              continue;
-            }
-          }
-        }    
+  for (CheckersOrdered::iterator I=Checkers.begin(),E=Checkers.end();I!=E;++I) {
+    void *tag = I->first;
+    Checker *checker = I->second;
+    checker->VisitBranchCondition(builder, *this, Condition, tag);
+  }
 
-        builder.generateNode(MarkBranch(PrevState, Term, true), true);
-        builder.generateNode(MarkBranch(PrevState, Term, false), false);
-        return;
-      }
+  // If the branch condition is undefined, return;
+  if (!builder.isFeasible(true) && !builder.isFeasible(false))
+    return;
 
-      assert(X.isUndef());
-      ExplodedNode *N = builder.generateNode(PrevState, true);
+  const GRState* PrevState = builder.getState();
+  SVal X = PrevState->getSVal(Condition);
 
-      if (N) {
-        N->markAsSink();
-        UndefBranches.insert(N);
+  if (X.isUnknown()) {
+    // Give it a chance to recover from unknown.
+    if (const Expr *Ex = dyn_cast<Expr>(Condition)) {
+      if (Ex->getType()->isIntegerType()) {
+        // Try to recover some path-sensitivity.  Right now casts of symbolic
+        // integers that promote their values are currently not tracked well.
+        // If 'Condition' is such an expression, try and recover the
+        // underlying value and use that instead.
+        SVal recovered = RecoverCastedSymbol(getStateManager(),
+                                             builder.getState(), Condition,
+                                             getContext());
+        
+        if (!recovered.isUnknown()) {
+          X = recovered;
+        }
       }
-
-      builder.markInfeasible(false);
+    }
+    // If the condition is still unknown, give up.
+    if (X.isUnknown()) {
+      builder.generateNode(MarkBranch(PrevState, Term, true), true);
+      builder.generateNode(MarkBranch(PrevState, Term, false), false);
       return;
     }
-    
-    break;
   }
 
+  DefinedSVal V = cast<DefinedSVal>(X);
+
   // Process the true branch.
   if (builder.isFeasible(true)) {
-    if (const GRState *state = PrevState->Assume(*V, true))
+    if (const GRState *state = PrevState->Assume(V, true))
       builder.generateNode(MarkBranch(state, Term, true), true);
     else
       builder.markInfeasible(true);
@@ -827,7 +821,7 @@ void GRExprEngine::ProcessBranch(Stmt* Condition, Stmt* Term,
 
   // Process the false branch.
   if (builder.isFeasible(false)) {
-    if (const GRState *state = PrevState->Assume(*V, false))
+    if (const GRState *state = PrevState->Assume(V, false))
       builder.generateNode(MarkBranch(state, Term, false), false);
     else
       builder.markInfeasible(false);
diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp
index e5eba0a5b0..d5fe20019d 100644
--- a/lib/Analysis/GRExprEngineInternalChecks.cpp
+++ b/lib/Analysis/GRExprEngineInternalChecks.cpp
@@ -306,7 +306,6 @@ void GRExprEngine::RegisterInternalChecks() {
   // create BugReports on-the-fly but instead wait until GRExprEngine finishes
   // analyzing a function.  Generation of BugReport objects is done via a call
   // to 'FlushReports' from BugReporter.
-  BR.Register(new UndefBranch(this));
   BR.Register(new UndefResult(this));
   BR.Register(new NilReceiverStructRet(this));
   BR.Register(new NilReceiverLargerThanVoidPtrRet(this));
@@ -325,4 +324,5 @@ void GRExprEngine::RegisterInternalChecks() {
   RegisterReturnUndefChecker(*this);
   RegisterUndefinedArraySubscriptChecker(*this);
   RegisterUndefinedAssignmentChecker(*this);
+  RegisterUndefBranchChecker(*this);
 }
diff --git a/lib/Analysis/GRExprEngineInternalChecks.h b/lib/Analysis/GRExprEngineInternalChecks.h
index 49c790a4ee..bbaf4df638 100644
--- a/lib/Analysis/GRExprEngineInternalChecks.h
+++ b/lib/Analysis/GRExprEngineInternalChecks.h
@@ -34,6 +34,7 @@ void RegisterUndefinedArgChecker(GRExprEngine &Eng);
 void RegisterArrayBoundChecker(GRExprEngine &Eng);
 void RegisterUndefinedArraySubscriptChecker(GRExprEngine &Eng);
 void RegisterUndefinedAssignmentChecker(GRExprEngine &Eng);
+void RegisterUndefBranchChecker(GRExprEngine &Eng);
 
 } // end clang namespace
 #endif
diff --git a/lib/Analysis/UndefBranchChecker.cpp b/lib/Analysis/UndefBranchChecker.cpp
new file mode 100644
index 0000000000..38cd107a9d
--- /dev/null
+++ b/lib/Analysis/UndefBranchChecker.cpp
@@ -0,0 +1,63 @@
+//=== UndefBranchChecker.cpp -----------------------------------*- C++ -*--===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines UndefBranchChecker, which checks for undefined branch
+// condition.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GRExprEngineInternalChecks.h"
+#include "clang/Analysis/PathSensitive/Checker.h"
+
+using namespace clang;
+
+namespace {
+
+class VISIBILITY_HIDDEN UndefBranchChecker : public Checker {
+  BuiltinBug *BT;
+public:
+  UndefBranchChecker() : BT(0) {}
+  static void *getTag();
+  void VisitBranchCondition(GRBranchNodeBuilder &Builder, GRExprEngine &Eng,
+                            Stmt *Condition, void *tag);
+};
+
+}
+
+void clang::RegisterUndefBranchChecker(GRExprEngine &Eng) {
+  Eng.registerCheck(new UndefBranchChecker());
+}
+
+void *UndefBranchChecker::getTag() {
+  static int x;
+  return &x;
+}
+
+void UndefBranchChecker::VisitBranchCondition(GRBranchNodeBuilder &Builder, 
+                                              GRExprEngine &Eng,
+                                              Stmt *Condition, void *tag) {
+  const GRState *state = Builder.getState();
+  SVal X = state->getSVal(Condition);
+  if (X.isUndef()) {
+    ExplodedNode *N = Builder.generateNode(state, true);
+    if (N) {
+      N->markAsSink();
+      if (!BT)
+        BT = new BuiltinBug("Undefined branch",
+                 "Branch condition evaluates to an undefined or garbage value");
+      EnhancedBugReport *R = new EnhancedBugReport(*BT, BT->getDescription(),N);
+      R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, 
+                           Condition);
+      Eng.getBugReporter().EmitReport(R);
+    }
+
+    Builder.markInfeasible(true);
+    Builder.markInfeasible(false);
+  }
+}