Skip to content

Conversation

@steakhal
Copy link
Contributor

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Oct 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Balazs Benics (steakhal)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/112891.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp (+45-31)
  • (modified) clang/test/Analysis/unary-sym-expr.c (+30-3)
diff --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index ecf7974c838650..c39fa81109c853 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1249,6 +1249,8 @@ class SymbolicRangeInferrer
         // calculate the effective range set by intersecting the range set
         // for A - B and the negated range set of B - A.
         getRangeForNegatedSymSym(SSE),
+        // If commutative, we may have constaints for the commuted variant.
+        getRangeCommutativeSymSym(SSE),
         // If Sym is a comparison expression (except <=>),
         // find any other comparisons with the same operands.
         // See function description.
@@ -1485,6 +1487,21 @@ class SymbolicRangeInferrer
         Sym->getType());
   }
 
+  std::optional<RangeSet> getRangeCommutativeSymSym(const SymSymExpr *SSE) {
+    auto Op = SSE->getOpcode();
+    bool IsCommutative = llvm::is_contained(
+        // ==, !=, |, &, +, *, ^
+        {BO_EQ, BO_NE, BO_Or, BO_And, BO_Add, BO_Mul, BO_Xor}, Op);
+    if (!IsCommutative)
+      return std::nullopt;
+
+    SymbolRef Commuted = State->getSymbolManager().getSymSymExpr(
+        SSE->getRHS(), Op, SSE->getLHS(), SSE->getType());
+    if (const RangeSet *Range = getConstraint(State, Commuted))
+      return *Range;
+    return std::nullopt;
+  }
+
   // Returns ranges only for binary comparison operators (except <=>)
   // when left and right operands are symbolic values.
   // Finds any other comparisons with the same operands.
@@ -1936,27 +1953,27 @@ class RangeConstraintManager : public RangedConstraintManager {
       const llvm::APSInt &To, const llvm::APSInt &Adjustment) override;
 
 private:
-  RangeSet::Factory F;
+  mutable RangeSet::Factory F;
 
-  RangeSet getRange(ProgramStateRef State, SymbolRef Sym);
+  RangeSet getRange(ProgramStateRef State, SymbolRef Sym) const;
   ProgramStateRef setRange(ProgramStateRef State, SymbolRef Sym,
                            RangeSet Range);
 
   RangeSet getSymLTRange(ProgramStateRef St, SymbolRef Sym,
                          const llvm::APSInt &Int,
-                         const llvm::APSInt &Adjustment);
+                         const llvm::APSInt &Adjustment) const;
   RangeSet getSymGTRange(ProgramStateRef St, SymbolRef Sym,
                          const llvm::APSInt &Int,
-                         const llvm::APSInt &Adjustment);
+                         const llvm::APSInt &Adjustment) const;
   RangeSet getSymLERange(ProgramStateRef St, SymbolRef Sym,
                          const llvm::APSInt &Int,
-                         const llvm::APSInt &Adjustment);
+                         const llvm::APSInt &Adjustment) const;
   RangeSet getSymLERange(llvm::function_ref<RangeSet()> RS,
                          const llvm::APSInt &Int,
-                         const llvm::APSInt &Adjustment);
+                         const llvm::APSInt &Adjustment) const;
   RangeSet getSymGERange(ProgramStateRef St, SymbolRef Sym,
                          const llvm::APSInt &Int,
-                         const llvm::APSInt &Adjustment);
+                         const llvm::APSInt &Adjustment) const;
 };
 
 //===----------------------------------------------------------------------===//
@@ -2863,21 +2880,18 @@ ConditionTruthVal RangeConstraintManager::checkNull(ProgramStateRef State,
 
 const llvm::APSInt *RangeConstraintManager::getSymVal(ProgramStateRef St,
                                                       SymbolRef Sym) const {
-  auto &MutableSelf = const_cast<RangeConstraintManager &>(*this);
-  return MutableSelf.getRange(St, Sym).getConcreteValue();
+  return getRange(St, Sym).getConcreteValue();
 }
 
 const llvm::APSInt *RangeConstraintManager::getSymMinVal(ProgramStateRef St,
                                                          SymbolRef Sym) const {
-  auto &MutableSelf = const_cast<RangeConstraintManager &>(*this);
-  RangeSet Range = MutableSelf.getRange(St, Sym);
+  RangeSet Range = getRange(St, Sym);
   return Range.isEmpty() ? nullptr : &Range.getMinValue();
 }
 
 const llvm::APSInt *RangeConstraintManager::getSymMaxVal(ProgramStateRef St,
                                                          SymbolRef Sym) const {
-  auto &MutableSelf = const_cast<RangeConstraintManager &>(*this);
-  RangeSet Range = MutableSelf.getRange(St, Sym);
+  RangeSet Range = getRange(St, Sym);
   return Range.isEmpty() ? nullptr : &Range.getMaxValue();
 }
 
@@ -3022,7 +3036,7 @@ RangeConstraintManager::removeDeadBindings(ProgramStateRef State,
 }
 
 RangeSet RangeConstraintManager::getRange(ProgramStateRef State,
-                                          SymbolRef Sym) {
+                                          SymbolRef Sym) const {
   return SymbolicRangeInferrer::inferRange(F, State, Sym);
 }
 
@@ -3077,10 +3091,10 @@ RangeConstraintManager::assumeSymEQ(ProgramStateRef St, SymbolRef Sym,
   return setRange(St, Sym, New);
 }
 
-RangeSet RangeConstraintManager::getSymLTRange(ProgramStateRef St,
-                                               SymbolRef Sym,
-                                               const llvm::APSInt &Int,
-                                               const llvm::APSInt &Adjustment) {
+RangeSet
+RangeConstraintManager::getSymLTRange(ProgramStateRef St, SymbolRef Sym,
+                                      const llvm::APSInt &Int,
+                                      const llvm::APSInt &Adjustment) const {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
   switch (AdjustmentType.testInRange(Int, true)) {
@@ -3114,10 +3128,10 @@ RangeConstraintManager::assumeSymLT(ProgramStateRef St, SymbolRef Sym,
   return setRange(St, Sym, New);
 }
 
-RangeSet RangeConstraintManager::getSymGTRange(ProgramStateRef St,
-                                               SymbolRef Sym,
-                                               const llvm::APSInt &Int,
-                                               const llvm::APSInt &Adjustment) {
+RangeSet
+RangeConstraintManager::getSymGTRange(ProgramStateRef St, SymbolRef Sym,
+                                      const llvm::APSInt &Int,
+                                      const llvm::APSInt &Adjustment) const {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
   switch (AdjustmentType.testInRange(Int, true)) {
@@ -3151,10 +3165,10 @@ RangeConstraintManager::assumeSymGT(ProgramStateRef St, SymbolRef Sym,
   return setRange(St, Sym, New);
 }
 
-RangeSet RangeConstraintManager::getSymGERange(ProgramStateRef St,
-                                               SymbolRef Sym,
-                                               const llvm::APSInt &Int,
-                                               const llvm::APSInt &Adjustment) {
+RangeSet
+RangeConstraintManager::getSymGERange(ProgramStateRef St, SymbolRef Sym,
+                                      const llvm::APSInt &Int,
+                                      const llvm::APSInt &Adjustment) const {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
   switch (AdjustmentType.testInRange(Int, true)) {
@@ -3191,7 +3205,7 @@ RangeConstraintManager::assumeSymGE(ProgramStateRef St, SymbolRef Sym,
 RangeSet
 RangeConstraintManager::getSymLERange(llvm::function_ref<RangeSet()> RS,
                                       const llvm::APSInt &Int,
-                                      const llvm::APSInt &Adjustment) {
+                                      const llvm::APSInt &Adjustment) const {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
   switch (AdjustmentType.testInRange(Int, true)) {
@@ -3217,10 +3231,10 @@ RangeConstraintManager::getSymLERange(llvm::function_ref<RangeSet()> RS,
   return F.intersect(Default, Lower, Upper);
 }
 
-RangeSet RangeConstraintManager::getSymLERange(ProgramStateRef St,
-                                               SymbolRef Sym,
-                                               const llvm::APSInt &Int,
-                                               const llvm::APSInt &Adjustment) {
+RangeSet
+RangeConstraintManager::getSymLERange(ProgramStateRef St, SymbolRef Sym,
+                                      const llvm::APSInt &Int,
+                                      const llvm::APSInt &Adjustment) const {
   return getSymLERange([&] { return getRange(St, Sym); }, Int, Adjustment);
 }
 
diff --git a/clang/test/Analysis/unary-sym-expr.c b/clang/test/Analysis/unary-sym-expr.c
index 7c4774f3cca82f..92e11b295bee7c 100644
--- a/clang/test/Analysis/unary-sym-expr.c
+++ b/clang/test/Analysis/unary-sym-expr.c
@@ -29,12 +29,39 @@ int test(int x, int y) {
   return 42;
 }
 
-void test_svalbuilder_simplification(int x, int y) {
+void test_svalbuilder_simplification_add(int x, int y) {
   if (x + y != 3)
     return;
   clang_analyzer_eval(-(x + y) == -3); // expected-warning{{TRUE}}
-  // FIXME Commutativity is not supported yet.
-  clang_analyzer_eval(-(y + x) == -3); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(-(y + x) == -3); // expected-warning{{TRUE}}
+}
+
+void test_svalbuilder_simplification_mul(int x, int y) {
+  if (x * y != 3)
+    return;
+  clang_analyzer_eval(-(x * y) == -3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(-(y * x) == -3); // expected-warning{{TRUE}}
+}
+
+void test_svalbuilder_simplification_and(int x, int y) {
+  if ((x & y) != 3)
+    return;
+  clang_analyzer_eval(-(x & y) == -3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(-(y & x) == -3); // expected-warning{{TRUE}}
+}
+
+void test_svalbuilder_simplification_or(int x, int y) {
+  if ((x | y) != 3)
+    return;
+  clang_analyzer_eval(-(x | y) == -3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(-(y | x) == -3); // expected-warning{{TRUE}}
+}
+
+void test_svalbuilder_simplification_xor(int x, int y) {
+  if ((x ^ y) != 3)
+    return;
+  clang_analyzer_eval(-(x ^ y) == -3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(-(y ^ x) == -3); // expected-warning{{TRUE}}
 }
 
 int test_fp(int flag) {

@steakhal steakhal requested a review from NagyDonat October 18, 2024 12:14
Copy link
Contributor

@NagyDonat NagyDonat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, straightforward NFC improvement. Thanks for cleaning this up!

(Currently the diff is a bit confusing, because this cleanup builds on the not-yet-merged commit #112887, but that's just GUI awkwardness.)

@steakhal steakhal merged commit 1b49ee7 into llvm:main Oct 18, 2024
5 of 6 checks passed
@steakhal steakhal deleted the improve-solver-cleanup branch October 18, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants