[Flang][OpenMP] Fix crash when block.end() is missed #147519

Stylie777 · 2025-07-08T13:20:30Z

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

As reported in llvm#145917 and llvm#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 llvm#147309

llvmbot · 2025-07-08T13:21:00Z

@llvm/pr-subscribers-flang-semantics
@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-openmp

Author: Jack Styles (Stylie777)

Changes

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

Full diff: https://github.com/llvm/llvm-project/pull/147519.diff

2 Files Affected:

(modified) flang/lib/Semantics/canonicalize-omp.cpp (+7-5)
(added) flang/test/Parser/OpenMP/loop-transformation-construct03.f90 (+75)

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

github-actions · 2025-07-08T13:22:57Z

✅ With the latest revision this PR passed the C/C++ code formatter.

mjklemm

LGTM

DavidSpickett

LGTM, I've tested an equivalent patch on Windows On Arm and it fixed the issue.

flang/test/Parser/OpenMP/loop-transformation-construct03.f90

kparzysz · 2025-07-08T17:28:52Z

I'm going to merge this, it fixes some failures we're seeing.

llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics flang:parser labels Jul 8, 2025

Stylie777 mentioned this pull request Jul 8, 2025

[Flang][OpenMP] Add Semantics support for Nested OpenMPLoopConstructs #145917

Merged

fix formatting

12a5cf1

Stylie777 requested review from kparzysz, DavidSpickett and ronlieb July 8, 2025 13:23

mjklemm approved these changes Jul 8, 2025

View reviewed changes

DavidSpickett approved these changes Jul 8, 2025

View reviewed changes

flang/test/Parser/OpenMP/loop-transformation-construct03.f90 Show resolved Hide resolved

kparzysz approved these changes Jul 8, 2025

View reviewed changes

kparzysz merged commit 9a8d45f into llvm:main Jul 8, 2025
9 checks passed

luporl mentioned this pull request Jul 9, 2025

[flang][OpenMP] Create unique reduction decls for different logical kinds #146558

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Flang][OpenMP] Fix crash when block.end() is missed #147519

[Flang][OpenMP] Fix crash when block.end() is missed #147519

Uh oh!

Stylie777 commented Jul 8, 2025

Uh oh!

llvmbot commented Jul 8, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jul 8, 2025 •

edited

Loading

Uh oh!

mjklemm left a comment

Uh oh!

DavidSpickett left a comment

Uh oh!

Uh oh!

kparzysz commented Jul 8, 2025

Uh oh!

Uh oh!

Uh oh!

[Flang][OpenMP] Fix crash when block.end() is missed #147519

[Flang][OpenMP] Fix crash when block.end() is missed #147519

Uh oh!

Conversation

Stylie777 commented Jul 8, 2025

Uh oh!

llvmbot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjklemm left a comment

Choose a reason for hiding this comment

Uh oh!

DavidSpickett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kparzysz commented Jul 8, 2025

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jul 8, 2025 •

edited

Loading

github-actions bot commented Jul 8, 2025 •

edited

Loading