From 72afb3739da0da02158242ae41a50cfe0bea78b4 Mon Sep 17 00:00:00 2001
From: Ted Kremenek <kremenek@apple.com>
Date: Sat, 17 Jan 2009 01:54:16 +0000
Subject: [PATCH] Fix analyzer crash found when scanning Wine sources where the
 analyzer used old logic to determine the value of a switch 'case' label.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@62395 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Analysis/GRExprEngine.cpp | 60 +++++++++++++++--------------------
 test/Analysis/null-deref-ps.c | 23 ++++++++++++++
 2 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 73df09a6f4..8656ef4a90 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -698,10 +698,8 @@ void GRExprEngine::VisitGuardedExpr(Expr* Ex, Expr* L, Expr* R,
 
 /// ProcessSwitch - Called by GRCoreEngine.  Used to generate successor
 ///  nodes by processing the 'effects' of a switch statement.
-void GRExprEngine::ProcessSwitch(SwitchNodeBuilder& builder) {
-  
-  typedef SwitchNodeBuilder::iterator iterator;
-  
+void GRExprEngine::ProcessSwitch(SwitchNodeBuilder& builder) {  
+  typedef SwitchNodeBuilder::iterator iterator;  
   const GRState* St = builder.getState();  
   Expr* CondE = builder.getCondition();
   SVal  CondV = GetSVal(St, CondE);
@@ -711,38 +709,32 @@ void GRExprEngine::ProcessSwitch(SwitchNodeBuilder& builder) {
     UndefBranches.insert(N);
     return;
   }
-  
-  const GRState*  DefaultSt = St;
-  
-  // While most of this can be assumed (such as the signedness), having it
-  // just computed makes sure everything makes the same assumptions end-to-end.
-  
-  unsigned bits = getContext().getTypeSize(CondE->getType());
 
-  APSInt V1(bits, false);
-  APSInt V2 = V1;
+  const GRState*  DefaultSt = St;  
   bool DefaultFeasible = false;
   
   for (iterator I = builder.begin(), EI = builder.end(); I != EI; ++I) {
-
     CaseStmt* Case = cast<CaseStmt>(I.getCase());
-    
-    // Evaluate the case.
-    if (!Case->getLHS()->isIntegerConstantExpr(V1, getContext(), 0, true)) {
-      assert (false && "Case condition must evaluate to an integer constant.");
-      return;
-    }
-    
+
+    // Evaluate the LHS of the case value.
+    Expr::EvalResult V1;
+    bool b = Case->getLHS()->Evaluate(V1, getContext());    
+    
+    // Sanity checks.  These go away in Release builds.
+    assert(b && V1.Val.isInt() && !V1.HasSideEffects 
+             && "Case condition must evaluate to an integer constant.");
+    b = b; // silence unused variable warning    
+    assert(V1.Val.getInt().getBitWidth() == 
+           getContext().getTypeSize(CondE->getType()));
+           
     // Get the RHS of the case, if it exists.
+    Expr::EvalResult V2;
     
     if (Expr* E = Case->getRHS()) {
-      if (!E->isIntegerConstantExpr(V2, getContext(), 0, true)) {
-        assert (false &&
-                "Case condition (RHS) must evaluate to an integer constant.");
-        return ;
-      }
-      
-      assert (V1 <= V2);
+      b = E->Evaluate(V2, getContext());
+      assert(b && V2.Val.isInt() && !V2.HasSideEffects 
+             && "Case condition must evaluate to an integer constant.");
+      b = b; // silence unused variable warning
     }
     else
       V2 = V1;
@@ -752,12 +744,10 @@ void GRExprEngine::ProcessSwitch(SwitchNodeBuilder& builder) {
     //  This should be easy once we have "ranges" for NonLVals.
         
     do {
-      nonloc::ConcreteInt CaseVal(getBasicVals().getValue(V1));
-      
+      nonloc::ConcreteInt CaseVal(getBasicVals().getValue(V1.Val.getInt()));      
       SVal Res = EvalBinOp(BinaryOperator::EQ, CondV, CaseVal);
       
-      // Now "assume" that the case matches.
-      
+      // Now "assume" that the case matches.      
       bool isFeasible = false;      
       const GRState* StNew = Assume(St, Res, true, isFeasible);
       
@@ -783,11 +773,11 @@ void GRExprEngine::ProcessSwitch(SwitchNodeBuilder& builder) {
       }
 
       // Concretize the next value in the range.
-      if (V1 == V2)
+      if (V1.Val.getInt() == V2.Val.getInt())
         break;
       
-      ++V1;
-      assert (V1 <= V2);
+      ++V1.Val.getInt();
+      assert (V1.Val.getInt() <= V2.Val.getInt());
       
     } while (true);
   }
diff --git a/test/Analysis/null-deref-ps.c b/test/Analysis/null-deref-ps.c
index e0dddc8a59..5092e82e41 100644
--- a/test/Analysis/null-deref-ps.c
+++ b/test/Analysis/null-deref-ps.c
@@ -188,3 +188,26 @@ void f11b(unsigned i) {
   }
 }
 
+// Test case for switch statements with weird case arms.
+typedef int     BOOL, *PBOOL, *LPBOOL;
+typedef long    LONG_PTR, *PLONG_PTR;
+typedef unsigned long ULONG_PTR, *PULONG_PTR;
+typedef ULONG_PTR DWORD_PTR, *PDWORD_PTR;
+typedef LONG_PTR LRESULT;
+typedef struct _F12ITEM *HF12ITEM;
+
+void f12(HF12ITEM i, char *q) {
+  char *p = 0;
+  switch ((DWORD_PTR) i) {
+  case 0 ... 10:
+    p = q;
+    break;
+  case (DWORD_PTR) ((HF12ITEM) - 65535):
+    return;
+  default:
+    return;
+  }
+  
+  *p = 1; // no-warning
+}
+
-- 
2.40.0