[analyzer] Drop assertion enforcing that assume args are known constants #151908
Conversation
We sometimes don't know if the operand of [[assume]] is true/false, and that's okay. We can just ignore the attribute in that case. If we wanted something more fancy, we could bring the assumption to the constraints, but dropping them should be just as fine for now.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balazs Benics (steakhal) ChangesWe sometimes don't know if the operand of [[assume]] is true/false, and that's okay. We can just ignore the attribute in that case. If we wanted something more fancy, we could bring the assumption to the constraints, but dropping them should be just as fine for now. Fixes #151854 Full diff: https://github.com/llvm/llvm-project/pull/151908.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp
index 1e3adb4f266ca..789c7772d123a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/AssumeModeling.cpp
@@ -45,7 +45,6 @@ void AssumeModelingChecker::checkPostStmt(const AttributedStmt *A,
continue;
const auto *Assumption = AssumptionVal.getAsInteger();
- assert(Assumption && "We should know the exact outcome of an assume expr");
if (Assumption && Assumption->isZero()) {
C.addSink();
}
diff --git a/clang/test/Analysis/cxx23-assume-attribute.cpp b/clang/test/Analysis/cxx23-assume-attribute.cpp
index 86e7662cd2af9..dd15ff5d43505 100644
--- a/clang/test/Analysis/cxx23-assume-attribute.cpp
+++ b/clang/test/Analysis/cxx23-assume-attribute.cpp
@@ -69,3 +69,8 @@ int assume_and_fallthrough_at_the_same_attrstmt(int a, int b) {
return 0;
}
+
+void assume_opaque_gh151854_no_crash() {
+ extern bool opaque();
+ [[assume(opaque())]]; // no-crash
+}
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/32123 Here is the relevant piece of the build log for the reference |
Fixed by 6c9f1ce. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/32629 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/34807 Here is the relevant piece of the build log for the reference |
|
@tru I have this (0a1eff2) simple patch that I want to backport to clang-21, however, it broke the CI build on main, so there was a followup patch (6c9f1ce) that consolidated the bots. FYI: both of these should apply cleanly if applied in the respective order. |
|
@steakhal you can cherry-pick both at the same time with cherry-pick that works for me. |
|
/pull-request #155284 |
…nts (llvm#151908) We sometimes don't know if the operand of [[assume]] is true/false, and that's okay. We can just ignore the attribute in that case. If we wanted something more fancy, we could bring the assumption to the constraints, but dropping them should be just as fine for now. Fixes llvm#151854 (cherry picked from commit 0a1eff2)
(cherry picked from commit 6c9f1ce)
5 participants
We sometimes don't know if the operand of [[assume]] is true/false, and that's okay. We can just ignore the attribute in that case.
If we wanted something more fancy, we could bring the assumption to the constraints, but dropping them should be just as fine for now.
Fixes #151854