[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Hey, it looks like we are finally converging on this one!  Great job!

NOTE: I don't know if you noticed, but I want to point out that there are three failing tests:
F17553262: Screen Shot 2021-06-23 at 11.35.49.png <https://reviews.llvm.org/F17553262>



================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1075-1076
+  ///       LHSOpd binop RHSOpd == Result, where binop is any binary operation
+  bool hasOverflowed(llvm::APSInt LHSOpd, llvm::APSInt RHSOpd,
+                     llvm::APSInt &Result, QualType T) {
+    llvm::APSInt Zero = ValueFactory.getAPSIntType(T).getZeroValue();
----------------
manas wrote:
> We should have these specific functions for other BO as well. Because they will lead us to reason about when `Operand1 binop Operand2` can overflow or not. I was thinking in the direction of having a simpler class which works for this.
I searched through a codebase a bit and here are the couple of functions (from `APInt.h`) that can come in handy:
```
  // Operations that return overflow indicators.
  APInt sadd_ov(const APInt &RHS, bool &Overflow) const;
  APInt uadd_ov(const APInt &RHS, bool &Overflow) const;
  APInt ssub_ov(const APInt &RHS, bool &Overflow) const;
  APInt usub_ov(const APInt &RHS, bool &Overflow) const;
  APInt sdiv_ov(const APInt &RHS, bool &Overflow) const;
  APInt smul_ov(const APInt &RHS, bool &Overflow) const;
  APInt umul_ov(const APInt &RHS, bool &Overflow) const;
  APInt sshl_ov(const APInt &Amt, bool &Overflow) const;
  APInt ushl_ov(const APInt &Amt, bool &Overflow) const;
```
`APSInt` is derived from `APInt`, so we can totally use these.


================
Comment at: clang/test/Analysis/constant-folding.c:269
+  // Checks for inclusive ranges for unsigned integers
+  if (a >= 0 && a <= 10 && b >= 0 && b <= 20) {
+    clang_analyzer_eval((a + b) >= 0); // expected-warning{{TRUE}}
----------------
They are already unsigned, we don't need `a >= 0` and `b >= 0`


================
Comment at: clang/test/Analysis/constant-folding.c:330
+    clang_analyzer_eval((c + d) == INT_MAX - 22); // expected-warning{{FALSE}}
+  }
+}
----------------
I don't see the cases where we overflow on both ends and the case where we overflow on one end, but `Min > Max`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103440/new/

https://reviews.llvm.org/D103440