-
Notifications
You must be signed in to change notification settings - Fork 13.9k
[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
[flang][OpenMP] Verify that arguments to COPYPRIVATE are variables #141823
Conversation
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
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-semantics Author: Krzysztof Parzyszek (kparzysz) ChangesThe 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 Full diff: https://github.com/llvm/llvm-project/pull/141823.diff 4 Files Affected:
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
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…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
…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
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