From fc212fa8385aefb133a93fbb88a64a44f7d27aef Mon Sep 17 00:00:00 2001 From: Jack Styles 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(*nextIt)}) { - std::get>(x.t) = - std::move(*endDir); - nextIt = block.erase(nextIt); + if(nextIt != block.end()) { + if (auto *endDir{ + GetConstructIf(*nextIt)}) { + std::get>(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 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(*nextIt)}) { std::get>(x.t) =