From 12887a202404471ddf77f9fae658700573cbebe8 Mon Sep 17 00:00:00 2001
From: Gabor Marton <gabor.marton@ericsson.com>
Date: Thu, 11 Nov 2021 14:55:24 +0100
Subject: [PATCH] [Analyzer][Core] Better simplification in
 SimpleSValBuilder::evalBinOpNN

Make the SValBuilder capable to simplify existing
SVals based on a newly added constraints when evaluating a BinOp.

Before this patch, we called `simplify` only in some edge cases.
However, we can and should investigate the constraints in all cases.

Differential Revision: https://reviews.llvm.org/D113753
---
 .../lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp  | 19 +++++++-------
 .../Analysis/svalbuilder-simplify-in-evalbinop.cpp | 30 ++++++++++++++++++++++
 2 files changed, 39 insertions(+), 10 deletions(-)
 create mode 100644 clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp

diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 54a2386..e179ecc 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -372,6 +372,15 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
   NonLoc InputLHS = lhs;
   NonLoc InputRHS = rhs;
 
+  // Constraints may have changed since the creation of a bound SVal. Check if
+  // the values can be simplified based on those new constraints.
+  SVal simplifiedLhs = simplifySVal(state, lhs);
+  SVal simplifiedRhs = simplifySVal(state, rhs);
+  if (auto simplifiedLhsAsNonLoc = simplifiedLhs.getAs<NonLoc>())
+    lhs = *simplifiedLhsAsNonLoc;
+  if (auto simplifiedRhsAsNonLoc = simplifiedRhs.getAs<NonLoc>())
+    rhs = *simplifiedRhsAsNonLoc;
+
   // Handle trivial case where left-side and right-side are the same.
   if (lhs == rhs)
     switch (op) {
@@ -619,16 +628,6 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
         }
       }
 
-      // Does the symbolic expression simplify to a constant?
-      // If so, "fold" the constant by setting 'lhs' to a ConcreteInt
-      // and try again.
-      SVal simplifiedLhs = simplifySVal(state, lhs);
-      if (simplifiedLhs != lhs)
-        if (auto simplifiedLhsAsNonLoc = simplifiedLhs.getAs<NonLoc>()) {
-          lhs = *simplifiedLhsAsNonLoc;
-          continue;
-        }
-
       // Is the RHS a constant?
       if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
         return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
diff --git a/clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp b/clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp
new file mode 100644
index 0000000..f9a0ee2
--- /dev/null
+++ b/clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+// Here we test whether the SValBuilder is capable to simplify existing
+// SVals based on a newly added constraints when evaluating a BinOp.
+
+void clang_analyzer_eval(bool);
+
+void test_evalBinOp_simplifies_lhs(int y) {
+  int x = y / 77;
+  if (y != 77)
+    return;
+
+  // Below `x` is the LHS being simplified.
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+  (void)(x * y);
+}
+
+void test_evalBinOp_simplifies_rhs(int y) {
+  int x = y / 77;
+  if (y != 77)
+    return;
+
+  // Below `x` is the RHS being simplified.
+  clang_analyzer_eval(1 == x); // expected-warning{{TRUE}}
+  (void)(x * y);
+}
-- 
2.7.4