martong updated this revision to Diff 374741.
martong added a comment.

- Break out the movement of RangeConstraintManager into a parent patch, this 
way the diff here is clearly visible and makes the review easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110357

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/constraint-assignor.c

Index: clang/test/Analysis/constraint-assignor.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/constraint-assignor.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -verify
+
+// expected-no-diagnostics
+
+void clang_analyzer_warnIfReached();
+
+void rem_constant_rhs(int x, int y) {
+  if (x % 3 == 0) // x % 3 != 0 -> x != 0
+    return;
+  if (x * y != 0) // x * y == 0
+    return;
+  if (y != 1)     // y == 1     -> x == 0
+    return;
+  clang_analyzer_warnIfReached(); // no-warning
+  (void)x; // keep the constraints alive.
+}
+
+void rem_symbolic_rhs(int x, int y, int z) {
+  if (x % z == 0) // x % 3 != 0 -> x != 0
+    return;
+  if (x * y != 0) // x * y == 0
+    return;
+  if (y != 1)     // y == 1     -> x == 0
+    return;
+  clang_analyzer_warnIfReached(); // no-warning
+  (void)x; // keep the constraints alive.
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1590,23 +1590,44 @@
 public:
   template <class ClassOrSymbol>
   LLVM_NODISCARD static ProgramStateRef
-  assign(ProgramStateRef State, SValBuilder &Builder, RangeSet::Factory &F,
-         ClassOrSymbol CoS, RangeSet NewConstraint) {
+  assign(ProgramStateRef State, RangeConstraintManager *RCM,
+         SValBuilder &Builder, RangeSet::Factory &F, ClassOrSymbol CoS,
+         RangeSet NewConstraint) {
     if (!State || NewConstraint.isEmpty())
       return nullptr;
 
-    ConstraintAssignor Assignor{State, Builder, F};
+    ConstraintAssignor Assignor{State, RCM, Builder, F};
     return Assignor.assign(CoS, NewConstraint);
   }
 
+  template <typename SymT>
+  bool handleRem(const SymT *Sym, RangeSet Constraint) {
+    // a % b != 0 implies that a != 0.
+    if (Sym->getOpcode() != BO_Rem)
+      return true;
+    if (!Constraint.containsZero()) {
+      const SymExpr *LHS = Sym->getLHS();
+      const llvm::APSInt &Zero =
+          Builder.getBasicValueFactory().getValue(0, LHS->getType());
+      State = RCM->assumeSymNE(State, LHS, Zero, Zero);
+      if (!State)
+        return false;
+    }
+    return true;
+  }
+
   inline bool assignSymExprToConst(const SymExpr *Sym, Const Constraint);
+  inline bool assignSymIntExprToRangeSet(const SymIntExpr *Sym,
+                                         RangeSet Constraint) {
+    return handleRem(Sym, Constraint);
+  }
   inline bool assignSymSymExprToRangeSet(const SymSymExpr *Sym,
                                          RangeSet Constraint);
 
 private:
-  ConstraintAssignor(ProgramStateRef State, SValBuilder &Builder,
-                     RangeSet::Factory &F)
-      : State(State), Builder(Builder), RangeFactory(F) {}
+  ConstraintAssignor(ProgramStateRef State, RangeConstraintManager *RCM,
+                     SValBuilder &Builder, RangeSet::Factory &F)
+      : State(State), RCM(RCM), Builder(Builder), RangeFactory(F) {}
   using Base = ConstraintAssignorBase<ConstraintAssignor>;
 
   /// Base method for handling new constraints for symbols.
@@ -1677,15 +1698,14 @@
     if (Constraint.getConcreteValue())
       return !Constraint.getConcreteValue()->isNullValue();
 
-    APSIntType T{Constraint.getMinValue()};
-    Const Zero = T.getZeroValue();
-    if (!Constraint.contains(Zero))
+    if (!Constraint.containsZero())
       return true;
 
     return llvm::None;
   }
 
   ProgramStateRef State;
+  RangeConstraintManager *RCM;
   SValBuilder &Builder;
   RangeSet::Factory &RangeFactory;
 };
@@ -1722,6 +1742,9 @@
 
 bool ConstraintAssignor::assignSymSymExprToRangeSet(const SymSymExpr *Sym,
                                                     RangeSet Constraint) {
+  if (!handleRem(Sym, Constraint))
+    return false;
+
   Optional<bool> ConstraintAsBool = interpreteAsBool(Constraint);
 
   if (!ConstraintAsBool)
@@ -2397,7 +2420,8 @@
 ProgramStateRef RangeConstraintManager::setRange(ProgramStateRef State,
                                                  SymbolRef Sym,
                                                  RangeSet Range) {
-  return ConstraintAssignor::assign(State, getSValBuilder(), F, Sym, Range);
+  return ConstraintAssignor::assign(State, this, getSValBuilder(), F, Sym,
+                                    Range);
 }
 
 //===------------------------------------------------------------------------===
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
@@ -281,6 +281,11 @@
   ///             where N = size(this)
   bool contains(llvm::APSInt Point) const { return containsImpl(Point); }
 
+  bool containsZero() const {
+    APSIntType T{getMinValue()};
+    return contains(T.getZeroValue());
+  }
+
   void dump(raw_ostream &OS) const;
 
   bool operator==(const RangeSet &Other) const { return *Impl == *Other.Impl; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to