[cfe-commits] r47726 - in /cfe/trunk: Analysis/GRExprEngine.cpp Analysis/GRSimpleVals.cpp Analysis/RValues.cpp Analysis/ValueManager.cpp include/clang/Analysis/PathSensitive/GRExprEngine.h include/clang/Analysis/PathSensitive/RValues.h include/clang/Analysis/PathSensitive/ValueManager.h

Ted Kremenek kremenek at apple.com
Thu Feb 28 12:32:03 PST 2008


Author: kremenek
Date: Thu Feb 28 14:32:03 2008
New Revision: 47726

URL: http://llvm.org/viewvc/llvm-project?rev=47726&view=rev
Log:
Added checking for undefined results of '<<' and '>>' (shifting by too many bits, etc.)
This current implementation only works when both operands are concrete values; later we will add support for symbolic values.

Modified:
    cfe/trunk/Analysis/GRExprEngine.cpp
    cfe/trunk/Analysis/GRSimpleVals.cpp
    cfe/trunk/Analysis/RValues.cpp
    cfe/trunk/Analysis/ValueManager.cpp
    cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
    cfe/trunk/include/clang/Analysis/PathSensitive/RValues.h
    cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h

Modified: cfe/trunk/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Analysis/GRExprEngine.cpp?rev=47726&r1=47725&r2=47726&view=diff

==============================================================================
--- cfe/trunk/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/Analysis/GRExprEngine.cpp Thu Feb 28 14:32:03 2008
@@ -1008,14 +1008,11 @@
           bool isFeasible = false;
           ValueState* ZeroSt =  Assume(St, RightV, false, isFeasible);
           
-          if (isFeasible) {
-            NodeTy* DivZeroNode = Builder->generateNode(B, ZeroSt, N2);
-            
-            if (DivZeroNode) {
+          if (isFeasible)
+            if (NodeTy* DivZeroNode = Builder->generateNode(B, ZeroSt, N2)) {
               DivZeroNode->markAsSink();
               BadDivides.insert(DivZeroNode);
             }
-          }
           
           // Second, "assume" that the denominator cannot be 0.
           
@@ -1029,6 +1026,7 @@
         // Fall-through.  The logic below processes the divide.
       }
       
+      
       if (Op <= BinaryOperator::Or) {
         
         // Process non-assignements except commas or short-circuited
@@ -1041,6 +1039,18 @@
           continue;
         }
         
+        if (Result.isUndef() && !LeftV.isUndef() && !RightV.isUndef()) {
+          
+          // The operands were not undefined, but the result is undefined.
+          
+          if (NodeTy* UndefNode = Builder->generateNode(B, St, N2)) {
+            UndefNode->markAsSink();            
+            UndefResults.insert(UndefNode);
+          }
+          
+          continue;
+        }
+        
         Nodify(Dst, B, N2, SetRVal(St, B, Result));
         continue;
       }
@@ -1195,6 +1205,19 @@
           }
 
           RVal Result = EvalCast(EvalBinOp(Op, V, RightV), B->getType());
+          
+          if (Result.isUndef()) {
+            
+            // The operands were not undefined, but the result is undefined.
+            
+            if (NodeTy* UndefNode = Builder->generateNode(B, St, N2)) {
+              UndefNode->markAsSink();            
+              UndefResults.insert(UndefNode);
+            }
+            
+            continue;
+          }
+          
           St = SetRVal(SetRVal(St, B, Result), LeftLV, Result);
         }
       }
@@ -1591,9 +1614,13 @@
         GraphPrintCheckerState->isUndefDeref(N) ||
         GraphPrintCheckerState->isUndefStore(N) ||
         GraphPrintCheckerState->isUndefControlFlow(N) ||
-        GraphPrintCheckerState->isBadDivide(N))
+        GraphPrintCheckerState->isBadDivide(N) ||
+        GraphPrintCheckerState->isUndefResult(N))
       return "color=\"red\",style=\"filled\"";
     
+    if (GraphPrintCheckerState->isNoReturnCall(N))
+      return "color=\"blue\",style=\"filled\"";
+    
     return "";
   }
     
@@ -1635,6 +1662,11 @@
         else if (GraphPrintCheckerState->isBadDivide(N)) {
           Out << "\\|Divide-by zero or undefined value.";
         }
+        else if (GraphPrintCheckerState->isUndefResult(N)) {
+          Out << "\\|Result of operation is undefined.";
+        }
+        else if (GraphPrintCheckerState->isNoReturnCall(N))
+          Out << "\\|Call to function marked \"noreturn\".";
         
         break;
       }

Modified: cfe/trunk/Analysis/GRSimpleVals.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Analysis/GRSimpleVals.cpp?rev=47726&r1=47725&r2=47726&view=diff

==============================================================================
--- cfe/trunk/Analysis/GRSimpleVals.cpp (original)
+++ cfe/trunk/Analysis/GRSimpleVals.cpp Thu Feb 28 14:32:03 2008
@@ -82,6 +82,11 @@
               CheckerState->bad_divides_begin(),
               CheckerState->bad_divides_end(),
               "Division by zero/undefined value.");
+  
+  EmitWarning(Diag, SrcMgr,
+              CheckerState->undef_results_begin(),
+              CheckerState->undef_results_end(),
+              "Result of operation is undefined.");
       
 #ifndef NDEBUG
   if (Visualize) CheckerState->ViewGraph();

Modified: cfe/trunk/Analysis/RValues.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Analysis/RValues.cpp?rev=47726&r1=47725&r2=47726&view=diff

==============================================================================
--- cfe/trunk/Analysis/RValues.cpp (original)
+++ cfe/trunk/Analysis/RValues.cpp Thu Feb 28 14:32:03 2008
@@ -48,11 +48,16 @@
 // Transfer function dispatch for Non-LVals.
 //===----------------------------------------------------------------------===//
 
-nonlval::ConcreteInt
+RVal
 nonlval::ConcreteInt::EvalBinOp(ValueManager& ValMgr, BinaryOperator::Opcode Op,
                                 const nonlval::ConcreteInt& R) const {
   
-  return ValMgr.EvaluateAPSInt(Op, getValue(), R.getValue());
+  const llvm::APSInt* X = ValMgr.EvaluateAPSInt(Op, getValue(), R.getValue());
+  
+  if (X)
+    return nonlval::ConcreteInt(*X);
+  else
+    return UndefinedVal();
 }
 
 
@@ -76,14 +81,19 @@
 // Transfer function dispatch for LVals.
 //===----------------------------------------------------------------------===//
 
-lval::ConcreteInt
+RVal
 lval::ConcreteInt::EvalBinOp(ValueManager& ValMgr, BinaryOperator::Opcode Op,
                              const lval::ConcreteInt& R) const {
   
   assert (Op == BinaryOperator::Add || Op == BinaryOperator::Sub ||
           (Op >= BinaryOperator::LT && Op <= BinaryOperator::NE));
   
-  return ValMgr.EvaluateAPSInt(Op, getValue(), R.getValue());
+  const llvm::APSInt* X = ValMgr.EvaluateAPSInt(Op, getValue(), R.getValue());
+  
+  if (X)
+    return lval::ConcreteInt(*X);
+  else
+    return UndefinedVal();
 }
 
 NonLVal LVal::EQ(ValueManager& ValMgr, const LVal& R) const {

Modified: cfe/trunk/Analysis/ValueManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Analysis/ValueManager.cpp?rev=47726&r1=47725&r2=47726&view=diff

==============================================================================
--- cfe/trunk/Analysis/ValueManager.cpp (original)
+++ cfe/trunk/Analysis/ValueManager.cpp Thu Feb 28 14:32:03 2008
@@ -76,7 +76,7 @@
   return *C;
 }
 
-const llvm::APSInt&
+const llvm::APSInt*
 ValueManager::EvaluateAPSInt(BinaryOperator::Opcode Op,
                              const llvm::APSInt& V1, const llvm::APSInt& V2) {
   
@@ -85,53 +85,83 @@
       assert (false && "Invalid Opcode.");
       
     case BinaryOperator::Mul:
-      return getValue( V1 * V2 );
+      return &getValue( V1 * V2 );
       
     case BinaryOperator::Div:
-      return getValue( V1 / V2 );
+      return &getValue( V1 / V2 );
       
     case BinaryOperator::Rem:
-      return getValue( V1 % V2 );
+      return &getValue( V1 % V2 );
       
     case BinaryOperator::Add:
-      return getValue( V1 + V2 );
+      return &getValue( V1 + V2 );
       
     case BinaryOperator::Sub:
-      return getValue( V1 - V2 );
+      return &getValue( V1 - V2 );
       
-    case BinaryOperator::Shl:
-      return getValue( V1.operator<<( (unsigned) V2.getZExtValue() ));
+    case BinaryOperator::Shl: {
+
+      // FIXME: This logic should probably go higher up, where we can
+      // test these conditions symbolically.
+      
+      // FIXME: Expand these checks to include all undefined behavior.
+      
+      if (V2.isSigned() && V2.isNegative())
+        return NULL;
+      
+      uint64_t Amt = V2.getZExtValue();
+      
+      if (Amt > V1.getBitWidth())
+        return NULL;
+      
+      return &getValue( V1.operator<<( (unsigned) Amt ));
+    }
+      
+    case BinaryOperator::Shr: {
+      
+      // FIXME: This logic should probably go higher up, where we can
+      // test these conditions symbolically.
+      
+      // FIXME: Expand these checks to include all undefined behavior.
+      
+      if (V2.isSigned() && V2.isNegative())
+        return NULL;
+      
+      uint64_t Amt = V2.getZExtValue();
+      
+      if (Amt > V1.getBitWidth())
+        return NULL;
       
-    case BinaryOperator::Shr:
-      return getValue( V1.operator>>( (unsigned) V2.getZExtValue() ));
+      return &getValue( V1.operator>>( (unsigned) Amt ));
+    }
       
     case BinaryOperator::LT:
-      return getTruthValue( V1 < V2 );
+      return &getTruthValue( V1 < V2 );
       
     case BinaryOperator::GT:
-      return getTruthValue( V1 > V2 );
+      return &getTruthValue( V1 > V2 );
       
     case BinaryOperator::LE:
-      return getTruthValue( V1 <= V2 );
+      return &getTruthValue( V1 <= V2 );
       
     case BinaryOperator::GE:
-      return getTruthValue( V1 >= V2 );
+      return &getTruthValue( V1 >= V2 );
       
     case BinaryOperator::EQ:
-      return getTruthValue( V1 == V2 );
+      return &getTruthValue( V1 == V2 );
       
     case BinaryOperator::NE:
-      return getTruthValue( V1 != V2 );
+      return &getTruthValue( V1 != V2 );
       
       // Note: LAnd, LOr, Comma are handled specially by higher-level logic.
       
     case BinaryOperator::And:
-      return getValue( V1 & V2 );
+      return &getValue( V1 & V2 );
       
     case BinaryOperator::Or:
-      return getValue( V1 | V2 );
+      return &getValue( V1 | V2 );
       
     case BinaryOperator::Xor:
-      return getValue( V1 ^ V2 );
+      return &getValue( V1 ^ V2 );
   }
 }

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h?rev=47726&r1=47725&r2=47726&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Thu Feb 28 14:32:03 2008
@@ -92,6 +92,8 @@
   typedef llvm::SmallPtrSet<NodeTy*,2> BadDerefTy;
   typedef llvm::SmallPtrSet<NodeTy*,2> BadDividesTy;
   typedef llvm::SmallPtrSet<NodeTy*,2> NoReturnCallsTy;  
+  typedef llvm::SmallPtrSet<NodeTy*,2> UndefResultsTy;  
+
   
   /// UndefBranches - Nodes in the ExplodedGraph that result from
   ///  taking a branch based on an undefined value.
@@ -121,6 +123,10 @@
   ///  a divide-by-zero or divide-by-undefined.
   BadDividesTy BadDivides;
   
+  /// UndefResults - Nodes in the ExplodedGraph where the operands are defined
+  ///  by the result is not.  Excludes divide-by-zero errors.
+  UndefResultsTy UndefResults;
+  
   bool StateCleaned;
   
 public:
@@ -183,7 +189,11 @@
   bool isNoReturnCall(const NodeTy* N) const {
     return N->isSink() && NoReturnCalls.count(const_cast<NodeTy*>(N)) != 0;
   }
-
+  
+  bool isUndefResult(const NodeTy* N) const {
+    return N->isSink() && UndefResults.count(const_cast<NodeTy*>(N)) != 0;
+  }
+  
   typedef BadDerefTy::iterator null_deref_iterator;
   null_deref_iterator null_derefs_begin() { return ExplicitNullDeref.begin(); }
   null_deref_iterator null_derefs_end() { return ExplicitNullDeref.end(); }
@@ -196,6 +206,10 @@
   bad_divide_iterator bad_divides_begin() { return BadDivides.begin(); }
   bad_divide_iterator bad_divides_end() { return BadDivides.end(); }
   
+  typedef UndefResultsTy::iterator undef_result_iterator;
+  undef_result_iterator undef_results_begin() { return UndefResults.begin(); }
+  undef_result_iterator undef_results_end() { return UndefResults.end(); }
+  
   /// ProcessStmt - Called by GRCoreEngine. Used to generate new successor
   ///  nodes by processing the 'effects' of a block-level statement.
   void ProcessStmt(Stmt* S, StmtNodeBuilder& builder);    

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/RValues.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/RValues.h?rev=47726&r1=47725&r2=47726&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/RValues.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/RValues.h Thu Feb 28 14:32:03 2008
@@ -211,8 +211,8 @@
   }
   
   // Transfer functions for binary/unary operations on ConcreteInts.
-  ConcreteInt EvalBinOp(ValueManager& ValMgr, BinaryOperator::Opcode Op,
-                        const ConcreteInt& R) const;
+  RVal EvalBinOp(ValueManager& ValMgr, BinaryOperator::Opcode Op,
+                 const ConcreteInt& R) const;
   
   ConcreteInt EvalComplement(ValueManager& ValMgr) const;
   
@@ -341,9 +341,8 @@
   }
 
   // Transfer functions for binary/unary operations on ConcreteInts.
-  ConcreteInt EvalBinOp(ValueManager& ValMgr,
-                           BinaryOperator::Opcode Op,
-                           const ConcreteInt& R) const;
+  RVal EvalBinOp(ValueManager& ValMgr, BinaryOperator::Opcode Op,
+                 const ConcreteInt& R) const;
       
   // Implement isa<T> support.
   static inline bool classof(const RVal* V) {

Modified: cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h?rev=47726&r1=47725&r2=47726&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/ValueManager.h Thu Feb 28 14:32:03 2008
@@ -65,7 +65,7 @@
   const SymIntConstraint& getConstraint(SymbolID sym, BinaryOperator::Opcode Op,
                                         const llvm::APSInt& V);
 
-  const llvm::APSInt& EvaluateAPSInt(BinaryOperator::Opcode Op,
+  const llvm::APSInt* EvaluateAPSInt(BinaryOperator::Opcode Op,
                                      const llvm::APSInt& V1,
                                      const llvm::APSInt& V2);
 };





More information about the cfe-commits mailing list