From 9ef7ac51af67d08212dc69e5a932c4aa447ee9b7 Mon Sep 17 00:00:00 2001
From: Vince Bridgers <vince.a.bridgers@gmail.com>
Date: Tue, 22 Mar 2022 19:33:19 -0500
Subject: [PATCH] [analyzer] Fix crash in RangedConstraintManager.cpp

This change fixes a crash in RangedConstraintManager.cpp:assumeSym due to an
unhandled BO_Div case.

clang: <root>clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp:51:
  virtual clang::ento::ProgramStateRef
  clang::ento::RangedConstraintManager::assumeSym(clang::ento::ProgramStateRef,
    clang::ento::SymbolRef, bool):
  Assertion `BinaryOperator::isComparisonOp(Op)' failed.

Reviewed By: NoQ

Differential Revision: https://reviews.llvm.org/D122277
---
 .../Core/RangedConstraintManager.cpp               | 71 +++++++++++-----------
 clang/test/Analysis/symbol-simplification-bo-div.c | 14 +++++
 2 files changed, 50 insertions(+), 35 deletions(-)
 create mode 100644 clang/test/Analysis/symbol-simplification-bo-div.c

diff --git a/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
index 892d64e..4bbe933 100644
--- a/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
@@ -48,47 +48,48 @@ ProgramStateRef RangedConstraintManager::assumeSym(ProgramStateRef State,
 
   if (const auto *SSE = dyn_cast<SymSymExpr>(Sym)) {
     BinaryOperator::Opcode Op = SSE->getOpcode();
-    assert(BinaryOperator::isComparisonOp(Op));
-
-    // We convert equality operations for pointers only.
-    if (Loc::isLocType(SSE->getLHS()->getType()) &&
-        Loc::isLocType(SSE->getRHS()->getType())) {
-      // Translate "a != b" to "(b - a) != 0".
-      // We invert the order of the operands as a heuristic for how loop
-      // conditions are usually written ("begin != end") as compared to length
-      // calculations ("end - begin"). The more correct thing to do would be to
-      // canonicalize "a - b" and "b - a", which would allow us to treat
-      // "a != b" and "b != a" the same.
-
-      SymbolManager &SymMgr = getSymbolManager();
-      QualType DiffTy = SymMgr.getContext().getPointerDiffType();
-      SymbolRef Subtraction =
-          SymMgr.getSymSymExpr(SSE->getRHS(), BO_Sub, SSE->getLHS(), DiffTy);
-
-      const llvm::APSInt &Zero = getBasicVals().getValue(0, DiffTy);
-      Op = BinaryOperator::reverseComparisonOp(Op);
-      if (!Assumption)
-        Op = BinaryOperator::negateComparisonOp(Op);
-      return assumeSymRel(State, Subtraction, Op, Zero);
-    }
+    if (BinaryOperator::isComparisonOp(Op)) {
+
+      // We convert equality operations for pointers only.
+      if (Loc::isLocType(SSE->getLHS()->getType()) &&
+          Loc::isLocType(SSE->getRHS()->getType())) {
+        // Translate "a != b" to "(b - a) != 0".
+        // We invert the order of the operands as a heuristic for how loop
+        // conditions are usually written ("begin != end") as compared to length
+        // calculations ("end - begin"). The more correct thing to do would be
+        // to canonicalize "a - b" and "b - a", which would allow us to treat
+        // "a != b" and "b != a" the same.
+
+        SymbolManager &SymMgr = getSymbolManager();
+        QualType DiffTy = SymMgr.getContext().getPointerDiffType();
+        SymbolRef Subtraction =
+            SymMgr.getSymSymExpr(SSE->getRHS(), BO_Sub, SSE->getLHS(), DiffTy);
+
+        const llvm::APSInt &Zero = getBasicVals().getValue(0, DiffTy);
+        Op = BinaryOperator::reverseComparisonOp(Op);
+        if (!Assumption)
+          Op = BinaryOperator::negateComparisonOp(Op);
+        return assumeSymRel(State, Subtraction, Op, Zero);
+      }
 
-    if (BinaryOperator::isEqualityOp(Op)) {
-      SymbolManager &SymMgr = getSymbolManager();
+      if (BinaryOperator::isEqualityOp(Op)) {
+        SymbolManager &SymMgr = getSymbolManager();
 
-      QualType ExprType = SSE->getType();
-      SymbolRef CanonicalEquality =
-          SymMgr.getSymSymExpr(SSE->getLHS(), BO_EQ, SSE->getRHS(), ExprType);
+        QualType ExprType = SSE->getType();
+        SymbolRef CanonicalEquality =
+            SymMgr.getSymSymExpr(SSE->getLHS(), BO_EQ, SSE->getRHS(), ExprType);
 
-      bool WasEqual = SSE->getOpcode() == BO_EQ;
-      bool IsExpectedEqual = WasEqual == Assumption;
+        bool WasEqual = SSE->getOpcode() == BO_EQ;
+        bool IsExpectedEqual = WasEqual == Assumption;
 
-      const llvm::APSInt &Zero = getBasicVals().getValue(0, ExprType);
+        const llvm::APSInt &Zero = getBasicVals().getValue(0, ExprType);
 
-      if (IsExpectedEqual) {
-        return assumeSymNE(State, CanonicalEquality, Zero, Zero);
-      }
+        if (IsExpectedEqual) {
+          return assumeSymNE(State, CanonicalEquality, Zero, Zero);
+        }
 
-      return assumeSymEQ(State, CanonicalEquality, Zero, Zero);
+        return assumeSymEQ(State, CanonicalEquality, Zero, Zero);
+      }
     }
   }
 
diff --git a/clang/test/Analysis/symbol-simplification-bo-div.c b/clang/test/Analysis/symbol-simplification-bo-div.c
new file mode 100644
index 0000000..122ad40
--- /dev/null
+++ b/clang/test/Analysis/symbol-simplification-bo-div.c
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core %s \
+// RUN:    -triple x86_64-pc-linux-gnu -verify
+
+// don't crash
+// expected-no-diagnostics
+
+int a, b;
+int c(void) {
+  unsigned d = a;
+  --d;
+  short e = b / b - a;
+  ++e;
+  return d <= 0 && e && e;
+}
-- 
2.7.4