[flang-commits] [flang] [Flang][OpenMP] Fix crash when block.end() is missed (PR #147519)
Jack Styles via flang-commits
flang-commits at lists.llvm.org
Tue Jul 8 06:23:22 PDT 2025
https://github.com/Stylie777 updated https://github.com/llvm/llvm-project/pull/147519
>From fc212fa8385aefb133a93fbb88a64a44f7d27aef Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.styles at arm.com>
Date: Tue, 8 Jul 2025 14:10:04 +0100
Subject: [PATCH 1/2] [Flang][OpenMP] Fix crash when block.end() is missed
As reported in #145917 and #147309, there are situation's where
flang may crash. This is because `nextIt` in
`RewriteOpenMPLoopConstruct` gets re-assigned when an iterator is
erased from the block. If this is missed, Flang may attempt to access
a location in memory that is not accessable and cause a compiler crash.
This adds protection where the crash can occur, and a test with a
reproducer that can trigger the crash.
Fixes #147309
---
flang/lib/Semantics/canonicalize-omp.cpp | 12 +--
.../loop-transformation-construct03.f90 | 75 +++++++++++++++++++
2 files changed, 82 insertions(+), 5 deletions(-)
create mode 100644 flang/test/Parser/OpenMP/loop-transformation-construct03.f90
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index 1edcb376596b0..f6e21babc1186 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -151,11 +151,13 @@ class CanonicalizationOfOmp {
std::move(*doCons);
nextIt = block.erase(nextIt);
// try to match OmpEndLoopDirective
- if (auto *endDir{
- GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
- std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
- std::move(*endDir);
- nextIt = block.erase(nextIt);
+ if(nextIt != block.end()) {
+ if (auto *endDir{
+ GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
+ std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
+ std::move(*endDir);
+ nextIt = block.erase(nextIt);
+ }
}
} else {
messages_.Say(dir.source,
diff --git a/flang/test/Parser/OpenMP/loop-transformation-construct03.f90 b/flang/test/Parser/OpenMP/loop-transformation-construct03.f90
new file mode 100644
index 0000000000000..81f0de1b76263
--- /dev/null
+++ b/flang/test/Parser/OpenMP/loop-transformation-construct03.f90
@@ -0,0 +1,75 @@
+! This reproducer hit an issue where when finding directive's, and end directive's would iterate over the block.end()
+! so Flang would crash. We should be able to parse this subroutine without flang crashing.
+! Reported in https://github.com/llvm/llvm-project/issues/147309 and https://github.com/llvm/llvm-project/pull/145917#issuecomment-3041570824
+
+!RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-PARSE
+!RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-UNPARSE
+
+subroutine loop_transformation_construct7
+implicit none
+real(kind=8), dimension(1:10, 2) :: a
+integer :: b,c
+
+!$omp target teams distribute parallel do collapse(2) private(b)
+do b = 1, 10
+ do c = 1, 10
+ a(b, 2) = a(c, 1)
+ end do
+end do
+end subroutine
+
+!CHECK-PARSE: | ExecutionPart -> Block
+!CHECK-PARSE-NEXT: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPLoopConstruct
+!CHECK-PARSE-NEXT: | | | OmpBeginLoopDirective
+!CHECK-PARSE-NEXT: | | | | OmpLoopDirective -> llvm::omp::Directive = target teams distribute parallel do
+!CHECK-PARSE-NEXT: | | | | OmpClauseList -> OmpClause -> Collapse -> Scalar -> Integer -> Constant -> Expr = '2_4'
+!CHECK-PARSE-NEXT: | | | | | LiteralConstant -> IntLiteralConstant = '2'
+!CHECK-PARSE-NEXT: | | | | OmpClause -> Private -> OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'b'
+!CHECK-PARSE-NEXT: | | | DoConstruct
+!CHECK-PARSE-NEXT: | | | | NonLabelDoStmt
+!CHECK-PARSE-NEXT: | | | | | LoopControl -> LoopBounds
+!CHECK-PARSE-NEXT: | | | | | | Scalar -> Name = 'b'
+!CHECK-PARSE-NEXT: | | | | | | Scalar -> Expr = '1_4'
+!CHECK-PARSE-NEXT: | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!CHECK-PARSE-NEXT: | | | | | | Scalar -> Expr = '10_4'
+!CHECK-PARSE-NEXT: | | | | | | | LiteralConstant -> IntLiteralConstant = '10'
+!CHECK-PARSE-NEXT: | | | | Block
+!CHECK-PARSE-NEXT: | | | | | ExecutionPartConstruct -> ExecutableConstruct -> DoConstruct
+!CHECK-PARSE-NEXT: | | | | | | NonLabelDoStmt
+!CHECK-PARSE-NEXT: | | | | | | | LoopControl -> LoopBounds
+!CHECK-PARSE-NEXT: | | | | | | | | Scalar -> Name = 'c'
+!CHECK-PARSE-NEXT: | | | | | | | | Scalar -> Expr = '1_4'
+!CHECK-PARSE-NEXT: | | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!CHECK-PARSE-NEXT: | | | | | | | | Scalar -> Expr = '10_4'
+!CHECK-PARSE-NEXT: | | | | | | | | | LiteralConstant -> IntLiteralConstant = '10'
+!CHECK-PARSE-NEXT: | | | | | | Block
+!CHECK-PARSE-NEXT: | | | | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'a(int(b,kind=8),2_8)=a(int(c,kind=8),1_8)'
+!CHECK-PARSE-NEXT: | | | | | | | | Variable = 'a(int(b,kind=8),2_8)'
+!CHECK-PARSE-NEXT: | | | | | | | | | Designator -> DataRef -> ArrayElement
+!CHECK-PARSE-NEXT: | | | | | | | | | | DataRef -> Name = 'a'
+!CHECK-PARSE-NEXT: | | | | | | | | | | SectionSubscript -> Integer -> Expr = 'b'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | Designator -> DataRef -> Name = 'b'
+!CHECK-PARSE-NEXT: | | | | | | | | | | SectionSubscript -> Integer -> Expr = '2_4'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | LiteralConstant -> IntLiteralConstant = '2'
+!CHECK-PARSE-NEXT: | | | | | | | | Expr = 'a(int(c,kind=8),1_8)'
+!CHECK-PARSE-NEXT: | | | | | | | | | Designator -> DataRef -> ArrayElement
+!CHECK-PARSE-NEXT: | | | | | | | | | | DataRef -> Name = 'a'
+!CHECK-PARSE-NEXT: | | | | | | | | | | SectionSubscript -> Integer -> Expr = 'c'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | Designator -> DataRef -> Name = 'c'
+!CHECK-PARSE-NEXT: | | | | | | | | | | SectionSubscript -> Integer -> Expr = '1_4'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!CHECK-PARSE-NEXT: | | | | | | EndDoStmt ->
+!CHECK-PARSE-NEXT: | | | | EndDoStmt ->
+!CHECK-PARSE-NEXT: | EndSubroutineStmt ->
+
+!CHECK-UNPARSE: SUBROUTINE loop_transformation_construct7
+!CHECK-UNPARSE-NEXT: IMPLICIT NONE
+!CHECK-UNPARSE-NEXT: REAL(KIND=8_4), DIMENSION(1_4:10_4,2_4) :: a
+!CHECK-UNPARSE-NEXT: INTEGER b, c
+!CHECK-UNPARSE-NEXT: !$OMP TARGET TEAMS DISTRIBUTE PARALLEL DO COLLAPSE(2_4) PRIVATE(b)
+!CHECK-UNPARSE-NEXT: DO b=1_4,10_4
+!CHECK-UNPARSE-NEXT: DO c=1_4,10_4
+!CHECK-UNPARSE-NEXT: a(int(b,kind=8),2_8)=a(int(c,kind=8),1_8)
+!CHECK-UNPARSE-NEXT: END DO
+!CHECK-UNPARSE-NEXT: END DO
+!CHECK-UNPARSE-NEXT: END SUBROUTINE
>From 12a5cf1bc0ce4c0aa685c14ba4dead0eb99e9909 Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.styles at arm.com>
Date: Tue, 8 Jul 2025 14:22:24 +0100
Subject: [PATCH 2/2] fix formatting
---
flang/lib/Semantics/canonicalize-omp.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index f6e21babc1186..cf05d8463277f 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -151,7 +151,7 @@ class CanonicalizationOfOmp {
std::move(*doCons);
nextIt = block.erase(nextIt);
// try to match OmpEndLoopDirective
- if(nextIt != block.end()) {
+ if (nextIt != block.end()) {
if (auto *endDir{
GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
More information about the flang-commits
mailing list