From: Ted Kremenek <kremenek@apple.com>
Date: Fri, 25 Sep 2009 00:18:15 +0000 (+0000)
Subject: Fix <rdar://problem/7249327> by allowing silent conversions between signed and unsign... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=80417471b01ab2726cd04773b2ab700ce564073c;p=clang

Fix <rdar://problem/7249327> by allowing silent conversions between signed and unsigned integer values for symbolic values.  This is an intermediate solution (i.e. hack) until we support extension/truncation of symbolic integers.


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

diff --git a/include/clang/Analysis/PathSensitive/BasicValueFactory.h b/include/clang/Analysis/PathSensitive/BasicValueFactory.h
index 69cd9db77c..12f0ce2d50 100644
--- a/include/clang/Analysis/PathSensitive/BasicValueFactory.h
+++ b/include/clang/Analysis/PathSensitive/BasicValueFactory.h
@@ -92,16 +92,25 @@ public:
 
   /// Convert - Create a new persistent APSInt with the same value as 'From'
   ///  but with the bitwidth and signedness of 'To'.
-  const llvm::APSInt& Convert(const llvm::APSInt& To,
+  const llvm::APSInt &Convert(const llvm::APSInt& To,
                               const llvm::APSInt& From) {
 
     if (To.isUnsigned() == From.isUnsigned() &&
         To.getBitWidth() == From.getBitWidth())
       return From;
 
-    return getValue(From.getSExtValue(),
-                    To.getBitWidth(),
-                    To.isUnsigned());
+    return getValue(From.getSExtValue(), To.getBitWidth(), To.isUnsigned());
+  }
+  
+  const llvm::APSInt &Convert(QualType T, const llvm::APSInt &From) {
+    assert(T->isIntegerType() || Loc::IsLocType(T));
+    unsigned bitwidth = Ctx.getTypeSize(T);
+    bool isUnsigned = T->isUnsignedIntegerType() || Loc::IsLocType(T);
+    
+    if (isUnsigned == From.isUnsigned() && bitwidth == From.getBitWidth())
+      return From;
+    
+    return getValue(From.getSExtValue(), bitwidth, isUnsigned);
   }
 
   const llvm::APSInt& getIntValue(uint64_t X, bool isUnsigned) {
diff --git a/lib/Analysis/SimpleConstraintManager.cpp b/lib/Analysis/SimpleConstraintManager.cpp
index e4ad85aa83..015db76080 100644
--- a/lib/Analysis/SimpleConstraintManager.cpp
+++ b/lib/Analysis/SimpleConstraintManager.cpp
@@ -162,8 +162,19 @@ const GRState *SimpleConstraintManager::AssumeAux(const GRState *state,
 
   case nonloc::SymExprValKind: {
     nonloc::SymExprVal V = cast<nonloc::SymExprVal>(Cond);
-    if (const SymIntExpr *SE = dyn_cast<SymIntExpr>(V.getSymbolicExpression()))
-      return AssumeSymInt(state, Assumption, SE);
+    if (const SymIntExpr *SE = dyn_cast<SymIntExpr>(V.getSymbolicExpression())){
+      // FIXME: This is a hack.  It silently converts the RHS integer to be
+      // of the same type as on the left side.  This should be removed once
+      // we support truncation/extension of symbolic values.      
+      GRStateManager &StateMgr = state->getStateManager();
+      ASTContext &Ctx = StateMgr.getContext();
+      QualType LHSType = SE->getLHS()->getType(Ctx);
+      BasicValueFactory &BasicVals = StateMgr.getBasicVals();
+      const llvm::APSInt &RHS = BasicVals.Convert(LHSType, SE->getRHS());
+      SymIntExpr SENew(SE->getLHS(), SE->getOpcode(), RHS, SE->getType(Ctx));
+
+      return AssumeSymInt(state, Assumption, &SENew);
+    }
 
     // For all other symbolic expressions, over-approximate and consider
     // the constraint feasible.
diff --git a/lib/Analysis/SimpleSValuator.cpp b/lib/Analysis/SimpleSValuator.cpp
index 6d944da45b..52a591927d 100644
--- a/lib/Analysis/SimpleSValuator.cpp
+++ b/lib/Analysis/SimpleSValuator.cpp
@@ -68,6 +68,15 @@ SVal SimpleSValuator::EvalCastNL(NonLoc val, QualType castTy) {
     QualType T = Ctx.getCanonicalType(se->getType(Ctx));
     if (T == Ctx.getCanonicalType(castTy))
       return val;
+    
+    // FIXME: Remove this hack when we support symbolic truncation/extension.
+    // HACK: If both castTy and T are integers, ignore the cast.  This is
+    // not a permanent solution.  Eventually we want to precisely handle
+    // extension/truncation of symbolic integers.  This prevents us from losing
+    // precision when we assign 'x = y' and 'y' is symbolic and x and y are
+    // different integer types.
+    if (T->isIntegerType() && castTy->isIntegerType())
+      return val;
 
     return UnknownVal();
   }
diff --git a/test/Analysis/misc-ps-region-store.m b/test/Analysis/misc-ps-region-store.m
index c5be02c821..dd1e6455f0 100644
--- a/test/Analysis/misc-ps-region-store.m
+++ b/test/Analysis/misc-ps-region-store.m
@@ -207,3 +207,33 @@ void rdar_7249340(ptr_rdar_7249340 x) {
   *p = 0xDEADBEEF; // no-warning
 }
 
+// <rdar://problem/7249327> - This test case tests both value tracking of
+// array values and that we handle symbolic values that are casted
+// between different integer types.  Note the assignment 'n = *a++'; here
+// 'n' is and 'int' and '*a' is 'unsigned'.  Previously we got a false positive
+// at 'x += *b++' (undefined value) because we got a false path.
+int rdar_7249327_aux(void);
+
+void rdar_7249327(unsigned int A[2*32]) {
+  int B[2*32];
+  int *b;
+  unsigned int *a;
+  int x = 0;
+  
+  int n;
+  
+  a = A;
+  b = B;
+  
+  n = *a++;
+  if (n)
+    *b++ = rdar_7249327_aux();
+
+  a = A;
+  b = B;
+  
+  n = *a++;
+  if (n)
+    x += *b++; // no-warning
+}
+