[clang] 44fa31f - [Analyzer][solver] Fix inconsistent equivalence class data

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2021-07-23T14:25:32+02:00
New Revision: 44fa31fa6da0c573ed972ec1d932c4ca3fb52f8f

URL: https://github.com/llvm/llvm-project/commit/44fa31fa6da0c573ed972ec1d932c4ca3fb52f8f
DIFF: https://github.com/llvm/llvm-project/commit/44fa31fa6da0c573ed972ec1d932c4ca3fb52f8f.diff

LOG: [Analyzer][solver] Fix inconsistent equivalence class data

https://bugs.llvm.org/show_bug.cgi?id=51109

When we merged two classes, `*this` became an obsolete representation of
the new `State`. This is b/c the member relations had changed during the
previous merge of another member of the same class in a way that `*this`
had no longer any members. (`mergeImpl` might keep the member relations
to `Other` and could dissolve `*this`.)

Differential Revision: https://reviews.llvm.org/D106285

Added: 
    clang/test/Analysis/solver-sym-simplification-no-crash2.c

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index 9f64be7a5309..de86fc685b07 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -600,9 +600,10 @@ class EquivalenceClass : public llvm::FoldingSetNode {
   areEqual(ProgramStateRef State, SymbolRef First, SymbolRef Second);
 
   /// Iterate over all symbols and try to simplify them.
-  LLVM_NODISCARD ProgramStateRef simplify(SValBuilder &SVB,
-                                          RangeSet::Factory &F,
-                                          ProgramStateRef State);
+  LLVM_NODISCARD static inline ProgramStateRef simplify(SValBuilder &SVB,
+                                                        RangeSet::Factory &F,
+                                                        ProgramStateRef State,
+                                                        EquivalenceClass Class);
 
   void dumpToStream(ProgramStateRef State, raw_ostream &os) const;
   LLVM_DUMP_METHOD void dump(ProgramStateRef State) const {
@@ -1696,7 +1697,7 @@ bool ConstraintAssignor::assignSymExprToConst(const SymExpr *Sym,
   ClassMembersTy Members = State->get<ClassMembers>();
   for (std::pair<EquivalenceClass, SymbolSet> ClassToSymbolSet : Members) {
     EquivalenceClass Class = ClassToSymbolSet.first;
-    State = Class.simplify(Builder, RangeFactory, State);
+    State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class);
     if (!State)
       return false;
     SimplifiedClasses.insert(Class);
@@ -1710,7 +1711,7 @@ bool ConstraintAssignor::assignSymExprToConst(const SymExpr *Sym,
     EquivalenceClass Class = ClassConstraint.first;
     if (SimplifiedClasses.count(Class)) // Already simplified.
       continue;
-    State = Class.simplify(Builder, RangeFactory, State);
+    State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class);
     if (!State)
       return false;
   }
@@ -2090,18 +2091,17 @@ inline Optional<bool> EquivalenceClass::areEqual(ProgramStateRef State,
 // class to this class. This way, we simplify not just the symbols but the
 // classes as well: we strive to keep the number of the classes to be the
 // absolute minimum.
-LLVM_NODISCARD ProgramStateRef EquivalenceClass::simplify(
-    SValBuilder &SVB, RangeSet::Factory &F, ProgramStateRef State) {
-  SymbolSet ClassMembers = getClassMembers(State);
+LLVM_NODISCARD ProgramStateRef
+EquivalenceClass::simplify(SValBuilder &SVB, RangeSet::Factory &F,
+                           ProgramStateRef State, EquivalenceClass Class) {
+  SymbolSet ClassMembers = Class.getClassMembers(State);
   for (const SymbolRef &MemberSym : ClassMembers) {
     SymbolRef SimplifiedMemberSym = ento::simplify(State, MemberSym);
     if (SimplifiedMemberSym && MemberSym != SimplifiedMemberSym) {
-      EquivalenceClass ClassOfSimplifiedSym =
-          EquivalenceClass::find(State, SimplifiedMemberSym);
       // The simplified symbol should be the member of the original Class,
       // however, it might be in another existing class at the moment. We
       // have to merge these classes.
-      State = merge(F, State, ClassOfSimplifiedSym);
+      State = merge(F, State, MemberSym, SimplifiedMemberSym);
       if (!State)
         return nullptr;
     }

diff  --git a/clang/test/Analysis/solver-sym-simplification-no-crash2.c b/clang/test/Analysis/solver-sym-simplification-no-crash2.c
new file mode 100644
index 000000000000..7963c1462e32
--- /dev/null
+++ b/clang/test/Analysis/solver-sym-simplification-no-crash2.c
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -verify
+
+// Here, we test that symbol simplification in the solver does not produce any
+// crashes.
+// https://bugs.llvm.org/show_bug.cgi?id=51109
+
+// expected-no-diagnostics
+
+int a, b, c, d;
+void f() {
+  a = -1;
+  d = b * a;
+  a = d / c;
+  if (a < 7 / b)
+    return;
+  if (d *a / c < 7 / b)
+    return;
+  if (b == 1 && c == -1)
+    return;
+}