[PATCH] D30565: [analyzer] Terminate analysis on OpenMP code instead of crashing
dcoughlin added a comment. @zaks.anna: What do you think? Should we try to get this into clang 4.0? Repository: rL LLVM https://reviews.llvm.org/D30565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30565: [analyzer] Terminate analysis on OpenMP code instead of crashing
a.sidorin added a comment. Thank you Devin. Should we submit this to 4.0? I guess there are not many users of both CSA and OpenMP but PR you pointed is already the second report about this issue I see. Repository: rL LLVM https://reviews.llvm.org/D30565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30565: [analyzer] Terminate analysis on OpenMP code instead of crashing
This revision was automatically updated to reflect the committed changes.
Closed by commit rL296884: [Analyzer] Terminate analysis on OpenMP code instead
of assertion crash (authored by a.sidorin).
Changed prior to commit:
https://reviews.llvm.org/D30565?vs=90441&id=90499#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30565
Files:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/openmp-unsupported.c
Index: cfe/trunk/test/Analysis/openmp-unsupported.c
===
--- cfe/trunk/test/Analysis/openmp-unsupported.c
+++ cfe/trunk/test/Analysis/openmp-unsupported.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze
-analyzer-checker=core.builtin -fopenmp -verify %s
+// expected-no-diagnostics
+
+void openmp_parallel_crash_test() {
+#pragma omp parallel
+ ;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -777,7 +777,7 @@
assert(!isa(S) || S == cast(S)->IgnoreParens());
switch (S->getStmtClass()) {
-// C++ and ARC stuff we don't support yet.
+// C++, OpenMP and ARC stuff we don't support yet.
case Expr::ObjCIndirectCopyRestoreExprClass:
case Stmt::CXXDependentScopeMemberExprClass:
case Stmt::CXXInheritedCtorInitExprClass:
@@ -805,36 +805,7 @@
case Stmt::SEHTryStmtClass:
case Stmt::SEHExceptStmtClass:
case Stmt::SEHLeaveStmtClass:
-case Stmt::SEHFinallyStmtClass: {
- const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState());
- Engine.addAbortedBlock(node, currBldrCtx->getBlock());
- break;
-}
-
-case Stmt::ParenExprClass:
- llvm_unreachable("ParenExprs already handled.");
-case Stmt::GenericSelectionExprClass:
- llvm_unreachable("GenericSelectionExprs already handled.");
-// Cases that should never be evaluated simply because they shouldn't
-// appear in the CFG.
-case Stmt::BreakStmtClass:
-case Stmt::CaseStmtClass:
-case Stmt::CompoundStmtClass:
-case Stmt::ContinueStmtClass:
-case Stmt::CXXForRangeStmtClass:
-case Stmt::DefaultStmtClass:
-case Stmt::DoStmtClass:
-case Stmt::ForStmtClass:
-case Stmt::GotoStmtClass:
-case Stmt::IfStmtClass:
-case Stmt::IndirectGotoStmtClass:
-case Stmt::LabelStmtClass:
-case Stmt::NoStmtClass:
-case Stmt::NullStmtClass:
-case Stmt::SwitchStmtClass:
-case Stmt::WhileStmtClass:
-case Expr::MSDependentExistsStmtClass:
-case Stmt::CapturedStmtClass:
+case Stmt::SEHFinallyStmtClass:
case Stmt::OMPParallelDirectiveClass:
case Stmt::OMPSimdDirectiveClass:
case Stmt::OMPForDirectiveClass:
@@ -882,6 +853,36 @@
case Stmt::OMPTargetTeamsDistributeParallelForDirectiveClass:
case Stmt::OMPTargetTeamsDistributeParallelForSimdDirectiveClass:
case Stmt::OMPTargetTeamsDistributeSimdDirectiveClass:
+case Stmt::CapturedStmtClass:
+{
+ const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState());
+ Engine.addAbortedBlock(node, currBldrCtx->getBlock());
+ break;
+}
+
+case Stmt::ParenExprClass:
+ llvm_unreachable("ParenExprs already handled.");
+case Stmt::GenericSelectionExprClass:
+ llvm_unreachable("GenericSelectionExprs already handled.");
+// Cases that should never be evaluated simply because they shouldn't
+// appear in the CFG.
+case Stmt::BreakStmtClass:
+case Stmt::CaseStmtClass:
+case Stmt::CompoundStmtClass:
+case Stmt::ContinueStmtClass:
+case Stmt::CXXForRangeStmtClass:
+case Stmt::DefaultStmtClass:
+case Stmt::DoStmtClass:
+case Stmt::ForStmtClass:
+case Stmt::GotoStmtClass:
+case Stmt::IfStmtClass:
+case Stmt::IndirectGotoStmtClass:
+case Stmt::LabelStmtClass:
+case Stmt::NoStmtClass:
+case Stmt::NullStmtClass:
+case Stmt::SwitchStmtClass:
+case Stmt::WhileStmtClass:
+case Expr::MSDependentExistsStmtClass:
llvm_unreachable("Stmt should not be in analyzer evaluation loop");
case Stmt::ObjCSubscriptRefExprClass:
Index: cfe/trunk/test/Analysis/openmp-unsupported.c
===
--- cfe/trunk/test/Analysis/openmp-unsupported.c
+++ cfe/trunk/test/Analysis/openmp-unsupported.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin -fopenmp -verify %s
+// expected-no-diagnostics
+
+void openmp_parallel_crash_test() {
+#pragma omp parallel
+ ;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -777,7 +777,7 @@
assert(!isa(S) || S == cast(S)->Ignore
[PATCH] D30565: [analyzer] Terminate analysis on OpenMP code instead of crashing
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. Thanks for fixing this! It looks like this is tracked by PR31835. https://bugs.llvm.org//show_bug.cgi?id=31835 https://reviews.llvm.org/D30565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30565: [analyzer] Terminate analysis on OpenMP code instead of crashing
a.sidorin added a comment. `git blame` shows that OMP* statements were added to the switch block by different authors while OpenMP support in clang was implemented. Looks like they were put to "Should not appear" section instead of "Unsupported" by mistake. https://reviews.llvm.org/D30565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30565: [analyzer] Terminate analysis on OpenMP code instead of crashing
a.sidorin created this revision.
ExprEngine assumes that OpenMP statements should never appear in CFG. However,
current CFG doesn't know anything about OpenMP and passes such statements as
CFG nodes causing "UNREACHABLE executed!" crashes. Since I have no ideas on
OpenMP implementation in ExprEngine or CFG, I'm changing the behaviour to stop
the analysis on OpenMP statements to avoid crashes.
https://reviews.llvm.org/D30565
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/openmp-unsupported.c
Index: test/Analysis/openmp-unsupported.c
===
--- test/Analysis/openmp-unsupported.c
+++ test/Analysis/openmp-unsupported.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze
-analyzer-checker=core.builtin -fopenmp -verify %s
+// expected-no-diagnostics
+
+void openmp_parallel_crash_test() {
+#pragma omp parallel
+ ;
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -769,7 +769,7 @@
assert(!isa(S) || S == cast(S)->IgnoreParens());
switch (S->getStmtClass()) {
-// C++ and ARC stuff we don't support yet.
+// C++, OpenMP and ARC stuff we don't support yet.
case Expr::ObjCIndirectCopyRestoreExprClass:
case Stmt::CXXDependentScopeMemberExprClass:
case Stmt::CXXInheritedCtorInitExprClass:
@@ -797,36 +797,7 @@
case Stmt::SEHTryStmtClass:
case Stmt::SEHExceptStmtClass:
case Stmt::SEHLeaveStmtClass:
-case Stmt::SEHFinallyStmtClass: {
- const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState());
- Engine.addAbortedBlock(node, currBldrCtx->getBlock());
- break;
-}
-
-case Stmt::ParenExprClass:
- llvm_unreachable("ParenExprs already handled.");
-case Stmt::GenericSelectionExprClass:
- llvm_unreachable("GenericSelectionExprs already handled.");
-// Cases that should never be evaluated simply because they shouldn't
-// appear in the CFG.
-case Stmt::BreakStmtClass:
-case Stmt::CaseStmtClass:
-case Stmt::CompoundStmtClass:
-case Stmt::ContinueStmtClass:
-case Stmt::CXXForRangeStmtClass:
-case Stmt::DefaultStmtClass:
-case Stmt::DoStmtClass:
-case Stmt::ForStmtClass:
-case Stmt::GotoStmtClass:
-case Stmt::IfStmtClass:
-case Stmt::IndirectGotoStmtClass:
-case Stmt::LabelStmtClass:
-case Stmt::NoStmtClass:
-case Stmt::NullStmtClass:
-case Stmt::SwitchStmtClass:
-case Stmt::WhileStmtClass:
-case Expr::MSDependentExistsStmtClass:
-case Stmt::CapturedStmtClass:
+case Stmt::SEHFinallyStmtClass:
case Stmt::OMPParallelDirectiveClass:
case Stmt::OMPSimdDirectiveClass:
case Stmt::OMPForDirectiveClass:
@@ -874,6 +845,36 @@
case Stmt::OMPTargetTeamsDistributeParallelForDirectiveClass:
case Stmt::OMPTargetTeamsDistributeParallelForSimdDirectiveClass:
case Stmt::OMPTargetTeamsDistributeSimdDirectiveClass:
+case Stmt::CapturedStmtClass:
+{
+ const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState());
+ Engine.addAbortedBlock(node, currBldrCtx->getBlock());
+ break;
+}
+
+case Stmt::ParenExprClass:
+ llvm_unreachable("ParenExprs already handled.");
+case Stmt::GenericSelectionExprClass:
+ llvm_unreachable("GenericSelectionExprs already handled.");
+// Cases that should never be evaluated simply because they shouldn't
+// appear in the CFG.
+case Stmt::BreakStmtClass:
+case Stmt::CaseStmtClass:
+case Stmt::CompoundStmtClass:
+case Stmt::ContinueStmtClass:
+case Stmt::CXXForRangeStmtClass:
+case Stmt::DefaultStmtClass:
+case Stmt::DoStmtClass:
+case Stmt::ForStmtClass:
+case Stmt::GotoStmtClass:
+case Stmt::IfStmtClass:
+case Stmt::IndirectGotoStmtClass:
+case Stmt::LabelStmtClass:
+case Stmt::NoStmtClass:
+case Stmt::NullStmtClass:
+case Stmt::SwitchStmtClass:
+case Stmt::WhileStmtClass:
+case Expr::MSDependentExistsStmtClass:
llvm_unreachable("Stmt should not be in analyzer evaluation loop");
case Stmt::ObjCSubscriptRefExprClass:
Index: test/Analysis/openmp-unsupported.c
===
--- test/Analysis/openmp-unsupported.c
+++ test/Analysis/openmp-unsupported.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin -fopenmp -verify %s
+// expected-no-diagnostics
+
+void openmp_parallel_crash_test() {
+#pragma omp parallel
+ ;
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -769,7 +769,7 @@
assert(!isa(S) || S == cast(S)->IgnoreParens());
switch (S->getStmtClass())
