[clang] 9ef7ac5 - [analyzer] Fix crash in RangedConstraintManager.cpp

[via cfe-commits]

Author: Vince Bridgers
Date: 2022-03-23T08:26:40-05:00
New Revision: 9ef7ac51af67d08212dc69e5a932c4aa447ee9b7

URL: https://github.com/llvm/llvm-project/commit/9ef7ac51af67d08212dc69e5a932c4aa447ee9b7
DIFF: https://github.com/llvm/llvm-project/commit/9ef7ac51af67d08212dc69e5a932c4aa447ee9b7.diff

LOG: [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

Added: 
    clang/test/Analysis/symbol-simplification-bo-div.c

Modified: 
    clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
index 892d64ea4e4e2..4bbe933be2129 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 0000000000000..122ad4083b1df
--- /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;
+}