Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[analyzer][Solver] Early return if sym is concrete on assuming" #116362

Merged

Conversation

steakhal
Copy link
Contributor

@steakhal steakhal merged commit 8d43c88 into main Nov 15, 2024
5 of 7 checks passed
@steakhal steakhal deleted the revert-115579-fix/clang-analyzer-range-simplify-before-assume branch November 15, 2024 09:28
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Nov 16, 2024
@llvmbot
Copy link

llvmbot commented Nov 16, 2024

@llvm/pr-subscribers-clang

Author: Balazs Benics (steakhal)

Changes

Reverts llvm/llvm-project#115579

This introduced a breakage: https://lab.llvm.org/buildbot/#/builders/46/builds/7928


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

6 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp (-2)
  • (modified) clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp (+1-8)
  • (removed) clang/test/Analysis/solver-sym-simplification-on-assumption.c (-37)
  • (removed) clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c (-43)
  • (modified) clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp (+3-3)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 5faaf9cf274531..4f30b2a0e7e7da 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1354,8 +1354,6 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
             if (BR.isInteresting(ArgSVal))
               OS << Msg;
           }));
-      if (NewNode->isSink())
-        break;
     }
   }
 }
diff --git a/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
index 2b77167fab86f2..c0b3f346b654df 100644
--- a/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
@@ -74,7 +74,7 @@ ConstraintManager::assumeDualImpl(ProgramStateRef &State,
       // it might happen that a Checker uncoditionally uses one of them if the
       // other is a nullptr. This may also happen with the non-dual and
       // adjacent `assume(true)` and `assume(false)` calls. By implementing
-      // assume in terms of assumeDual, we can keep our API contract there as
+      // assume in therms of assumeDual, we can keep our API contract there as
       // well.
       return ProgramStatePair(StInfeasible, StInfeasible);
     }
diff --git a/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
index 171b4c6392d2f8..4bbe933be2129e 100644
--- a/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
@@ -23,14 +23,7 @@ RangedConstraintManager::~RangedConstraintManager() {}
 ProgramStateRef RangedConstraintManager::assumeSym(ProgramStateRef State,
                                                    SymbolRef Sym,
                                                    bool Assumption) {
-  SVal SimplifiedVal = simplifyToSVal(State, Sym);
-  if (SimplifiedVal.isConstant()) {
-    bool Feasible = SimplifiedVal.isZeroConstant() != Assumption;
-    return Feasible ? State : nullptr;
-  }
-
-  if (SymbolRef SimplifiedSym = SimplifiedVal.getAsSymbol())
-    Sym = SimplifiedSym;
+  Sym = simplify(State, Sym);
 
   // Handle SymbolData.
   if (isa<SymbolData>(Sym))
diff --git a/clang/test/Analysis/solver-sym-simplification-on-assumption.c b/clang/test/Analysis/solver-sym-simplification-on-assumption.c
deleted file mode 100644
index c2db144b8a7244..00000000000000
--- a/clang/test/Analysis/solver-sym-simplification-on-assumption.c
+++ /dev/null
@@ -1,37 +0,0 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux-gnu %s \
-// RUN:   -analyzer-checker=debug.ExprInspection \
-// RUN:   -verify
-
-void clang_analyzer_eval(int);
-void clang_analyzer_value(int);
-
-void test_derived_sym_simplification_on_assume(int s0, int s1) {
-  int elem = s0 + s1 + 1;
-  if (elem-- == 0)
-    return;
-
-  if (elem-- == 0)
-    return;
-
-  if (s0 < 1)
-    return;
-  clang_analyzer_value(s0); // expected-warning{{[1, 2147483647]}}
-
-  if (s1 < 1)
-    return;
-  clang_analyzer_value(s1); // expected-warning{{[1, 2147483647]}}
-
-  clang_analyzer_eval(elem); // expected-warning{{UNKNOWN}}
-  if (elem-- == 0)
-    return;
-
-  if (s0 > 1)
-    return;
-  clang_analyzer_eval(s0 == 1); // expected-warning{{TRUE}}
-
-  if (s1 > 1)
-    return;
-  clang_analyzer_eval(s1 == 1); // expected-warning{{TRUE}}
-
-  clang_analyzer_eval(elem); // expected-warning{{FALSE}}
-}
diff --git a/clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c b/clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c
deleted file mode 100644
index 595b8b30ee1dff..00000000000000
--- a/clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c
+++ /dev/null
@@ -1,43 +0,0 @@
-// RUN: %clang_analyze_cc1 %s \
-// RUN:   -analyzer-max-loop 6 \
-// RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
-// RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \
-// RUN:   -analyzer-checker=debug.ExprInspection \
-// RUN:   -triple x86_64-unknown-linux-gnu \
-// RUN:   -verify
-
-#include "Inputs/std-c-library-functions-POSIX.h"
-
-void clang_analyzer_value(int);
-void clang_analyzer_warnIfReached();
-void clang_analyzer_printState();
-
-void _add_one_to_index_C(int *indices, int *shape) {
-  int k = 1;
-  for (; k >= 0; k--)
-    if (indices[k] < shape[k])
-      indices[k]++;
-    else
-      indices[k] = 0;
-}
-
-void PyObject_CopyData_sptr(char *i, char *j, int *indices, int itemsize,
-    int *shape, struct sockaddr *restrict sa) {
-  int elements = 1;
-  for (int k = 0; k < 2; k++)
-    elements += shape[k];
-
-  // no contradiction after 3 iterations when 'elements' could be
-  // simplified to 0
-  int iterations = 0;
-  while (elements--) {
-    iterations++;
-    _add_one_to_index_C(indices, shape);
-    getnameinfo(sa, 10, i, itemsize, i, itemsize, 0); // no crash here
-  }
-
-  if (shape[0] == 1 && shape[1] == 1 && indices[0] == 0 && indices[1] == 0) {
-    clang_analyzer_value(iterations == 3 && elements == -1);
-    // expected-warning@-1{{1}}
-  }
-}
diff --git a/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp b/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
index 3f34d9982e7c8a..679ed3fda7a7a7 100644
--- a/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
+++ b/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
@@ -28,13 +28,13 @@ void test(int a, int b, int c, int d) {
     return;
   clang_analyzer_printState();
   // CHECK:       "constraints": [
-  // CHECK-NEXT:    { "symbol": "((reg_$0<int a>) + (reg_$2<int c>)) != (reg_$3<int d>)", "range": "{ [0, 0] }" },
+  // CHECK-NEXT:    { "symbol": "(reg_$0<int a>) != (reg_$3<int d>)", "range": "{ [0, 0] }" },
   // CHECK-NEXT:    { "symbol": "reg_$1<int b>", "range": "{ [0, 0] }" },
   // CHECK-NEXT:    { "symbol": "reg_$2<int c>", "range": "{ [0, 0] }" }
   // CHECK-NEXT:  ],
   // CHECK-NEXT:  "equivalence_classes": [
-  // CHECK-NEXT:    [ "((reg_$0<int a>) + (reg_$2<int c>)) != (reg_$3<int d>)" ],
-  // CHECK-NEXT:    [ "(reg_$0<int a>) + (reg_$2<int c>)", "reg_$3<int d>" ],
+  // CHECK-NEXT:    [ "(reg_$0<int a>) != (reg_$3<int d>)" ],
+  // CHECK-NEXT:    [ "reg_$0<int a>", "reg_$3<int d>" ],
   // CHECK-NEXT:    [ "reg_$2<int c>" ]
   // CHECK-NEXT:  ],
   // CHECK-NEXT:  "disequality_info": null,

@llvmbot
Copy link

llvmbot commented Nov 16, 2024

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

Author: Balazs Benics (steakhal)

Changes

Reverts llvm/llvm-project#115579

This introduced a breakage: https://lab.llvm.org/buildbot/#/builders/46/builds/7928


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

6 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp (-2)
  • (modified) clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp (+1-8)
  • (removed) clang/test/Analysis/solver-sym-simplification-on-assumption.c (-37)
  • (removed) clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c (-43)
  • (modified) clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp (+3-3)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 5faaf9cf274531..4f30b2a0e7e7da 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1354,8 +1354,6 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
             if (BR.isInteresting(ArgSVal))
               OS << Msg;
           }));
-      if (NewNode->isSink())
-        break;
     }
   }
 }
diff --git a/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
index 2b77167fab86f2..c0b3f346b654df 100644
--- a/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
@@ -74,7 +74,7 @@ ConstraintManager::assumeDualImpl(ProgramStateRef &State,
       // it might happen that a Checker uncoditionally uses one of them if the
       // other is a nullptr. This may also happen with the non-dual and
       // adjacent `assume(true)` and `assume(false)` calls. By implementing
-      // assume in terms of assumeDual, we can keep our API contract there as
+      // assume in therms of assumeDual, we can keep our API contract there as
       // well.
       return ProgramStatePair(StInfeasible, StInfeasible);
     }
diff --git a/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
index 171b4c6392d2f8..4bbe933be2129e 100644
--- a/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
@@ -23,14 +23,7 @@ RangedConstraintManager::~RangedConstraintManager() {}
 ProgramStateRef RangedConstraintManager::assumeSym(ProgramStateRef State,
                                                    SymbolRef Sym,
                                                    bool Assumption) {
-  SVal SimplifiedVal = simplifyToSVal(State, Sym);
-  if (SimplifiedVal.isConstant()) {
-    bool Feasible = SimplifiedVal.isZeroConstant() != Assumption;
-    return Feasible ? State : nullptr;
-  }
-
-  if (SymbolRef SimplifiedSym = SimplifiedVal.getAsSymbol())
-    Sym = SimplifiedSym;
+  Sym = simplify(State, Sym);
 
   // Handle SymbolData.
   if (isa<SymbolData>(Sym))
diff --git a/clang/test/Analysis/solver-sym-simplification-on-assumption.c b/clang/test/Analysis/solver-sym-simplification-on-assumption.c
deleted file mode 100644
index c2db144b8a7244..00000000000000
--- a/clang/test/Analysis/solver-sym-simplification-on-assumption.c
+++ /dev/null
@@ -1,37 +0,0 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux-gnu %s \
-// RUN:   -analyzer-checker=debug.ExprInspection \
-// RUN:   -verify
-
-void clang_analyzer_eval(int);
-void clang_analyzer_value(int);
-
-void test_derived_sym_simplification_on_assume(int s0, int s1) {
-  int elem = s0 + s1 + 1;
-  if (elem-- == 0)
-    return;
-
-  if (elem-- == 0)
-    return;
-
-  if (s0 < 1)
-    return;
-  clang_analyzer_value(s0); // expected-warning{{[1, 2147483647]}}
-
-  if (s1 < 1)
-    return;
-  clang_analyzer_value(s1); // expected-warning{{[1, 2147483647]}}
-
-  clang_analyzer_eval(elem); // expected-warning{{UNKNOWN}}
-  if (elem-- == 0)
-    return;
-
-  if (s0 > 1)
-    return;
-  clang_analyzer_eval(s0 == 1); // expected-warning{{TRUE}}
-
-  if (s1 > 1)
-    return;
-  clang_analyzer_eval(s1 == 1); // expected-warning{{TRUE}}
-
-  clang_analyzer_eval(elem); // expected-warning{{FALSE}}
-}
diff --git a/clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c b/clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c
deleted file mode 100644
index 595b8b30ee1dff..00000000000000
--- a/clang/test/Analysis/std-c-library-functions-bufsize-nocrash-with-correct-solver.c
+++ /dev/null
@@ -1,43 +0,0 @@
-// RUN: %clang_analyze_cc1 %s \
-// RUN:   -analyzer-max-loop 6 \
-// RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
-// RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \
-// RUN:   -analyzer-checker=debug.ExprInspection \
-// RUN:   -triple x86_64-unknown-linux-gnu \
-// RUN:   -verify
-
-#include "Inputs/std-c-library-functions-POSIX.h"
-
-void clang_analyzer_value(int);
-void clang_analyzer_warnIfReached();
-void clang_analyzer_printState();
-
-void _add_one_to_index_C(int *indices, int *shape) {
-  int k = 1;
-  for (; k >= 0; k--)
-    if (indices[k] < shape[k])
-      indices[k]++;
-    else
-      indices[k] = 0;
-}
-
-void PyObject_CopyData_sptr(char *i, char *j, int *indices, int itemsize,
-    int *shape, struct sockaddr *restrict sa) {
-  int elements = 1;
-  for (int k = 0; k < 2; k++)
-    elements += shape[k];
-
-  // no contradiction after 3 iterations when 'elements' could be
-  // simplified to 0
-  int iterations = 0;
-  while (elements--) {
-    iterations++;
-    _add_one_to_index_C(indices, shape);
-    getnameinfo(sa, 10, i, itemsize, i, itemsize, 0); // no crash here
-  }
-
-  if (shape[0] == 1 && shape[1] == 1 && indices[0] == 0 && indices[1] == 0) {
-    clang_analyzer_value(iterations == 3 && elements == -1);
-    // expected-warning@-1{{1}}
-  }
-}
diff --git a/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp b/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
index 3f34d9982e7c8a..679ed3fda7a7a7 100644
--- a/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
+++ b/clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
@@ -28,13 +28,13 @@ void test(int a, int b, int c, int d) {
     return;
   clang_analyzer_printState();
   // CHECK:       "constraints": [
-  // CHECK-NEXT:    { "symbol": "((reg_$0<int a>) + (reg_$2<int c>)) != (reg_$3<int d>)", "range": "{ [0, 0] }" },
+  // CHECK-NEXT:    { "symbol": "(reg_$0<int a>) != (reg_$3<int d>)", "range": "{ [0, 0] }" },
   // CHECK-NEXT:    { "symbol": "reg_$1<int b>", "range": "{ [0, 0] }" },
   // CHECK-NEXT:    { "symbol": "reg_$2<int c>", "range": "{ [0, 0] }" }
   // CHECK-NEXT:  ],
   // CHECK-NEXT:  "equivalence_classes": [
-  // CHECK-NEXT:    [ "((reg_$0<int a>) + (reg_$2<int c>)) != (reg_$3<int d>)" ],
-  // CHECK-NEXT:    [ "(reg_$0<int a>) + (reg_$2<int c>)", "reg_$3<int d>" ],
+  // CHECK-NEXT:    [ "(reg_$0<int a>) != (reg_$3<int d>)" ],
+  // CHECK-NEXT:    [ "reg_$0<int a>", "reg_$3<int d>" ],
   // CHECK-NEXT:    [ "reg_$2<int c>" ]
   // CHECK-NEXT:  ],
   // CHECK-NEXT:  "disequality_info": null,

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.

2 participants