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 @@ -104,6 +104,23 @@ } } +// Checks if the negation the value and flipping sign preserve +// the semantics on the operation in the resultType +static bool isNegationValuePreserving(const llvm::APSInt &Value, + APSIntType ResultType) { + const unsigned ValueBits = Value.getSignificantBits(); + if (ValueBits == ResultType.getBitWidth()) { + // The value is the lowest negative value that is representable + // in signed integer with bitWith of result type. The + // negation is representable if resultType is unsigned. + return ResultType.isUnsigned(); + } + + // If resultType bitWith is higher that number of bits required + // to represent RHS, the sign flip produce same value. + return ValueBits < ResultType.getBitWidth(); +} + //===----------------------------------------------------------------------===// // Transfer function for binary operators. //===----------------------------------------------------------------------===// @@ -197,6 +214,17 @@ if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType()) ConvertedRHS = &BasicVals.Convert(SymbolType, RHS); } + } 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); + if (isNegationValuePreserving(RHS, resultIntTy)) { + ConvertedRHS = &BasicVals.getValue(-resultIntTy.convert(RHS)); + op = (op == BO_Add) ? BO_Sub : BO_Add; + } else { + ConvertedRHS = &BasicVals.Convert(resultTy, RHS); + } } else ConvertedRHS = &BasicVals.Convert(resultTy, RHS); @@ -636,16 +664,26 @@ 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,169 @@ +// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux-gnu -analyzer-checker=core,apiModeling,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 +} + +const int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negation value is not representable +const long longMin = 1L << (sizeof(long) * 8 - 1); // LONG_MIN, negation value is not representable + +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 + + // Do not normalize representation if negation would not be representable + clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} no change + clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }} no change + // 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 +} + +void changingToUnsinged(unsigned u, int i) { + unsigned c = u + (unsigned)i; + unsigned d = u - (unsigned)i; + if (i == -1) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }} + return; + } + if (i == intMin) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }} + return; + } +} + +void extendingToSigned(long l, int i) { + long c = l + (long)i; + long d = l - (long)i; + if (i == -1) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} + return; + } + if (i == intMin) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} + return; + } +} + +void extendingToUnigned(unsigned long ul, int i) { + unsigned long c = ul + (unsigned long)i; + unsigned long d = ul - (unsigned long)i; + if (i == -1) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }} + return; + } + if (i == intMin) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }} + return; + } +} + +void truncatingToSigned(int i, long l) { + int c = i + (int)l; + int d = i - (int)l; + if (l == -1L) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} + return; + } + if (l == (long)intMin) { // negation outside of range, no-changes + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} + return; + } + if (l == ((long)intMin - 1L)) { // outside or range, no changes + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483647 }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483647 }} + return; + } + if (l == longMin) { // outside of range, no changes + clang_analyzer_dump(c + 0); // expected-warning-re {{reg_${{[0-9]+}} }} + clang_analyzer_dump(d + 0); // expected-warning-re {{reg_${{[0-9]+}} }} + return; + } +} + +void truncatingToUnsigned(unsigned u, long l) { + unsigned c = u + (unsigned)l; + unsigned d = u - (unsigned)l; + if (l == -1L) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }} + return; + } + if (l == (long)intMin) { + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }} + return; + } + if (l == ((long)intMin - 1L)) { // outside or range, no changes + clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483647U }} + clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483647U }} + return; + } + if (l == longMin) { // outside of range, no changes + clang_analyzer_dump(c + 0); // expected-warning-re {{reg_${{[0-9]+}} }} + clang_analyzer_dump(d + 0); // expected-warning-re {{reg_${{[0-9]+}} }} + return; + } +} + +// Test for crashes +typedef long ssize_t; +ssize_t write(int, const void *, unsigned long); + +int crashTest(int x, int fd) { + unsigned wres = write(fd, "a", 1); + if (wres) { + } + int t1 = x - wres; + if (wres < 0) { + } + return x + t1; // no crash +} 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}}