Added checking for undefined results of '<<' and '>>' (shifting by too many bits...

This current implementation only works when both operands are concrete values; later we will add support for symbolic values.

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

diff --git a/Analysis/GRExprEngine.cpp b/Analysis/GRExprEngine.cpp
index 6106ef39cb228f6ec5f96a888a31269748beeb8b..74f3a9a48d7955f216f2def3c30ae0958d88764b 100644 (file)
--- a/Analysis/GRExprEngine.cpp
+++ b/Analysis/GRExprEngine.cpp
@@ -1008,14 +1008,11 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B,
           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 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B,
         // Fall-through.  The logic below processes the divide.
       }
       
+      
       if (Op <= BinaryOperator::Or) {
         
         // Process non-assignements except commas or short-circuited
@@ -1041,6 +1039,18 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B,
           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 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B,
           }
 
           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 @@ struct VISIBILITY_HIDDEN DOTGraphTraits<GRExprEngine::NodeTy*> :
         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 @@ struct VISIBILITY_HIDDEN DOTGraphTraits<GRExprEngine::NodeTy*> :
         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;
       }

diff --git a/Analysis/GRSimpleVals.cpp b/Analysis/GRSimpleVals.cpp
index 4ed80afe1026f773840b782f9cdc145483c19b2d..8b5e3c85a97fd6d5efec3a8b9d24d212cb627b4c 100644 (file)
--- a/Analysis/GRSimpleVals.cpp
+++ b/Analysis/GRSimpleVals.cpp
@@ -82,6 +82,11 @@ unsigned RunGRSimpleVals(CFG& cfg, FunctionDecl& FD, ASTContext& Ctx,
               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();

diff --git a/Analysis/RValues.cpp b/Analysis/RValues.cpp
index d7c4dac252b89db0ee2914bcd578e9873c1212b8..6369da4d7ffb1e2a7c2b491b71938915ad571a94 100644 (file)
--- a/Analysis/RValues.cpp
+++ b/Analysis/RValues.cpp
@@ -48,11 +48,16 @@ RVal::symbol_iterator RVal::symbol_end() const {
 // 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 @@ nonlval::ConcreteInt::EvalMinus(ValueManager& ValMgr, UnaryOperator* U) const {
 // 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 {

diff --git a/Analysis/ValueManager.cpp b/Analysis/ValueManager.cpp
index 64f4b27d3fe3d9685e1deea4df2bc2003b39f58d..2a8d23d02ca21d9c8ad5bc1446e1c22ac6abecfa 100644 (file)
--- a/Analysis/ValueManager.cpp
+++ b/Analysis/ValueManager.cpp
@@ -76,7 +76,7 @@ ValueManager::getConstraint(SymbolID sym, BinaryOperator::Opcode Op,
   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 @@ ValueManager::EvaluateAPSInt(BinaryOperator::Opcode Op,
       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 );
   }
 }

diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index f4984e1e5d51794f7b1f372adc1011bca6da1761..efccf02f6e80527c1712172217a9e0c706ddacff 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -92,6 +92,8 @@ protected:
   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 @@ protected:
   ///  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 @@ public:
   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 @@ public:
   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);    

diff --git a/include/clang/Analysis/PathSensitive/RValues.h b/include/clang/Analysis/PathSensitive/RValues.h
index ca94e4ddbaf07007cbd5586ab083b807c0522e9f..e2d07c0856e36b30f0878102d4cbd92d25f6914d 100644 (file)
--- a/include/clang/Analysis/PathSensitive/RValues.h
+++ b/include/clang/Analysis/PathSensitive/RValues.h
@@ -211,8 +211,8 @@ public:
   }
   
   // 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 @@ public:
   }
 
   // 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) {

diff --git a/include/clang/Analysis/PathSensitive/ValueManager.h b/include/clang/Analysis/PathSensitive/ValueManager.h
index 29d392cda65020a5226b6fdc0914c09629a4e46f..0b463fba3b777a72af0f4e62d7436d63dc6b3acb 100644 (file)
--- a/include/clang/Analysis/PathSensitive/ValueManager.h
+++ b/include/clang/Analysis/PathSensitive/ValueManager.h
@@ -65,7 +65,7 @@ public:
   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);
 };