When folding additive operations, convert the values to the same type. When assuming relationships, convert the integers to the same type as the symbol, at least for now.

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

lib/Checker/SimpleConstraintManager.cpp

@@ -173,17 +173,6 @@ const GRState *SimpleConstraintManager::AssumeAux(const GRState *state,
     if (!SE)
       return state;
 
-    GRStateManager &StateMgr = state->getStateManager();
-    ASTContext &Ctx = StateMgr.getContext();
-    BasicValueFactory &BasicVals = StateMgr.getBasicVals();
-
-    // 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.   
-    const SymExpr *LHS = SE->getLHS();
-    QualType LHSType = LHS->getType(Ctx);
-    const llvm::APSInt &RHS = BasicVals.Convert(LHSType, SE->getRHS());
-
     BinaryOperator::Opcode op = SE->getOpcode();
     // FIXME: We should implicitly compare non-comparison expressions to 0.
     if (!BinaryOperator::isComparisonOp(op))
@@ -193,7 +182,7 @@ const GRState *SimpleConstraintManager::AssumeAux(const GRState *state,
     if (!Assumption)
       op = NegateComparison(op);
   
-    return AssumeSymRel(state, LHS, op, RHS);
+    return AssumeSymRel(state, SE->getLHS(), op, SE->getRHS());
   }
 
   case nonloc::ConcreteIntKind: {
@@ -222,11 +211,13 @@ const GRState *SimpleConstraintManager::AssumeSymRel(const GRState *state,
    // x < 4 has the solution [0, 3]. x+2 < 4 has the solution [0-2, 3-2], which
    // in modular arithmetic is [0, 1] U [UINT_MAX-1, UINT_MAX]. It's up to
    // the subclasses of SimpleConstraintManager to handle the adjustment.
-   llvm::APSInt Adjustment(Int.getBitWidth(), Int.isUnsigned());
+   llvm::APSInt Adjustment;
 
   // First check if the LHS is a simple symbol reference.
   SymbolRef Sym = dyn_cast<SymbolData>(LHS);
-  if (!Sym) {
+  if (Sym) {
+    Adjustment = 0;
+  } else {
     // Next, see if it's a "($sym+constant1)" expression.
     const SymIntExpr *SE = dyn_cast<SymIntExpr>(LHS);
 
@@ -256,28 +247,48 @@ const GRState *SimpleConstraintManager::AssumeSymRel(const GRState *state,
     }
   }
 
+  // FIXME: This next section is a hack. It silently converts the integers to
+  // be of the same type as the symbol, which is not always correct. Really the
+  // comparisons should be performed using the Int's type, then mapped back to
+  // the symbol's range of values.
+  GRStateManager &StateMgr = state->getStateManager();
+  ASTContext &Ctx = StateMgr.getContext();
+
+  QualType T = Sym->getType(Ctx);
+  assert(T->isIntegerType() || Loc::IsLocType(T));
+  unsigned bitwidth = Ctx.getTypeSize(T);
+  bool isSymUnsigned = T->isUnsignedIntegerType() || Loc::IsLocType(T);
+
+  // Convert the adjustment.
+  Adjustment.setIsUnsigned(isSymUnsigned);
+  Adjustment.extOrTrunc(bitwidth);
+
+  // Convert the right-hand side integer.
+  llvm::APSInt ConvertedInt(Int, isSymUnsigned);
+  ConvertedInt.extOrTrunc(bitwidth);
+
   switch (op) {
   default:
     // No logic yet for other operators.  Assume the constraint is feasible.
     return state;
 
   case BinaryOperator::EQ:
-    return AssumeSymEQ(state, Sym, Int, Adjustment);
+    return AssumeSymEQ(state, Sym, ConvertedInt, Adjustment);
 
   case BinaryOperator::NE:
-    return AssumeSymNE(state, Sym, Int, Adjustment);
+    return AssumeSymNE(state, Sym, ConvertedInt, Adjustment);
 
   case BinaryOperator::GT:
-    return AssumeSymGT(state, Sym, Int, Adjustment);
+    return AssumeSymGT(state, Sym, ConvertedInt, Adjustment);
 
   case BinaryOperator::GE:
-    return AssumeSymGE(state, Sym, Int, Adjustment);
+    return AssumeSymGE(state, Sym, ConvertedInt, Adjustment);
 
   case BinaryOperator::LT:
-    return AssumeSymLT(state, Sym, Int, Adjustment);
+    return AssumeSymLT(state, Sym, ConvertedInt, Adjustment);
 
   case BinaryOperator::LE:
-    return AssumeSymLE(state, Sym, Int, Adjustment);
+    return AssumeSymLE(state, Sym, ConvertedInt, Adjustment);
   } // end switch
 }
 

lib/Checker/SimpleSValuator.cpp

@@ -411,15 +411,22 @@ SVal SimpleSValuator::EvalBinOpNN(const GRState *state,
         BinaryOperator::Opcode lop = symIntExpr->getOpcode();
         if (BinaryOperator::isAdditiveOp(lop)) {
           BasicValueFactory &BVF = ValMgr.getBasicValueFactory();
+
+          // resultTy may not be the best type to convert to, but it's
+          // probably the best choice in expressions with mixed type
+          // (such as x+1U+2LL). The rules for implicit conversions should
+          // choose a reasonable type to preserve the expression, and will
+          // at least match how the value is going to be used.
+          const llvm::APSInt &first =
+            BVF.Convert(resultTy, symIntExpr->getRHS());
+          const llvm::APSInt &second =
+            BVF.Convert(resultTy, rhsInt->getValue());
+
           const llvm::APSInt *newRHS;
           if (lop == op)
-            newRHS = BVF.EvaluateAPSInt(BinaryOperator::Add,
-                                        symIntExpr->getRHS(),
-                                        rhsInt->getValue());
+            newRHS = BVF.EvaluateAPSInt(BinaryOperator::Add, first, second);
           else
-            newRHS = BVF.EvaluateAPSInt(BinaryOperator::Sub,
-                                        symIntExpr->getRHS(),
-                                        rhsInt->getValue());
+            newRHS = BVF.EvaluateAPSInt(BinaryOperator::Sub, first, second);
           return MakeSymIntVal(symIntExpr->getLHS(), lop, *newRHS, resultTy);
         }
       }

test/Analysis/additive-folding.c

@@ -33,6 +33,22 @@ void oneLongExpression (int a) {
   free(buf);
 }
 
+void mixedTypes (int a) {
+  char* buf = malloc(1);
+
+  // Different additive types should not cause crashes when constant-folding.
+  // This is part of PR7406.
+  int b = a + 1LL;
+  if (a != 0 && (b-1) == 0) // not crash
+    return; // no warning
+
+  int c = a + 1U;
+  if (a != 0 && (c-1) == 0) // not crash
+    return; // no warning
+
+  free(buf);
+}
+
 //---------------
 //  Comparisons
 //---------------
@@ -60,6 +76,30 @@ void ne_eq (unsigned a) {
   free(b);
 }
 
+// Mixed typed inequalities (part of PR7406)
+// These should not crash.
+void mixed_eq_ne (int a) {
+  char* b = NULL;
+  if (a == 1)
+    b = malloc(1);
+  if (a+1U != 2)
+    return; // no-warning
+  if (a-1U != 0)
+    return; // no-warning
+  free(b);
+}
+
+void mixed_ne_eq (int a) {
+  char* b = NULL;
+  if (a != 1)
+    b = malloc(1);
+  if (a+1U == 2)
+    return; // no-warning
+  if (a-1U == 0)
+    return; // no-warning
+  free(b);
+}
+
 
 // Simple order comparisons with no adjustment
 void baselineGT (unsigned a) {