================
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



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to