[flang-commits] [flang] [flang][OpenMP] Verify that arguments to COPYPRIVATE are variables (PR #141823)

[Krzysztof Parzyszek via flang-commits]

https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/141823

>From 7103b042de5e0bf6212a0a13b1a76e66bf633b67 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 28 May 2025 13:49:56 -0500
Subject: [PATCH 1/2] [flang][OpenMP] Verify that arguments to COPYPRIVATE are
 variables

The check if the arguments are variable list items was missing, leading
to a crash in lowering in some invalid situations.

This fixes the first testcase reported in
https://github.com/llvm/llvm-project/issues/141481
---
 flang/lib/Semantics/check-omp-structure.cpp   | 25 ++++++++++---------
 flang/lib/Semantics/check-omp-structure.h     |  1 +
 flang/test/Semantics/OpenMP/copyprivate04.f90 |  1 +
 flang/test/Semantics/OpenMP/copyprivate05.f90 | 12 +++++++++
 4 files changed, 27 insertions(+), 12 deletions(-)
 create mode 100644 flang/test/Semantics/OpenMP/copyprivate05.f90

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index bda0d62829506..297cd32270705 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -390,6 +390,16 @@ std::optional<bool> OmpStructureChecker::IsContiguous(
       object.u);
 }
 
+void OmpStructureChecker::CheckVariableListItem(
+    const SymbolSourceMap &symbols) {
+  for (auto &[symbol, source] : symbols) {
+    if (!IsVariableListItem(*symbol)) {
+      context_.SayWithDecl(*symbol, source, "'%s' must be a variable"_err_en_US,
+                           symbol->name());
+    }
+  }
+}
+
 void OmpStructureChecker::CheckMultipleOccurrence(
     semantics::UnorderedSymbolSet &listVars,
     const std::list<parser::Name> &nameList, const parser::CharBlock &item,
@@ -4587,6 +4597,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Copyprivate &x) {
   CheckAllowedClause(llvm::omp::Clause::OMPC_copyprivate);
   SymbolSourceMap symbols;
   GetSymbolsInObjectList(x.v, symbols);
+  CheckVariableListItem(symbols);
   CheckIntentInPointer(symbols, llvm::omp::Clause::OMPC_copyprivate);
   CheckCopyingPolymorphicAllocatable(
       symbols, llvm::omp::Clause::OMPC_copyprivate);
@@ -4859,12 +4870,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::From &x) {
   const auto &objList{std::get<parser::OmpObjectList>(x.v.t)};
   SymbolSourceMap symbols;
   GetSymbolsInObjectList(objList, symbols);
-  for (const auto &[symbol, source] : symbols) {
-    if (!IsVariableListItem(*symbol)) {
-      context_.SayWithDecl(
-          *symbol, source, "'%s' must be a variable"_err_en_US, symbol->name());
-    }
-  }
+  CheckVariableListItem(symbols);
 
   // Ref: [4.5:109:19]
   // If a list item is an array section it must specify contiguous storage.
@@ -4904,12 +4910,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::To &x) {
   const auto &objList{std::get<parser::OmpObjectList>(x.v.t)};
   SymbolSourceMap symbols;
   GetSymbolsInObjectList(objList, symbols);
-  for (const auto &[symbol, source] : symbols) {
-    if (!IsVariableListItem(*symbol)) {
-      context_.SayWithDecl(
-          *symbol, source, "'%s' must be a variable"_err_en_US, symbol->name());
-    }
-  }
+  CheckVariableListItem(symbols);
 
   // Ref: [4.5:109:19]
   // If a list item is an array section it must specify contiguous storage.
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 587959f7d506f..1a8059d8548ed 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -174,6 +174,7 @@ class OmpStructureChecker
   bool IsExtendedListItem(const Symbol &sym);
   bool IsCommonBlock(const Symbol &sym);
   std::optional<bool> IsContiguous(const parser::OmpObject &object);
+  void CheckVariableListItem(const SymbolSourceMap &symbols);
   void CheckMultipleOccurrence(semantics::UnorderedSymbolSet &listVars,
       const std::list<parser::Name> &nameList, const parser::CharBlock &item,
       const std::string &clauseName);
diff --git a/flang/test/Semantics/OpenMP/copyprivate04.f90 b/flang/test/Semantics/OpenMP/copyprivate04.f90
index 291cf1103fb27..8d7800229bc5f 100644
--- a/flang/test/Semantics/OpenMP/copyprivate04.f90
+++ b/flang/test/Semantics/OpenMP/copyprivate04.f90
@@ -70,6 +70,7 @@ program omp_copyprivate
   ! Named constants are shared.
   !$omp single
   !ERROR: COPYPRIVATE variable 'pi' is not PRIVATE or THREADPRIVATE in outer context
+  !ERROR: 'pi' must be a variable
   !$omp end single copyprivate(pi)
 
   !$omp parallel do
diff --git a/flang/test/Semantics/OpenMP/copyprivate05.f90 b/flang/test/Semantics/OpenMP/copyprivate05.f90
new file mode 100644
index 0000000000000..129f8f0b5144e
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/copyprivate05.f90
@@ -0,0 +1,12 @@
+!RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+
+! The first testcase from https://github.com/llvm/llvm-project/issues/141481
+
+subroutine f00
+  type t
+  end type
+
+!ERROR: 't' must be a variable
+!$omp single copyprivate(t)
+!$omp end single
+end

>From e52b6bf45685fc806b9cab2fe0f3243d0f9467ab Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 28 May 2025 14:02:30 -0500
Subject: [PATCH 2/2] format

---
 flang/lib/Semantics/check-omp-structure.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 297cd32270705..b0bc478d96a1e 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -394,8 +394,8 @@ void OmpStructureChecker::CheckVariableListItem(
     const SymbolSourceMap &symbols) {
   for (auto &[symbol, source] : symbols) {
     if (!IsVariableListItem(*symbol)) {
-      context_.SayWithDecl(*symbol, source, "'%s' must be a variable"_err_en_US,
-                           symbol->name());
+      context_.SayWithDecl(
+          *symbol, source, "'%s' must be a variable"_err_en_US, symbol->name());
     }
   }
 }