Skip to content

[flang][OpenMP] Verify that arguments to COPYPRIVATE are variables #141823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 29, 2025

Conversation

kparzysz
Copy link
Contributor

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
#141481

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
llvm#141481
@kparzysz kparzysz requested review from ergawy and luporl May 28, 2025 18:53
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels May 28, 2025
@llvmbot
Copy link
Member

llvmbot commented May 28, 2025

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

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
#141481


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

4 Files Affected:

  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+13-12)
  • (modified) flang/lib/Semantics/check-omp-structure.h (+1)
  • (modified) flang/test/Semantics/OpenMP/copyprivate04.f90 (+1)
  • (added) flang/test/Semantics/OpenMP/copyprivate05.f90 (+12)
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

Copy link

github-actions bot commented May 28, 2025

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

Copy link
Contributor

@luporl luporl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kparzysz kparzysz merged commit 798ae82 into llvm:main May 29, 2025
11 checks passed
@kparzysz kparzysz deleted the users/kparzysz/copyprivate-variable branch May 29, 2025 20:05
svkeerthy pushed a commit that referenced this pull request May 29, 2025
…141823)

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
#141481
google-yfyang pushed a commit to google-yfyang/llvm-project that referenced this pull request May 29, 2025
…lvm#141823)

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
llvm#141481
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…lvm#141823)

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
llvm#141481
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:openmp flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants