[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/huang-me created
https://github.com/llvm/llvm-project/pull/82089
StaticAnalyzer didn't check if the variable is declared in `CompoundStmt` under
`SwitchStmt`, which make static analyzer reach root without finding the
declaration.
Fixes #68819
>From a590abda0570d922eb7032096de6fdd8cbbe4c63 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH] Fix crash on StaticAnalyzer loop unrolling
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..b91dfa26774aa4 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for(const Stmt *CB: dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (huang-me)
Changes
StaticAnalyzer didn't check if the variable is declared in `CompoundStmt` under
`SwitchStmt`, which make static analyzer reach root without finding the
declaration.
Fixes #68819
---
Full diff: https://github.com/llvm/llvm-project/pull/82089.diff
1 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp (+11)
``diff
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..b91dfa26774aa4 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for(const Stmt *CB: dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
``
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: None (huang-me)
Changes
StaticAnalyzer didn't check if the variable is declared in `CompoundStmt` under
`SwitchStmt`, which make static analyzer reach root without finding the
declaration.
Fixes #68819
---
Full diff: https://github.com/llvm/llvm-project/pull/82089.diff
1 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp (+11)
``diff
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..b91dfa26774aa4 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for(const Stmt *CB: dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
``
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 8ba915fe1096de290f5e569d6de97a8bcc8652f7
a590abda0570d922eb7032096de6fdd8cbbe4c63 --
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index b91dfa2677..4001268bde 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -228,7 +228,7 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
}
if (const SwitchStmt *SS = dyn_cast(S)) {
- for(const Stmt *CB: dyn_cast(SS->getBody())->body()) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
for (const Decl *D : dyn_cast(CB)->decls()) {
// Once we reach the declaration of the VD we can return.
if (D->getCanonicalDecl() == VD)
``
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/huang-me updated
https://github.com/llvm/llvm-project/pull/82089
>From 4ebdf475b79d059c97881ad83aeecf677fb32631 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH] Fix crash on StaticAnalyzer loop unrolling
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..205f13fdcb81e3 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for(const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/huang-me updated
https://github.com/llvm/llvm-project/pull/82089
>From 2802ef4b9ed88da3cacb16ab7738907ee806 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH] Fix crash on StaticAnalyzer loop unrolling
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..4001268bde6677 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
NagyDonat wrote:
Be careful, this may cause a crash in standard-compliant but unusual code where
the body of a `switch` statement is *not* a compound statement.
For example code like
```
switch (get_value()) case 0: case 1: run_foobar();
```
is completely valid; and here the body of the switch statement is a `CaseStmt`
(which has another case statement as a child).
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/NagyDonat requested changes to this pull request. https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
NagyDonat wrote:
And this will crash on all statements that are within a switch and not
`DeclStmt`s (because the `dyn_cast` returns a nullpointer and you're calling a
method on it).
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
huang-me wrote:
I didn't think of this scenario, thanks for reminding me.
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
huang-me wrote:
Oh, that's a stupid mistake. Thanks again.
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/huang-me updated
https://github.com/llvm/llvm-project/pull/82089
>From 2802ef4b9ed88da3cacb16ab7738907ee806 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH 1/2] Fix crash on StaticAnalyzer loop unrolling
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..4001268bde6677 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
>From e9e195e4462da7f3ca2317096ddace6ce3e88d13 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Mon, 19 Feb 2024 18:17:27 +0800
Subject: [PATCH 2/2] Check if dynamic cast get pointer to valid elements
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index 4001268bde6677..093e9bbf4ce5e0 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -228,11 +228,15 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
}
if (const SwitchStmt *SS = dyn_cast(S)) {
- for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
-for (const Decl *D : dyn_cast(CB)->decls()) {
- // Once we reach the declaration of the VD we can return.
- if (D->getCanonicalDecl() == VD)
-return false;
+ if (const CompoundStmt *CST = dyn_cast(SS->getBody())) {
+for (const Stmt *CB : CST->body()) {
+ if (const DeclStmt *DST = dyn_cast(CB)) {
+for (const Decl *D : DST->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
NagyDonat wrote:
If I understand it correctly, your change doesn't handle declarations that are
in inner statements, e.g. the variable "x" in the following code:
```cpp
switch (get_value()) {
case 42:
do {
int x;
// ...
} while (running);
//...
}
```
Is this compatible with the goals of your commit, or would the original crash
remain in a situation like this?
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
huang-me wrote:
> If I understand it correctly, your change doesn't handle declarations that
> are in inner statements, e.g. the variable "x" in the following code:
>
> ```c++
> switch (get_value()) {
> case 42:
> do {
> int x;
> // ...
> } while (running);
> //...
> }
> ```
>
> Is this compatible with the goals of your commit, or would the original crash
> remain in a situation like this?
As far as I understand it, the declaration within the `CaseStmt` would be found
before reaching `SwitchStmt` so I don't think we need to consider this
situation.
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
NagyDonat wrote: That sounds reasonable. Unfortunately I don't know much about the context of this change, so let's wait for a review from @danix800 (or somebody else who's knows enough) before merging this change. Also, perhaps it would be a good idea to add a testcase (or a few of them) in the directory `clang/test/Analysis` to prevent regressions and illustrate the goals of this change. (As far as I know our test system is just a heap of random C/C++ files with a "how to compile/analyze this" header and "what warnings/errors should appear" markers in comments. I usually add testcases by following the example of older analogous cases.) https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/huang-me updated
https://github.com/llvm/llvm-project/pull/82089
>From 2802ef4b9ed88da3cacb16ab7738907ee806 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH 1/3] Fix crash on StaticAnalyzer loop unrolling
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..4001268bde6677 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
>From e9e195e4462da7f3ca2317096ddace6ce3e88d13 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Mon, 19 Feb 2024 18:17:27 +0800
Subject: [PATCH 2/3] Check if dynamic cast get pointer to valid elements
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index 4001268bde6677..093e9bbf4ce5e0 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -228,11 +228,15 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
}
if (const SwitchStmt *SS = dyn_cast(S)) {
- for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
-for (const Decl *D : dyn_cast(CB)->decls()) {
- // Once we reach the declaration of the VD we can return.
- if (D->getCanonicalDecl() == VD)
-return false;
+ if (const CompoundStmt *CST = dyn_cast(SS->getBody())) {
+for (const Stmt *CB : CST->body()) {
+ if (const DeclStmt *DST = dyn_cast(CB)) {
+for (const Decl *D : DST->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
}
}
}
>From 6ed9ea88865e91f1727077b1a3a24d7b110060fd Mon Sep 17 00:00:00 2001
From: huang-me
Date: Tue, 20 Feb 2024 11:31:23 +0800
Subject: [PATCH 3/3] Add testcase for finding declaration within SwitchStmt
---
.../test-escaping-on-var-before-switch-case.c | 11 +++
1 file changed, 11 insertions(+)
create mode 100644
clang/test/Analysis/test-escaping-on-var-before-switch-case.c
diff --git a/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
b/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
new file mode 100644
index 00..95aed8cab06b55
--- /dev/null
+++ b/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config
unroll-loops=true -verify %s
+
+void test_escaping_on_var_before_switch_case_no_crash(int c) {
+ switch (c) {
+int i; // expected error{{Reached root without finding the declaration of
VD}}
+case 0: {
+ for (i = 0; i < 16; i++) {}
+ break;
+}
+ }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
huang-me wrote: I've added the test case to illustrate the goal of this change. https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,21 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ if (const CompoundStmt *CST = dyn_cast(SS->getBody())) {
+for (const Stmt *CB : CST->body()) {
+ if (const DeclStmt *DST = dyn_cast(CB)) {
+for (const Decl *D : DST->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
steakhal wrote:
This is duplicated from the previous check. Consider refactoring the code to
hoist and name the common functionality.
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/steakhal requested changes to this pull request. Thanks for working on this. I think iterating the direct child nodes of the switch is fine. I can't think of a better way. https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config
unroll-loops=true -verify %s
+
+void test_escaping_on_var_before_switch_case_no_crash(int c) {
+ switch (c) {
+int i; // expected error{{Reached root without finding the declaration of
VD}}
+case 0: {
+ for (i = 0; i < 16; i++) {}
+ break;
+}
+ }
+}
steakhal wrote:
It took me some time to realize that its not a verify expectation.
BTW in this shape, this test fails.
```suggestion
// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config
unroll-loops=true -verify %s
// expected-no-diagnostics
void test_escaping_on_var_before_switch_case_no_crash(int c) {
// https://github.com/llvm/llvm-project/issues/68819
switch (c) {
int i; // no-crash: The declaration of `i` is found here.
case 0: {
for (i = 0; i < 16; i++) {}
break;
}
}
}
```
I'd also suggest to rename the test file to highlight that this is only for
loop unrolling. Or maybe even better, just append this to the end of
`clang/test/Analysis/loop-unrolling.cpp`.
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,21 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ if (const CompoundStmt *CST = dyn_cast(SS->getBody())) {
steakhal wrote:
```suggestion
if (const auto *SS = dyn_cast(S)) {
if (const auto *CST = dyn_cast(SS->getBody())) {
```
The type is already mentioned on the line.
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/huang-me updated
https://github.com/llvm/llvm-project/pull/82089
>From 2802ef4b9ed88da3cacb16ab7738907ee806 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH 1/4] Fix crash on StaticAnalyzer loop unrolling
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..4001268bde6677 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
>From e9e195e4462da7f3ca2317096ddace6ce3e88d13 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Mon, 19 Feb 2024 18:17:27 +0800
Subject: [PATCH 2/4] Check if dynamic cast get pointer to valid elements
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index 4001268bde6677..093e9bbf4ce5e0 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -228,11 +228,15 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
}
if (const SwitchStmt *SS = dyn_cast(S)) {
- for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
-for (const Decl *D : dyn_cast(CB)->decls()) {
- // Once we reach the declaration of the VD we can return.
- if (D->getCanonicalDecl() == VD)
-return false;
+ if (const CompoundStmt *CST = dyn_cast(SS->getBody())) {
+for (const Stmt *CB : CST->body()) {
+ if (const DeclStmt *DST = dyn_cast(CB)) {
+for (const Decl *D : DST->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
}
}
}
>From 6ed9ea88865e91f1727077b1a3a24d7b110060fd Mon Sep 17 00:00:00 2001
From: huang-me
Date: Tue, 20 Feb 2024 11:31:23 +0800
Subject: [PATCH 3/4] Add testcase for finding declaration within SwitchStmt
---
.../test-escaping-on-var-before-switch-case.c | 11 +++
1 file changed, 11 insertions(+)
create mode 100644
clang/test/Analysis/test-escaping-on-var-before-switch-case.c
diff --git a/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
b/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
new file mode 100644
index 00..95aed8cab06b55
--- /dev/null
+++ b/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config
unroll-loops=true -verify %s
+
+void test_escaping_on_var_before_switch_case_no_crash(int c) {
+ switch (c) {
+int i; // expected error{{Reached root without finding the declaration of
VD}}
+case 0: {
+ for (i = 0; i < 16; i++) {}
+ break;
+}
+ }
+}
>From 294b7c960233cbef8ee0d8721c60792fd1e6a064 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Thu, 22 Feb 2024 21:04:06 +0800
Subject: [PATCH 4/4] Hoist duplicated code into function
---
.../lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 29 ++-
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index 093e9bbf4ce5e0..697e811470e708 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -190,6 +190,17 @@ static bool isCapturedByReference(ExplodedNode *N, const
DeclRefExpr *DR) {
return FD->getType()->isReferenceType();
}
+static bool isFoundInStmt(const Stmt *S, const VarDecl *VD) {
+ if (const DeclStmt *DS = dyn_cast(S)) {
+for (const Decl *D : DS->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return true;
+}
+ }
+ return false;
+}
+
// A loop counter is considered escaped if:
// case 1: It is a global variable.
// case 2: It is a reference parameter or a reference capture.
@@ -219,24 +230,16 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,21 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ if (const CompoundStmt *CST = dyn_cast(SS->getBody())) {
+for (const Stmt *CB : CST->body()) {
+ if (const DeclStmt *DST = dyn_cast(CB)) {
+for (const Decl *D : DST->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
huang-me wrote:
Sure, I can do that.
https://github.com/llvm/llvm-project/pull/82089
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/huang-me updated
https://github.com/llvm/llvm-project/pull/82089
>From 2802ef4b9ed88da3cacb16ab7738907ee806 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH 1/6] Fix crash on StaticAnalyzer loop unrolling
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index a80352816be613..4001268bde6677 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -226,6 +226,17 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
return false;
}
}
+
+if (const SwitchStmt *SS = dyn_cast(S)) {
+ for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
+for (const Decl *D : dyn_cast(CB)->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
+}
+
// Check the usage of the pass-by-ref function calls and adress-of operator
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
>From e9e195e4462da7f3ca2317096ddace6ce3e88d13 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Mon, 19 Feb 2024 18:17:27 +0800
Subject: [PATCH 2/6] Check if dynamic cast get pointer to valid elements
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index 4001268bde6677..093e9bbf4ce5e0 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -228,11 +228,15 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR) {
}
if (const SwitchStmt *SS = dyn_cast(S)) {
- for (const Stmt *CB : dyn_cast(SS->getBody())->body()) {
-for (const Decl *D : dyn_cast(CB)->decls()) {
- // Once we reach the declaration of the VD we can return.
- if (D->getCanonicalDecl() == VD)
-return false;
+ if (const CompoundStmt *CST = dyn_cast(SS->getBody())) {
+for (const Stmt *CB : CST->body()) {
+ if (const DeclStmt *DST = dyn_cast(CB)) {
+for (const Decl *D : DST->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return false;
+}
+ }
}
}
}
>From 6ed9ea88865e91f1727077b1a3a24d7b110060fd Mon Sep 17 00:00:00 2001
From: huang-me
Date: Tue, 20 Feb 2024 11:31:23 +0800
Subject: [PATCH 3/6] Add testcase for finding declaration within SwitchStmt
---
.../test-escaping-on-var-before-switch-case.c | 11 +++
1 file changed, 11 insertions(+)
create mode 100644
clang/test/Analysis/test-escaping-on-var-before-switch-case.c
diff --git a/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
b/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
new file mode 100644
index 00..95aed8cab06b55
--- /dev/null
+++ b/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config
unroll-loops=true -verify %s
+
+void test_escaping_on_var_before_switch_case_no_crash(int c) {
+ switch (c) {
+int i; // expected error{{Reached root without finding the declaration of
VD}}
+case 0: {
+ for (i = 0; i < 16; i++) {}
+ break;
+}
+ }
+}
>From 294b7c960233cbef8ee0d8721c60792fd1e6a064 Mon Sep 17 00:00:00 2001
From: huang-me
Date: Thu, 22 Feb 2024 21:04:06 +0800
Subject: [PATCH 4/6] Hoist duplicated code into function
---
.../lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 29 ++-
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index 093e9bbf4ce5e0..697e811470e708 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -190,6 +190,17 @@ static bool isCapturedByReference(ExplodedNode *N, const
DeclRefExpr *DR) {
return FD->getType()->isReferenceType();
}
+static bool isFoundInStmt(const Stmt *S, const VarDecl *VD) {
+ if (const DeclStmt *DS = dyn_cast(S)) {
+for (const Decl *D : DS->decls()) {
+ // Once we reach the declaration of the VD we can return.
+ if (D->getCanonicalDecl() == VD)
+return true;
+}
+ }
+ return false;
+}
+
// A loop counter is considered escaped if:
// case 1: It is a global variable.
// case 2: It is a reference parameter or a reference capture.
@@ -219,24 +230,16 @@ static bool isPossiblyEscaped(ExplodedNode *N, const
DeclRefExpr *DR
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/steakhal approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
