tomasz-kaminski-sonarsource updated this revision to Diff 427693.
tomasz-kaminski-sonarsource added a comment.
I have removed the assertions and updated the code to handle both extensions of
reductions of bitwith from RHS to ResultType.
Expanded test, to cover the above scenarios.
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
Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===================================================================
--- clang/test/Analysis/additive-op-on-sym-int-expr.c
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+// 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);
@@ -42,17 +42,127 @@
clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}<int x>) - 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]+}}<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 }}
+ clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) + -2147483648 }} no change
+ clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) - -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]+}}<long l>) - 2147483648 }} instead of + -2147483648
clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) + 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]+}}<unsigned int u>) - 1U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) + 1U }}
+ return;
+ }
+ if (i == intMin) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) - 2147483648U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) + 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]+}}<long l>) - 1 }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) + 1 }}
+ return;
+ }
+ if (i == intMin) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) - 2147483648 }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<long l>) + 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]+}}<unsigned long ul>) - 1U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned long ul>) + 1U }}
+ return;
+ }
+ if (i == intMin) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned long ul>) - 2147483648U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned long ul>) + 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]+}}<int i>) - 1 }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) + 1 }}\
+ return;
+ }
+ if (l == (long)intMin) { // negation outside of range, no-changes
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) + -2147483648 }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) + -2147483648 }}
+ return;
+ }
+ if (l == ((long)intMin - 1L)) { // outside or range, no changes
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) + 2147483647 }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<int i>) - 2147483647 }}
+ return;
+ }
+ if (l == longMin) { // outside of range, no changes
+ clang_analyzer_dump(c + 0); // expected-warning-re {{reg_${{[0-9]+}}<int i> }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{reg_${{[0-9]+}}<int i> }}
+ return;
+ }
+}
+
+void truncatingToUnsigned(unsigned u, long l) {
+ unsigned c = u + l;
+ unsigned d = u - l;
+ if (l == -1L) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) - 1U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) + 1U }}
+ return;
+ }
+ if (l == (long)intMin) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) - 2147483648U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) + 2147483648U }}
+ return;
+ }
+ if (l == ((long)intMin - 1L)) { // outside or range, no changes
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) - 2147483649U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}<unsigned int u>) + 2147483649U }}
+ return;
+ }
+ if (l == longMin) { // outside of range, no changes
+ clang_analyzer_dump(c + 0); // expected-warning-re {{reg_${{[0-9]+}}<unsigned int u> }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{reg_${{[0-9]+}}<unsigned int u> }}
+ 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
+}
+
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ 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 & val,
+ const APSIntType resultType) {
+ const unsigned valueBits = val.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();
+ } else {
+ // 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.
//===----------------------------------------------------------------------===//
@@ -202,18 +219,11 @@
// 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);
+ 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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
