Catch uses of undefined values when they are used in assignment, thus catching such bugs closer to the source.

llvm-svn: 86003

clang/include/clang/Analysis/PathSensitive/Checker.h

@@ -114,11 +114,19 @@ private:
     _PreVisit(C, stmt);
   }
 
-
+  void GR_VisitBind(ExplodedNodeSet &Dst,
+                    GRStmtNodeBuilder &Builder, GRExprEngine &Eng,
+                    const Stmt *stmt, ExplodedNode *Pred, void *tag, 
+                    SVal location, SVal val,
+                    bool isPrevisit) {
+    CheckerContext C(Dst, Builder, Eng, Pred, tag, isPrevisit);
+    assert(isPrevisit && "Only previsit supported for now.");
+    PreVisitBind(C, stmt, location, val);
+  }
 
 public:
   virtual ~Checker() {}
-  virtual void _PreVisit(CheckerContext &C, const Stmt *stmt) {}
+  virtual void _PreVisit(CheckerContext &C, const Stmt *ST) {}
   
   // This is a previsit which takes a node returns a node.
   virtual ExplodedNode *CheckLocation(const Stmt *S, ExplodedNode *Pred,
@@ -126,6 +134,9 @@ public:
                                       GRExprEngine &Eng) {
     return Pred;
   }
+  
+  virtual void PreVisitBind(CheckerContext &C, const Stmt *ST, 
+                            SVal location, SVal val) {}
 
   virtual ExplodedNode *CheckType(QualType T, ExplodedNode *Pred, 
                                   const GRState *state, Stmt *S, 

clang/include/clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h

@@ -0,0 +1,32 @@
+//===--- UndefinedAssignmentChecker.h ---------------------------*- C++ -*--==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines UndefinedAssginmentChecker, a builtin check in GRExprEngine that
+// checks for assigning undefined values.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_UNDEFASSIGNMENTCHECKER
+#define LLVM_CLANG_UNDEFASSIGNMENTCHECKER
+
+#include "clang/Analysis/PathSensitive/CheckerVisitor.h"
+
+namespace clang {
+class UndefinedAssignmentChecker
+  : public CheckerVisitor<UndefinedAssignmentChecker> {
+  BugType *BT;
+public:
+  UndefinedAssignmentChecker() : BT(0) {}
+  static void *getTag();
+  virtual void PreVisitBind(CheckerContext &C, const Stmt *S, SVal location,
+                            SVal val);
+};
+}
+#endif
+

clang/include/clang/Analysis/PathSensitive/GRExprEngine.h

@@ -411,6 +411,10 @@ protected:
   ///  at specific statements.
   void CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src, 
                     bool isPrevisit);
+  
+  void CheckerVisitBind(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src, 
+                        SVal location, SVal val, bool isPrevisit);
+
 
   /// Visit - Transfer function logic for all statements.  Dispatches to
   ///  other functions that handle specific kinds of statements.

clang/lib/Analysis/GRExprEngine.cpp

@@ -139,6 +139,41 @@ void GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst,
   // automatically.
 }
 
+// FIXME: This is largely copy-paste from CheckerVisit().  Need to 
+// unify.
+void GRExprEngine::CheckerVisitBind(Stmt *S, ExplodedNodeSet &Dst,
+                                    ExplodedNodeSet &Src,
+                                    SVal location, SVal val, bool isPrevisit) {
+  
+  if (Checkers.empty()) {
+    Dst = Src;
+    return;
+  }
+  
+  ExplodedNodeSet Tmp;
+  ExplodedNodeSet *PrevSet = &Src;
+  
+  for (CheckersOrdered::iterator I=Checkers.begin(),E=Checkers.end(); I!=E; ++I)
+  {
+    ExplodedNodeSet *CurrSet = (I+1 == E) ? &Dst 
+    : (PrevSet == &Tmp) ? &Src : &Tmp;
+    
+    CurrSet->clear();
+    void *tag = I->first;
+    Checker *checker = I->second;
+    
+    for (ExplodedNodeSet::iterator NI = PrevSet->begin(), NE = PrevSet->end();
+         NI != NE; ++NI)
+      checker->GR_VisitBind(*CurrSet, *Builder, *this, S, *NI, tag, location,
+                            val, isPrevisit);
+    
+    // Update which NodeSet is the current one.
+    PrevSet = CurrSet;
+  }
+  
+  // Don't autotransition.  The CheckerContext objects should do this
+  // automatically.
+}
 //===----------------------------------------------------------------------===//
 // Engine construction and deletion.
 //===----------------------------------------------------------------------===//
@@ -1093,36 +1128,49 @@ void GRExprEngine::VisitMemberExpr(MemberExpr* M, ExplodedNode* Pred,
 void GRExprEngine::EvalBind(ExplodedNodeSet& Dst, Stmt* Ex, ExplodedNode* Pred,
                             const GRState* state, SVal location, SVal Val,
                             bool atDeclInit) {
+  
+  
+  // Do a previsit of the bind.
+  ExplodedNodeSet CheckedSet, Src;
+  Src.Add(Pred);
+  CheckerVisitBind(Ex, CheckedSet, Src, location, Val, true);
+  
+  for (ExplodedNodeSet::iterator I = CheckedSet.begin(), E = CheckedSet.end();
+       I!=E; ++I) {
+    
+    if (Pred != *I)
+      state = GetState(*I);
+    
+    const GRState* newState = 0;
 
-  const GRState* newState = 0;
+    if (atDeclInit) {
+      const VarRegion *VR =
+        cast<VarRegion>(cast<loc::MemRegionVal>(location).getRegion());
 
-  if (atDeclInit) {
-    const VarRegion *VR =
-      cast<VarRegion>(cast<loc::MemRegionVal>(location).getRegion());
-
-    newState = state->bindDecl(VR, Val);
-  }
-  else {
-    if (location.isUnknown()) {
-      // We know that the new state will be the same as the old state since
-      // the location of the binding is "unknown".  Consequently, there
-      // is no reason to just create a new node.
-      newState = state;
+      newState = state->bindDecl(VR, Val);
     }
     else {
-      // We are binding to a value other than 'unknown'.  Perform the binding
-      // using the StoreManager.
-      newState = state->bindLoc(cast<Loc>(location), Val);
+      if (location.isUnknown()) {
+        // We know that the new state will be the same as the old state since
+        // the location of the binding is "unknown".  Consequently, there
+        // is no reason to just create a new node.
+        newState = state;
+      }
+      else {
+        // We are binding to a value other than 'unknown'.  Perform the binding
+        // using the StoreManager.
+        newState = state->bindLoc(cast<Loc>(location), Val);
+      }
     }
+
+    // The next thing to do is check if the GRTransferFuncs object wants to
+    // update the state based on the new binding.  If the GRTransferFunc object
+    // doesn't do anything, just auto-propagate the current state.
+    GRStmtNodeBuilderRef BuilderRef(Dst, *Builder, *this, *I, newState, Ex,
+                                    newState != state);
+
+    getTF().EvalBind(BuilderRef, location, Val);
   }
-
-  // The next thing to do is check if the GRTransferFuncs object wants to
-  // update the state based on the new binding.  If the GRTransferFunc object
-  // doesn't do anything, just auto-propagate the current state.
-  GRStmtNodeBuilderRef BuilderRef(Dst, *Builder, *this, Pred, newState, Ex,
-                                  newState != state);
-
-  getTF().EvalBind(BuilderRef, location, Val);
 }
 
 /// EvalStore - Handle the semantics of a store via an assignment.

clang/lib/Analysis/GRExprEngineInternalChecks.cpp

@@ -19,6 +19,7 @@
 #include "clang/Analysis/PathSensitive/Checkers/DivZeroChecker.h"
 #include "clang/Analysis/PathSensitive/Checkers/BadCallChecker.h"
 #include "clang/Analysis/PathSensitive/Checkers/UndefinedArgChecker.h"
+#include "clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h"
 #include "clang/Analysis/PathSensitive/Checkers/AttrNonNullChecker.h"
 #include "clang/Analysis/PathSensitive/Checkers/VLASizeChecker.h"
 #include "clang/Analysis/PathDiagnostic.h"
@@ -476,12 +477,13 @@ void GRExprEngine::RegisterInternalChecks() {
   // their associated BugType will get registered with the BugReporter
   // automatically.  Note that the check itself is owned by the GRExprEngine
   // object.
-  registerCheck<AttrNonNullChecker>(new AttrNonNullChecker());
-  registerCheck<UndefinedArgChecker>(new UndefinedArgChecker());
-  registerCheck<BadCallChecker>(new BadCallChecker());
-  registerCheck<DivZeroChecker>(new DivZeroChecker());
-  registerCheck<UndefDerefChecker>(new UndefDerefChecker());
-  registerCheck<NullDerefChecker>(new NullDerefChecker());
-  registerCheck<UndefSizedVLAChecker>(new UndefSizedVLAChecker());
-  registerCheck<ZeroSizedVLAChecker>(new ZeroSizedVLAChecker());
+  registerCheck(new AttrNonNullChecker());
+  registerCheck(new UndefinedArgChecker());
+  registerCheck(new UndefinedAssignmentChecker());
+  registerCheck(new BadCallChecker());
+  registerCheck(new DivZeroChecker());
+  registerCheck(new UndefDerefChecker());
+  registerCheck(new NullDerefChecker());
+  registerCheck(new UndefSizedVLAChecker());
+  registerCheck(new ZeroSizedVLAChecker());
 }

clang/lib/Analysis/UndefinedAssignmentChecker.cpp

@@ -0,0 +1,59 @@
+//===--- UndefinedAssignmentChecker.h ---------------------------*- C++ -*--==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines UndefinedAssginmentChecker, a builtin check in GRExprEngine that
+// checks for assigning undefined values.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h"
+#include "clang/Analysis/PathSensitive/BugReporter.h"
+
+using namespace clang;
+
+void *UndefinedAssignmentChecker::getTag() {
+  static int x = 0;
+  return &x;
+}
+
+void UndefinedAssignmentChecker::PreVisitBind(CheckerContext &C, 
+                                              const Stmt *S,
+                                              SVal location,
+                                              SVal val) {
+  if (!val.isUndef())
+    return;
+
+  ExplodedNode *N = C.GenerateNode(S, true);
+
+  if (!N)
+    return;
+
+  if (!BT)
+    BT = new BugType("Assigned value is garbage or undefined",
+                     "Logic error");
+
+  // Generate a report for this bug.
+  EnhancedBugReport *R = new EnhancedBugReport(*BT, BT->getName().c_str(), N);
+  const Expr *ex = 0;
+
+  if (const BinaryOperator *B = dyn_cast<BinaryOperator>(S))
+    ex = B->getRHS();
+  else if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
+    const VarDecl* VD = dyn_cast<VarDecl>(DS->getSingleDecl());
+    ex = VD->getInit();
+  }
+
+  if (ex) {
+    R->addRange(ex->getSourceRange());
+    R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, ex);
+  }
+
+  C.EmitReport(R);
+}  
+

clang/test/Analysis/uninit-vals-ps-region.c

@@ -24,9 +24,20 @@ void test_uninit_pos() {
   struct TestUninit v1 = { 0, 0 };
   struct TestUninit v2 = test_uninit_aux();
   int z;
-  v1.y = z;
-  test_unit_aux2(v2.x + v1.y);  // expected-warning{{The right operand of '+' is a garbage value}}
+  v1.y = z; // expected-warning{{Assigned value is garbage or undefined}}
+  test_unit_aux2(v2.x + v1.y);
 }
+void test_uninit_pos_2() {
+  struct TestUninit v1 = { 0, 0 };
+  struct TestUninit v2;
+  test_unit_aux2(v2.x + v1.y);  // expected-warning{{The left operand of '+' is a garbage value}}
+}
+void test_uninit_pos_3() {
+  struct TestUninit v1 = { 0, 0 };
+  struct TestUninit v2;
+  test_unit_aux2(v1.y + v2.x);  // expected-warning{{The right operand of '+' is a garbage value}}
+}
+
 void test_uninit_neg() {
   struct TestUninit v1 = { 0, 0 };
   struct TestUninit v2 = test_uninit_aux();