[clang] [clang][analyzer] Fix crash in loop unrolling (PR #82089)
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 13 23:54:01 PDT 2024
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/82089
>From 2802ef4b9ed88da3cac3333b16ab7738907ee806 Mon Sep 17 00:00:00 2001
From: huang-me <amos0107 at gmail.com>
Date: Sat, 17 Feb 2024 10:43:48 +0800
Subject: [PATCH 1/9] 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<SwitchStmt>(S)) {
+ for (const Stmt *CB : dyn_cast<CompoundStmt>(SS->getBody())->body()) {
+ for (const Decl *D : dyn_cast<DeclStmt>(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 <amos0107 at gmail.com>
Date: Mon, 19 Feb 2024 18:17:27 +0800
Subject: [PATCH 2/9] 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<SwitchStmt>(S)) {
- for (const Stmt *CB : dyn_cast<CompoundStmt>(SS->getBody())->body()) {
- for (const Decl *D : dyn_cast<DeclStmt>(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<CompoundStmt>(SS->getBody())) {
+ for (const Stmt *CB : CST->body()) {
+ if (const DeclStmt *DST = dyn_cast<DeclStmt>(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 <amos0107 at gmail.com>
Date: Tue, 20 Feb 2024 11:31:23 +0800
Subject: [PATCH 3/9] 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 00000000000000..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 <amos0107 at gmail.com>
Date: Thu, 22 Feb 2024 21:04:06 +0800
Subject: [PATCH 4/9] 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<DeclStmt>(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) {
continue;
}
- if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
- for (const Decl *D : DS->decls()) {
- // Once we reach the declaration of the VD we can return.
- if (D->getCanonicalDecl() == VD)
- return false;
- }
+ if (isFoundInStmt(S, VD)) {
+ return false;
}
+
if (const SwitchStmt *SS = dyn_cast<SwitchStmt>(S)) {
if (const CompoundStmt *CST = dyn_cast<CompoundStmt>(SS->getBody())) {
for (const Stmt *CB : CST->body()) {
- if (const DeclStmt *DST = dyn_cast<DeclStmt>(CB)) {
- for (const Decl *D : DST->decls()) {
- // Once we reach the declaration of the VD we can return.
- if (D->getCanonicalDecl() == VD)
- return false;
- }
- }
+ if (isFoundInStmt(CB, VD))
+ return false;
}
}
}
>From 126887716ffd9004fa9ee8955441d17c5e8a7767 Mon Sep 17 00:00:00 2001
From: huang-me <amos0107 at gmail.com>
Date: Thu, 22 Feb 2024 21:09:04 +0800
Subject: [PATCH 5/9] Move testing code to loop-unrolling
---
clang/test/Analysis/loop-unrolling.cpp | 14 ++++++++++++++
.../test-escaping-on-var-before-switch-case.c | 11 -----------
2 files changed, 14 insertions(+), 11 deletions(-)
delete mode 100644 clang/test/Analysis/test-escaping-on-var-before-switch-case.c
diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp
index fc1fb06cdc014e..eb6d47be993ba1 100644
--- a/clang/test/Analysis/loop-unrolling.cpp
+++ b/clang/test/Analysis/loop-unrolling.cpp
@@ -1,5 +1,7 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config unroll-loops=true -verify %s
+// expected-no-diagnostics
void clang_analyzer_numTimesReached();
void clang_analyzer_warnIfReached();
@@ -547,3 +549,15 @@ void capture_implicitly_by_ref_as_loop_counter() {
}
};
}
+
+
+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;
+ }
+ }
+}
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
deleted file mode 100644
index 95aed8cab06b55..00000000000000
--- a/clang/test/Analysis/test-escaping-on-var-before-switch-case.c
+++ /dev/null
@@ -1,11 +0,0 @@
-// 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 5113063f9165313827ec4307cc5e63a4364b48e9 Mon Sep 17 00:00:00 2001
From: huang-me <amos0107 at gmail.com>
Date: Thu, 22 Feb 2024 21:12:10 +0800
Subject: [PATCH 6/9] Remove explicitly type declaration
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index 697e811470e708..b85d92c08c0d5a 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -235,8 +235,8 @@ static bool isPossiblyEscaped(ExplodedNode *N, const DeclRefExpr *DR) {
}
- if (const SwitchStmt *SS = dyn_cast<SwitchStmt>(S)) {
- if (const CompoundStmt *CST = dyn_cast<CompoundStmt>(SS->getBody())) {
+ if (const auto *SS = dyn_cast<SwitchStmt>(S)) {
+ if (const auto *CST = dyn_cast<CompoundStmt>(SS->getBody())) {
for (const Stmt *CB : CST->body()) {
if (isFoundInStmt(CB, VD))
return false;
>From 5a053e1e6595976bc585ef5e1fac63bb6c7bc792 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Wed, 13 Mar 2024 21:35:47 +0100
Subject: [PATCH 7/9] clang-format
---
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index b85d92c08c0d5a..7042f1aeb803fc 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -234,7 +234,6 @@ static bool isPossiblyEscaped(ExplodedNode *N, const DeclRefExpr *DR) {
return false;
}
-
if (const auto *SS = dyn_cast<SwitchStmt>(S)) {
if (const auto *CST = dyn_cast<CompoundStmt>(SS->getBody())) {
for (const Stmt *CB : CST->body()) {
>From 04466deb2a06ef37bdbde0034fb1273725756923 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Wed, 13 Mar 2024 23:04:07 +0100
Subject: [PATCH 8/9] Expect diagnostics in test
---
clang/test/Analysis/loop-unrolling.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp
index eb6d47be993ba1..6a539f13db9f77 100644
--- a/clang/test/Analysis/loop-unrolling.cpp
+++ b/clang/test/Analysis/loop-unrolling.cpp
@@ -1,7 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config unroll-loops=true -verify %s
-// expected-no-diagnostics
void clang_analyzer_numTimesReached();
void clang_analyzer_warnIfReached();
>From d4f290e6a6f8a396e07a4e66b54dd2b1b0ecfacd Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Thu, 14 Mar 2024 07:53:50 +0100
Subject: [PATCH 9/9] Remove unneeded RUN line
---
clang/test/Analysis/loop-unrolling.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp
index 6a539f13db9f77..66a828abfb5133 100644
--- a/clang/test/Analysis/loop-unrolling.cpp
+++ b/clang/test/Analysis/loop-unrolling.cpp
@@ -1,6 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config unroll-loops=true -verify %s
void clang_analyzer_numTimesReached();
void clang_analyzer_warnIfReached();
More information about the cfe-commits
mailing list