diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp --- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -197,8 +197,27 @@ if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType()) ConvertedRHS = &BasicVals.Convert(SymbolType, RHS); } - } else + } else if (BinaryOperator::isAdditiveOp(op) && RHS.isNegative()) { + // Change a+(-N) into a-N, and a-(-N) into a+N + // Adjust addition/subtraction of negative value, to + // subtraction/addition of the negated value. + APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy); + assert(resultIntTy.getBitWidth() >= RHS.getBitWidth() && + "The result operation type must have at least the same " + "number of bits as its operands."); + + llvm::APSInt ConvertedRHSValue = resultIntTy.convert(RHS); + // Check if the negation of the RHS is representable: + // * if resultIntTy is unsigned, then negation is always representable + // * if resultIntTy is signed, and RHS is not the lowest representable + // signed value + if (resultIntTy.isUnsigned() || !ConvertedRHSValue.isMinSignedValue()) { + ConvertedRHS = &BasicVals.getValue(-ConvertedRHSValue); + op = (op == BO_Add) ? BO_Sub : BO_Add; + } + } else { ConvertedRHS = &BasicVals.Convert(resultTy, RHS); + } return makeNonLoc(LHS, op, *ConvertedRHS, resultTy); } @@ -636,16 +655,27 @@ const llvm::APSInt &first = IntType.convert(symIntExpr->getRHS()); const llvm::APSInt &second = IntType.convert(*RHSValue); + // If the op and lop agrees, then we just need to + // sum the constants. Otherwise, we change to operation + // type if substraction would produce negative value + // (and cause overflow for unsigned integers), + // as consequence x+1U-10 produces x-9U, instead + // of x+4294967287U, that would be produced without this + // additional check. const llvm::APSInt *newRHS; - if (lop == op) + if (lop == op) { newRHS = BasicVals.evalAPSInt(BO_Add, first, second); - else + } else if (first >= second) { newRHS = BasicVals.evalAPSInt(BO_Sub, first, second); + op = lop; + } else { + newRHS = BasicVals.evalAPSInt(BO_Sub, second, first); + } assert(newRHS && "Invalid operation despite common type!"); rhs = nonloc::ConcreteInt(*newRHS); lhs = nonloc::SymbolVal(symIntExpr->getLHS()); - op = lop; + continue; } } diff --git a/clang/test/Analysis/additive-op-on-sym-int-expr.c b/clang/test/Analysis/additive-op-on-sym-int-expr.c new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/additive-op-on-sym-int-expr.c @@ -0,0 +1,58 @@ +// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s + +void clang_analyzer_dump(int); +void clang_analyzer_dumpL(long); +void clang_analyzer_warnIfReached(); + +void testInspect(int x) { + if ((x < 10) || (x > 100)) { + return; + } + // x: [10, 100] + + int i = x + 1; + long l = i - 10U; + clang_analyzer_dump(i); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} + clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U + clang_analyzer_dumpL(l + 0L); // expected-warning-re {{(reg_${{[0-9]+}}) - 9 }} instead of + 4294967287 + + if ((l - 1000) > 0) { + clang_analyzer_warnIfReached(); // no-warning + } + if (l > 1000) { + clang_analyzer_warnIfReached(); // no-warning + } + if (l > 1000L) { + clang_analyzer_warnIfReached(); // no-warning + } + if ((l + 0L) > 1000) { + clang_analyzer_warnIfReached(); // no-warning + } + + i = x - 1; + l = i + 10U; + clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) + 9U }} instead of - 4294967287U + + i = x + (-1); + l = i - 10U; + clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 11U }} instead of + 4294967285U + + i = x + 1U; + l = i - 10U; + clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U +} + +void testMin(int i, long l) { + clang_analyzer_dump(i + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1 + clang_analyzer_dump(i - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1 + clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1 + clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1 + + int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negative value is not representable + // Do not normalize representation if negation would not be representable + clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} + clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }} + // Produced value has higher bit with (long) so negation if representable + clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648 + clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648 +} diff --git a/clang/test/Analysis/expr-inspection.c b/clang/test/Analysis/expr-inspection.c --- a/clang/test/Analysis/expr-inspection.c +++ b/clang/test/Analysis/expr-inspection.c @@ -11,7 +11,7 @@ void foo(int x) { clang_analyzer_dump(x); // expected-warning{{reg_$0}} - clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}} + clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) - 1}} int y = 1; for (; y < 3; ++y) { clang_analyzer_numTimesReached(); // expected-warning{{2}}