From: Jordy Rose Date: Mon, 21 Jun 2010 20:15:15 +0000 (+0000) Subject: When folding additive operations, convert the values to the same type. When assuming... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b4954a4175b36d912bdfc43834d09754faddd855;p=clang 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 --- diff --git a/lib/Checker/SimpleConstraintManager.cpp b/lib/Checker/SimpleConstraintManager.cpp index 3d6930b355..a1594a9e9e 100644 --- a/lib/Checker/SimpleConstraintManager.cpp +++ b/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(LHS); - if (!Sym) { + if (Sym) { + Adjustment = 0; + } else { // Next, see if it's a "($sym+constant1)" expression. const SymIntExpr *SE = dyn_cast(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 } diff --git a/lib/Checker/SimpleSValuator.cpp b/lib/Checker/SimpleSValuator.cpp index 0799380e80..bf539defd4 100644 --- a/lib/Checker/SimpleSValuator.cpp +++ b/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); } } diff --git a/test/Analysis/additive-folding.c b/test/Analysis/additive-folding.c index 14fd42a2bf..dd3308713a 100644 --- a/test/Analysis/additive-folding.c +++ b/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) {