[analyzer] A fix for substraction of an integer from a pointer.

[NoQ]

Summary

In Clang Static Analyzer, there's a typo in evalBinOpLN() which leads to incorrect modeling of pointer arithmetic: when substracting an integral value from a pointer value (assuming the latter is a SubRegion that is not an ElementRegion), the index value of the newly produced ElementRegion has incorrect sign.

The patch fixes the problem and adds relevant regression tests.

[zaks.anna]

Let's make the test more explicit about what is being tested; for example,
void negativeIndex(char *str) {

char *ptr = str + 1;
*ptr = 'a';
clang_analyzer_eval(*ptr == 'a'); // expected-warning{{TRUE}}
ptr = str - 1;
clang_analyzer_eval(*ptr == 'a'); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(*(str + 1) == 'a'); // expected-warning{{TRUE}}
ptr = str;
ptr -= 1;
clang_analyzer_eval(*ptr == 'a'); // expected-warning{{UNKNOWN}}
ptr = str;
--ptr;
clang_analyzer_eval(*ptr == 'a'); // expected-warning{{UNKNOWN}}

}

Otherwise, LGTM

[NoQ]

Make the tests easier to understand.

[NoQ]

Thanks, fixed :)

[xazax.hun]

In D12725#243150, @NoQ wrote:

Thanks, fixed :)

Can you commit this or do you need someone to commit this for you?

[NoQ]

I've got no commit access yet, sorry, that's my first patch here actually :)

[xazax.hun]

In D12725#248931, @NoQ wrote:

I've got no commit access yet, sorry, that's my first patch here actually :)

No problem, I committed it in r248021. Thank you for your contribution!

cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

if (const MemRegion *region = lhs.getAsRegion()) {
     if (const ElementRegion *elemReg = dyn_cast<ElementRegion>(region)) {
       assert(op == BO_Add || op == BO_Sub);
       index = evalBinOpNN(state, op, elemReg->getIndex(), rhs,
                           getArrayIndexType());
       superR = elemReg->getSuperRegion();
       elementType = elemReg->getElementType();
     }
     else if (isa<SubRegion>(region)) {
+      assert(op == BO_Add || op == BO_Sub);
+      index = (op == BO_Add) ? rhs : evalMinus(rhs);
       superR = region;
-      index = rhs;
       if (resultTy->isAnyPointerType())
         elementType = resultTy->getPointeeType();
     }
 
     if (Optional<NonLoc> indexV = index.getAs<NonLoc>()) {
       return loc::MemRegionVal(MemMgr.getElementRegion(elementType, *indexV,
                                                        superR, getContext()));
     }

cfe/trunk/test/Analysis/ptr-arith.c

struct Point {
   int y;
 };
 void symbolicFieldRegion(struct Point *points, int i, int j) {
   clang_analyzer_eval(&points[i].x == &points[j].x);// expected-warning{{UNKNOWN}}
   clang_analyzer_eval(&points[i].x == &points[i].y);// expected-warning{{FALSE}}
   clang_analyzer_eval(&points[i].x < &points[i].y);// expected-warning{{TRUE}}
 }
 
+void negativeIndex(char *str) {
+  *(str + 1) = 'a';
+  clang_analyzer_eval(*(str + 1) == 'a'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(*(str - 1) == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr1 = str - 1;
+  clang_analyzer_eval(*ptr1 == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr2 = str;
+  ptr2 -= 1;
+  clang_analyzer_eval(*ptr2 == 'a'); // expected-warning{{UNKNOWN}}
+
+  char *ptr3 = str;
+  --ptr3;
+  clang_analyzer_eval(*ptr3 == 'a'); // expected-warning{{UNKNOWN}}
+}
+