manas created this revision.
manas added a reviewer: steakhal.
Herald added subscribers: ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
manas requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes certain cases where solver was not able to infer
disequality due to overlapping of values in rangeset. This case was
casting from lower signed type to bigger unsigned type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140086

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/constant-folding.c

Index: clang/test/Analysis/constant-folding.c
===================================================================
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -303,7 +303,8 @@
 
   if (s1 < 0 && s1 > -4 && u1 > UINT_MAX - 4 && u1 < UINT_MAX - 1) {
     // s1: [-3, -1], u1: [UINT_MAX - 3, UINT_MAX - 2]
-    clang_analyzer_eval(u1 != s1); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(u1 != s1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(s1 != u1); // expected-warning{{TRUE}}
   }
 
   if (s1 < 1 && s1 > -6 && s1 != -4 && s1 != -3 &&
@@ -400,12 +401,10 @@
     clang_analyzer_eval(uch != sch); // expected-warning{{UNKNOWN}}
   }
 
-  // FIXME: Casting smaller signed types to unsigned one may leave us with
-  // overlapping values, falsely indicating UNKNOWN, where it is possible to
-  // assert TRUE.
   if (uch > 1 && sch < 1) {
     // uch: [2, UCHAR_MAX], sch: [SCHAR_MIN, 0]
-    clang_analyzer_eval(uch != sch); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(uch != sch); // expected-warning{{TRUE}}
+    clang_analyzer_eval(sch != uch); // expected-warning{{TRUE}}
   }
 
   if (uch <= 1 && uch >= 1 && sch <= 1 && sch >= 1) {
@@ -419,10 +418,9 @@
     clang_analyzer_eval(ush != ssh); // expected-warning{{UNKNOWN}}
   }
 
-  // FIXME: Casting leave us with overlapping values. Should be TRUE.
   if (ush > 1 && ssh < 1) {
     // ush: [2, USHRT_MAX], ssh: [SHRT_MIN, 0]
-    clang_analyzer_eval(ush != ssh); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(ush != ssh); // expected-warning{{TRUE}}
   }
 
   if (ush <= 1 && ush >= 1 && ssh <= 1 && ssh >= 1) {
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -17,6 +17,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/STLExtras.h"
@@ -1624,7 +1625,52 @@
   if (LHS.getAPSIntType() == RHS.getAPSIntType()) {
     if (intersect(RangeFactory, LHS, RHS).isEmpty())
       return getTrueRange(T);
+
   } else {
+    // We can only lose information if we are casting smaller signed type to
+    // bigger unsigned type. For e.g.,
+    //    LHS (unsigned short): [2, USHRT_MAX]
+    //    RHS   (signed short): [SHRT_MIN, 0]
+    //
+    // Casting RHS to LHS type will leave us with overlapping values
+    //    CastedRHS : [0, 0] U [SHRT_MAX + 1, USHRT_MAX]
+    //
+    // We can avoid this by checking if signed type's maximum value is lesser
+    // than unsigned type's minimum value.
+
+    // If both have different signs then only we can get more information.
+    if (LHS.isUnsigned() ^ RHS.isUnsigned()) {
+      if (LHS.isUnsigned() && (LHS.getBitWidth() >= RHS.getBitWidth())) {
+
+        // If signed range is <Zero, then we can simply infer that expression
+        // will return true.
+        llvm::APSInt Zero = RHS.getAPSIntType().getZeroValue();
+        bool IsRHSNegative = RHS.getMaxValue() < Zero;
+        if (IsRHSNegative)
+          return getTrueRange(T);
+
+        // If signed range may have APSInt >=Zero, then maximum value of signed
+        // type should be lesser than minimum value of unsigned type.
+        const llvm::APSInt CastedRHSMax =
+            LHS.getAPSIntType().convert(RHS.getMaxValue());
+
+        if (CastedRHSMax < LHS.getMinValue())
+          return getTrueRange(T);
+
+      } else if (RHS.isUnsigned() && (LHS.getBitWidth() <= RHS.getBitWidth())) {
+
+        llvm::APSInt Zero = LHS.getAPSIntType().getZeroValue();
+        bool IsLHSNegative = LHS.getMaxValue() < Zero;
+        if (IsLHSNegative)
+          return getTrueRange(T);
+
+        const llvm::APSInt CastedLHSMax =
+            RHS.getAPSIntType().convert(LHS.getMaxValue());
+        if (CastedLHSMax < RHS.getMinValue())
+          return getTrueRange(T);
+      }
+    }
+
     // Both RangeSets should be casted to bigger unsigned type.
     APSIntType CastingType(std::max(LHS.getBitWidth(), RHS.getBitWidth()),
                            LHS.isUnsigned() || RHS.isUnsigned());
@@ -2147,7 +2193,6 @@
   RangeSet::Factory &RangeFactory;
 };
 
-
 bool ConstraintAssignor::assignSymExprToConst(const SymExpr *Sym,
                                               const llvm::APSInt &Constraint) {
   llvm::SmallSet<EquivalenceClass, 4> SimplifiedClasses;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to