tomasz-kaminski-sonarsource created this revision. tomasz-kaminski-sonarsource added reviewers: vsavchenko, NoQ, steakhal. Herald added a subscriber: martong. Herald added a project: All. tomasz-kaminski-sonarsource requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
This PR changes the SymIntExpr so the expression that uses negative value as RHS, for example: x +/- (-N), are modeled as as x -/+ N. This avoid producing very large RHS in case when the symbol is cased to unsigned number, and as consequence makes the value more robust in presence of cast. This change is not apllied if N is lowest negative value for which negation would not be representable. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D124658 Files: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp clang/test/Analysis/additive-op-on-sym-int-expr.c clang/test/Analysis/expr-inspection.c
Index: clang/test/Analysis/expr-inspection.c =================================================================== --- clang/test/Analysis/expr-inspection.c +++ clang/test/Analysis/expr-inspection.c @@ -11,7 +11,7 @@ void foo(int x) { clang_analyzer_dump(x); // expected-warning{{reg_$0<int x>}} - clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0<int x>) + -1}} + clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0<int x>) - 1}} int y = 1; for (; y < 3; ++y) { clang_analyzer_numTimesReached(); // expected-warning{{2}} Index: clang/test/Analysis/additive-op-on-sym-int-expr.c =================================================================== --- /dev/null +++ clang/test/Analysis/additive-op-on-sym-int-expr.c @@ -0,0 +1,53 @@ +// RUN: %clang_analyze_cc1 -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; + } + + int i = x + 1; + long l = i - 10U; + clang_analyzer_dump(i); // expected-warning {{(reg_$0<int x>) + 1}} + clang_analyzer_dumpL(l); // expected-warning {{(reg_$0<int x>) - 9U}} + clang_analyzer_dumpL(l + 0L); // expected-warning {{(reg_$0<int x>) - 9}} + if ((l - 1000) > 0) { + clang_analyzer_warnIfReached(); + } + if (l > 1000) { + clang_analyzer_warnIfReached(); + } + if (l > 1000L) { + clang_analyzer_warnIfReached(); + } + if ((l + 0L) > 1000) { + clang_analyzer_warnIfReached(); + } + + i = x - 1; + l = i + 10U; + clang_analyzer_dump(i); // expected-warning {{(reg_$0<int x>) - 1}} + clang_analyzer_dumpL(l); // expected-warning {{(reg_$0<int x>) + 9U}} + + l = -1l; + i = l; + clang_analyzer_dump(x + i); // expected-warning {{(reg_$0<int x>) - 1}} +} + +void testMin(int i, long long l) { + clang_analyzer_dump(i + (-1)); // expected-warning {{(reg_$0<int i>) - 1}} + clang_analyzer_dump(i - (-1)); // expected-warning {{(reg_$0<int i>) + 1}} + clang_analyzer_dumpL(l + (-1)); // expected-warning {{(reg_$1<long long l>) - 1}} + clang_analyzer_dumpL(l - (-1)); // expected-warning {{(reg_$1<long long l>) + 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 {{(reg_$0<int i>) + -2147483648}} + clang_analyzer_dump(i - intMin); // expected-warning {{(reg_$0<int i>) - -2147483648}} + // Produced value has higher bit with (long) so negation if representable + clang_analyzer_dumpL(l + intMin); // expected-warning {{(reg_$1<long long l>) - 2147483648}} + clang_analyzer_dumpL(l - intMin); // expected-warning {{(reg_$1<long long l>) + 2147483648}} +} Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -197,8 +197,30 @@ if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType()) ConvertedRHS = &BasicVals.Convert(SymbolType, RHS); } - } else + } else if (BinaryOperator::isAdditiveOp(op)) { + // 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. + if (!RHS.isNegative()) { + ConvertedRHS = &BasicVals.Convert(resultTy, RHS); + } else { + 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, + // i.e., the resultTy is signed, and it is not the lowest + // representable negative value. + if (ConvertedRHSValue > resultIntTy.getMinValue()) { + 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 +658,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; } }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits