[PATCH] [analyzer][Bugfix/improvement] Fix for PR16833

[Jordan Rose]

================
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:610-614
@@ +609,7 @@
+    const llvm::APSInt &To, const llvm::APSInt &Adjustment) {
+  APSIntType AdjustmentType(Adjustment);
+  llvm::APSInt Lower = AdjustmentType.convert(From) - Adjustment;
+  llvm::APSInt Upper = AdjustmentType.convert(To) - Adjustment;
+  RangeSet New = GetRange(State, Sym).Intersect(getBasicVals(), F, Lower,
+                                                Upper);
+  return New.isEmpty() ? nullptr : State->set<ConstraintRange>(Sym, New);
----------------
Unfortunately this logic isn't correct—assumeSymGE does a lot more than this to account for `From` and `To` being the min or max values of the symbol type, or completely outside the range of the symbol. I'd be more comfortable with factoring out assumeSymGE into getSymGERange, like you did with assumeSymGT. Or just going back to the simple form from before.

================
Comment at: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp:215-220
@@ +214,8 @@
+
+  case nonloc::ConcreteIntKind: {
+    const llvm::APSInt &IntVal = Value.castAs<nonloc::ConcreteInt>().getValue();
+    bool IsInRange = IntVal >= From && IntVal <= To;
+    bool isFeasible = (IsInRange == InRange);
+    return isFeasible ? State : nullptr;
+  }
+  } // end switch
----------------
I //believe// this ignores the case where the bounds have a different type, and APSInt will assert. I don't know if this can happen in the analyzer, but if you don't want to worry about it for now it at least deserves a comment and possibly an explicit assertion.

http://reviews.llvm.org/D5102