[PATCH] D16178: [analyzer] A quick fix on top of D12901/r257464

[Artem Dergachev via cfe-commits]

NoQ created this revision.
NoQ added reviewers: pgousseau, zaks.anna, dcoughlin, xazax.hun.
NoQ added a subscriber: cfe-commits.

Sorry for being a bit slow, i should have had a look at the review earlier; i noticed some stuff after the recent patch by Pierre Gousseau was committed.

1. There's an off-by-one in the comparison `evalBinOp` in `evalIntegralCast`, the patch attached to this revision fixes it, and tests it via bool-assignment checker.

2. There's also a FIXME test for the same checker, which is a regression after this patch (used to throw a warning, now it doesn't); seems not as bad as the crash, so it can probably be addressed later.

3. I noticed that it's still possible to trigger the crash with the following test, not sure what to do with it:
```
void f1(long foo)
{
  unsigned index = -1;
  if (index < foo - 1) index = foo;
  if (index + 1 == 0) {}
}
```

So my best idea is to push this quick fix for off-by-one and then think where to go further. Probably we need to teach `RangeConstraintManager` to work with `SymbolCast`'s correctly (eg. understand how their range is related to a parent symbol's range). Eventually, once it gets better, we'd probably want to change the following in `evalIntegralCast`:
```
   std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
-  if (!IsNotTruncated && IsTruncated) {
+  if (IsTruncated) {
```
but right now it breaks tests (not only bool-assignment, but also other checkers that start failing can probably be shown to loose warnings after this patch, but i didn't have a look at this yet, so not sure, just hand-waving).

Anyway, generally, overally, i think r257464 is a step in the very right direction if we want to work with casts better, and i was actually very happy to see it went that way :)

http://reviews.llvm.org/D16178

Files:
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  test/Analysis/bool-assignment.c

Index: test/Analysis/bool-assignment.c
===================================================================
--- test/Analysis/bool-assignment.c
+++ test/Analysis/bool-assignment.c
@@ -42,6 +42,15 @@
     BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}
     return;
   }
+  if (y > 200 && y < 250) {
+    // FIXME: Currently we are loosing this warning due to a SymbolCast in RHS.
+    BOOL x = y; // no-warning
+    return;
+  }
+  if (y >= 127 && y < 150) {
+    BOOL x = y; // expected-warning{{Assignment of a non-Boolean value}}
+    return;
+  }
   if (y > 1) {
     BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}
     return;
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===================================================================
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -451,7 +451,7 @@
   NonLoc FromVal = val.castAs<NonLoc>();
   QualType CmpTy = getConditionType();
   NonLoc CompVal =
-      evalBinOpNN(state, BO_LT, FromVal, ToTypeMaxVal, CmpTy).castAs<NonLoc>();
+      evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs<NonLoc>();
   ProgramStateRef IsNotTruncated, IsTruncated;
   std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
   if (!IsNotTruncated && IsTruncated) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16178.44842.patch
Type: text/x-patch
Size: 1319 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160114/95d3f9ed/attachment.bin>