tomasz-kaminski-sonarsource updated this revision to Diff 426056.
tomasz-kaminski-sonarsource added a comment.
Uploading updated diff with suggested fixes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124658/new/
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,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]+}}<int x>) + 1 }}
+ clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}<int x>) - 9U }} instead of + 4294967287U
+ clang_analyzer_dumpL(l + 0L); // expected-warning-re {{(reg_${{[0-9]+}}<int x>) - 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]+}}<int x>) + 9U }} instead of - 4294967287U
+
+ i = x + (-1);
+ l = i - 10U;
+ clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}<int x>) + 9U }} instead of + 4294967285U
+
+ i = x + 1U;
+ l = i - 10U;
+ clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}<int x>) - 9U }} instead of + 4294967287U
+}
+
+void testMin(int i, long l) {
+ clang_analyzer_dump(i + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) - 1 }} instead of + -1
+ clang_analyzer_dump(i - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) + 1 }} instead of - -1
+ clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) - 1 }} instead of + -1
+ clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) + 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]+}}<int i>) + -2147483648 }}
+ clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) - -2147483648 }}
+ // Produced value has higher bit with (long) so negation if representable
+ clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) - 2147483648 }} instead of + -2147483648
+ clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) + 2147483648 }} instead of - -2147483648
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ 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,
+ // 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 +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;
}
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
