[flang-commits] [flang] [flang][OpenMP] Validate substrings (PR #234661)
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Fri Oct 23 03:10:02 PDT 2026
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/234661
>From 72bfe76c8eeddc2d53d91801edc23a13c5b38f4d Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 22 Oct 2026 14:06:33 +0100
Subject: [PATCH 1/2] [flang][OpenMP] Validate substrings and retain useful
diagnostics
IsSubstring discarded expression-analysis diagnostics while leaving symbols
marked erroneous. Those erroneous symbols would cause an assertion failure
if visited again.
Retain those diagnostics and improve diagnostics with more useful errors
that distinguish correctly between substrings and arrays.
Assisted-by: Codex
---
flang/include/flang/Semantics/openmp-utils.h | 4 +-
flang/lib/Semantics/check-omp-structure.cpp | 48 +++++++++++++-
flang/lib/Semantics/check-omp-structure.h | 1 +
flang/lib/Semantics/expression.cpp | 18 ++++++
flang/lib/Semantics/openmp-utils.cpp | 11 +++-
.../Semantics/OpenMP/substring-bounds.f90 | 53 +++++++++++++++
.../Semantics/OpenMP/substring-invalid.f90 | 64 +++++++++++++++++++
flang/test/Semantics/substring-stride.f90 | 27 ++++++++
8 files changed, 219 insertions(+), 7 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/substring-bounds.f90
create mode 100644 flang/test/Semantics/OpenMP/substring-invalid.f90
create mode 100644 flang/test/Semantics/substring-stride.f90
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index a20af4f546d7bf..a3441892348815 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -115,7 +115,9 @@ bool IsLocatorListItem(
bool IsVariableListItem(
const parser::OmpObject &object, SemanticsContext *semaCtx);
-bool IsSubstring(const parser::OmpObject &object, SemanticsContext *semaCtx);
+// A missing result indicates an invalid designator, already diagnosed.
+std::optional<bool> IsSubstring(
+ const parser::OmpObject &object, SemanticsContext *semaCtx);
bool IsArrayElement(const parser::OmpObject &object, SemanticsContext *semaCtx);
const Symbol *GetHostSymbol(const Symbol &sym);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 81f0f146c3f2a9..a631053c40abee 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -653,6 +653,38 @@ void OmpStructureChecker::AnalyzeObject(const parser::OmpObject &object) {
object.u);
}
+void OmpStructureChecker::AnalyzeSubstring(const parser::OmpObject &object) {
+ const auto *designator{GetDesignatorFromObj(object)};
+ if (!designator) {
+ return;
+ }
+ if (const auto *array{GetArrayElementFromObj(object)}) {
+ // Scalar character substrings can be parsed as array elements, including
+ // scalar components of array-valued parents.
+ const auto *symbol{GetObjectSymbol(object, /*ultimate=*/true)};
+ if (!symbol || symbol->Rank() != 0 || IsProcedure(*symbol) ||
+ IsStmtFunction(*symbol) || !symbol->GetType() ||
+ symbol->GetType()->category() != DeclTypeSpec::Category::Character) {
+ return;
+ }
+ // CheckArraySection already diagnoses substring strides. Do not emit
+ // another diagnostic for the same error.
+ if (llvm::any_of(array->Subscripts(), [](const auto &subscript) {
+ const auto *triplet{
+ std::get_if<parser::SubscriptTriplet>(&subscript.u)};
+ return triplet && std::get<2>(triplet->t).has_value();
+ })) {
+ return;
+ }
+ } else if (!std::holds_alternative<parser::Substring>(designator->u)) {
+ return;
+ }
+ // Keep diagnostics, and only rewrite the parse tree after clause-specific
+ // checks that distinguish ArrayElement from Substring have completed.
+ evaluate::ExpressionAnalyzer ea{context_};
+ ea.Analyze(*designator);
+}
+
const parser::OpenMPConstruct *
OmpStructureChecker::GetCurrentConstruct() const {
for (const LoopOrConstruct &c : llvm::reverse(constructStack_)) {
@@ -4564,7 +4596,7 @@ void OmpStructureChecker::CheckSubstring(
const parser::OmpObjectList &objects, llvm::omp::Clause clauseId) {
llvm::omp::Version version{context_.langOptions().getOpenMPVersion()};
for (const parser::OmpObject &object : objects.v) {
- if (IsSubstring(object, &context_)) {
+ if (IsSubstring(object, &context_).value_or(false)) {
auto source{GetObjectSource(object)};
context_.Say(source ? *source : GetContext().clauseSource,
"A substring cannot appear in a %s clause"_err_en_US,
@@ -4674,7 +4706,11 @@ void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar(
void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar(
const parser::CharBlock &source, const parser::OmpObject &object,
llvm::StringRef clause, bool checkSubstrings) {
- if (!checkSubstrings && IsSubstring(object, &context_)) {
+ auto isSubstring{IsSubstring(object, &context_)};
+ if (!isSubstring) {
+ return; // Expression analysis has already diagnosed the invalid item.
+ }
+ if (!checkSubstrings && *isSubstring) {
// Clause-specific substring checks are needed before OpenMP 5.2 only;
// CheckArgumentObjectKind diagnoses them generically in newer versions.
return;
@@ -4686,7 +4722,7 @@ void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar(
llvm::StringRef kind{};
if (IsStructureComponent(*symbol)) {
kind = "A structure component";
- } else if (IsSubstring(object, &context_)) {
+ } else if (*isSubstring) {
kind = "A substring";
} else if (IsArrayElement(object, &context_)) {
kind = "An array element";
@@ -5355,6 +5391,9 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) {
}
}
}
+ if (!isDepobjType) {
+ AnalyzeSubstring(object);
+ }
}
auto &modifiers{OmpGetModifiers(*taskDep)};
if (auto *iter{OmpGetUniqueModifier<parser::OmpIterator>(modifiers)}) {
@@ -6913,6 +6952,9 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Affinity &x) {
// last part-ref of the list item must have a section subscript list.
CheckLastPartRefForArraySection(
*designator, llvm::omp::Clause::OMPC_affinity);
+ if (!std::holds_alternative<parser::Substring>(designator->u)) {
+ AnalyzeSubstring(object);
+ }
}
}
}
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 384b6228b5bc47..9e9880172b2439 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -396,6 +396,7 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
llvm::iterator_range<ClauseIterator> beginClauses,
llvm::iterator_range<ClauseIterator> endClauses);
void AnalyzeObject(const parser::OmpObject &object);
+ void AnalyzeSubstring(const parser::OmpObject &object);
std::pair<const parser::OmpClause *, const parser::OmpClause *>
FindMutuallyExclusiveClauses(llvm::omp::Clauses exclusive,
const std::vector<const parser::OmpClause *> &clauses);
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index ec7a6646fd9939..a1099c6d5f25d9 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -1472,6 +1472,24 @@ std::vector<Subscript> ExpressionAnalyzer::AnalyzeSectionSubscripts(
}
MaybeExpr ExpressionAnalyzer::Analyze(const parser::ArrayElement &ae) {
+ // A scalar CHARACTER part with a triplet denotes a substring, even when
+ // its parent is array-valued. Substrings cannot specify a stride.
+ if (const Symbol * symbol{parser::GetLastName(ae.Base()).symbol}) {
+ const Symbol &ultimate{symbol->GetUltimate()};
+ const auto *type{ultimate.GetType()};
+ if (ultimate.Rank() == 0 && type &&
+ type->category() == semantics::DeclTypeSpec::Character &&
+ !semantics::IsProcedure(ultimate) && ae.Subscripts().size() == 1) {
+ if (const auto *triplet{std::get_if<parser::SubscriptTriplet>(
+ &ae.Subscripts().front().u)}) {
+ if (const auto &stride{std::get<2>(triplet->t)}) {
+ Say(stride->thing.thing.value().source,
+ "A substring cannot have a stride"_err_en_US);
+ return std::nullopt;
+ }
+ }
+ }
+ }
MaybeExpr baseExpr;
{
auto restorer{AllowWholeAssumedSizeArray()};
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 4004c1897f56f3..3f7e2153792f19 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -327,13 +327,18 @@ bool IsVariableListItem(
return false;
}
-bool IsSubstring(const parser::OmpObject &object, SemanticsContext *semaCtx) {
- if (auto *desg{GetDesignatorFromObj(object)}) {
+std::optional<bool> IsSubstring(
+ const parser::OmpObject &object, SemanticsContext *semaCtx) {
+ // Whole objects and other designators cannot be substrings. Analyzing them
+ // as expressions can reject otherwise valid OpenMP list items.
+ if (auto *desg{GetDesignatorFromObj(object)}; desg &&
+ (std::holds_alternative<parser::Substring>(desg->u) ||
+ GetArrayElementFromObj(object))) {
evaluate::ExpressionAnalyzer ea(*semaCtx);
- auto restorer{ea.GetContextualMessages().DiscardMessages()};
if (MaybeExpr expr{ea.Analyze(*desg)}) {
return ExtractSubstring(*expr).has_value();
}
+ return std::nullopt;
}
return false;
}
diff --git a/flang/test/Semantics/OpenMP/substring-bounds.f90 b/flang/test/Semantics/OpenMP/substring-bounds.f90
new file mode 100644
index 00000000000000..b152460dcf5815
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/substring-bounds.f90
@@ -0,0 +1,53 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=52
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=60
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=61
+
+! Permitted substring extensions still require valid substring bounds.
+! An error in an earlier clause must not suppress later diagnostics.
+subroutine substring_bounds(s)
+ character(8) :: s
+ !PORTABILITY: The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2.
+ !ERROR: Substring must begin at 1 or later, not 0
+ !$omp task depend(in: s(0:4))
+ !$omp end task
+ !PORTABILITY: The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2.
+ !ERROR: Substring must end at 8 or earlier, not 9
+ !$omp task affinity(s(1:9))
+ !$omp end task
+
+ ! Subsequent uses of the same symbol remain valid.
+ !PORTABILITY: The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2.
+ !$omp task depend(in: s(1:4))
+ !$omp end task
+ !PORTABILITY: The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2.
+ !$omp task affinity(s(1:4))
+ !$omp end task
+end subroutine
+
+subroutine component_substring_bounds()
+ type box
+ character(8) :: field
+ end type
+ type(box) :: b(2)
+ !PORTABILITY: The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2.
+ !ERROR: Substring must end at 8 or earlier, not 9
+ !$omp task depend(in: b(1:2)%field(1:9))
+ !$omp end task
+ !PORTABILITY: The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2.
+ !ERROR: Substring must begin at 1 or later, not 0
+ !$omp task affinity(b(1:2)%field(0:4))
+ !$omp end task
+end subroutine
+
+! Explicit substring parse nodes also need validation on DEPEND.
+subroutine array_element_substring_bounds(a)
+ character(8) :: a(2)
+ !ERROR: Substring must begin at 1 or later, not 0
+ !$omp task depend(in: a(1)(0:4))
+ !$omp end task
+ !ERROR: Substring must end at 8 or earlier, not 9
+ !$omp task depend(in: a(2)(1:9))
+ !$omp end task
+ !$omp task depend(in: a(1)(1:4))
+ !$omp end task
+end subroutine
diff --git a/flang/test/Semantics/OpenMP/substring-invalid.f90 b/flang/test/Semantics/OpenMP/substring-invalid.f90
new file mode 100644
index 00000000000000..e4d8f1ae305802
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/substring-invalid.f90
@@ -0,0 +1,64 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=50
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=52
+
+! Recognizing malformed substrings must not discard diagnostics and mark
+! symbols erroneous: later references to the same symbol used to crash.
+subroutine scalar_substrings(s)
+ character(8) :: s
+ !ERROR: A substring cannot have a stride
+ !$omp parallel private(s(::2))
+ !$omp end parallel
+ !ERROR: A substring cannot have a stride
+ !$omp parallel private(s(1::2))
+ !$omp end parallel
+ !ERROR: A substring cannot have a stride
+ !$omp parallel private(s(:8:2))
+ !$omp end parallel
+ !ERROR: A substring cannot have a stride
+ !$omp parallel private(s(1:8:2))
+ !$omp end parallel
+
+ !$omp parallel shared(s)
+ !$omp end parallel
+end subroutine
+
+subroutine component_substrings()
+ type box
+ character(8) :: field
+ character(8) :: array(8)
+ end type
+ type(box) :: b(2)
+
+ ! Malformed component substrings must also retain their diagnostics.
+ !ERROR: A substring cannot have a stride
+ !$omp parallel private(b(1)%field(1:4:2))
+ !$omp end parallel
+ !ERROR: A substring cannot have a stride
+ !$omp parallel private(b(1:2)%field(::2))
+ !$omp end parallel
+
+ ! A character array section is not a substring.
+ !$omp target map(tofrom: b(1)%array(1:4))
+ !$omp end target
+end subroutine
+
+! Other malformed references still exercise symbol-error recovery.
+subroutine scalar_array_section(i)
+ integer :: i
+ !ERROR: 'i' is not an array
+ !$omp parallel private(i(:))
+ !$omp end parallel
+ !$omp parallel shared(i)
+ !$omp end parallel
+end subroutine
+
+! Invalid bounds must not also produce a clause-specific substring error.
+subroutine invalid_substring_bounds(s)
+ character(8) :: s
+ !ERROR: Substring must begin at 1 or later, not 0
+ !$omp parallel private(s(0:4))
+ !$omp end parallel
+ !ERROR: Substring must end at 8 or earlier, not 9
+ !$omp parallel private(s(1:9))
+ !$omp end parallel
+end subroutine
diff --git a/flang/test/Semantics/substring-stride.f90 b/flang/test/Semantics/substring-stride.f90
new file mode 100644
index 00000000000000..199e94ffe850a5
--- /dev/null
+++ b/flang/test/Semantics/substring-stride.f90
@@ -0,0 +1,27 @@
+! RUN: %python %S/test_errors.py %s %flang_fc1
+
+subroutine substring_stride(s, a)
+ character(8) :: s, a(8)
+ type box
+ character(8) :: field
+ character(8) :: array(8)
+ end type
+ type(box) :: b(2)
+
+ !ERROR: A substring cannot have a stride
+ print *, s(::2)
+ !ERROR: A substring cannot have a stride
+ print *, s(1::2)
+ !ERROR: A substring cannot have a stride
+ print *, s(:8:2)
+ !ERROR: A substring cannot have a stride
+ print *, s(1:8:2)
+ !ERROR: A substring cannot have a stride
+ print *, b(1)%field(1:8:2)
+ !ERROR: A substring cannot have a stride
+ print *, b(:)%field(1:8:2)
+
+ ! Valid references and strided character array sections remain valid.
+ print *, s, s(1:4), b(1)%field, b(:)%field(1:4)
+ print *, a(::2), b(1)%array(::2)
+end subroutine
>From 693e6df594bdc8280ad6e128915f7963c0554a56 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Fri, 23 Oct 2026 11:09:22 +0100
Subject: [PATCH 2/2] Add comments
---
flang/include/flang/Semantics/openmp-utils.h | 6 ++++--
flang/lib/Semantics/check-omp-structure.cpp | 4 ++--
flang/lib/Semantics/openmp-utils.cpp | 7 ++++++-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index a3441892348815..75a4b71db72ead 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -115,8 +115,10 @@ bool IsLocatorListItem(
bool IsVariableListItem(
const parser::OmpObject &object, SemanticsContext *semaCtx);
-// A missing result indicates an invalid designator, already diagnosed.
-std::optional<bool> IsSubstring(
+// Analyzes potential substrings and retains diagnostics; may modify semantic
+// state and the parse tree. A missing result indicates an invalid designator,
+// already diagnosed.
+std::optional<bool> IsSubstringAndValidate(
const parser::OmpObject &object, SemanticsContext *semaCtx);
bool IsArrayElement(const parser::OmpObject &object, SemanticsContext *semaCtx);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index a631053c40abee..e5ec85721fab39 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -4596,7 +4596,7 @@ void OmpStructureChecker::CheckSubstring(
const parser::OmpObjectList &objects, llvm::omp::Clause clauseId) {
llvm::omp::Version version{context_.langOptions().getOpenMPVersion()};
for (const parser::OmpObject &object : objects.v) {
- if (IsSubstring(object, &context_).value_or(false)) {
+ if (IsSubstringAndValidate(object, &context_).value_or(false)) {
auto source{GetObjectSource(object)};
context_.Say(source ? *source : GetContext().clauseSource,
"A substring cannot appear in a %s clause"_err_en_US,
@@ -4706,7 +4706,7 @@ void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar(
void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar(
const parser::CharBlock &source, const parser::OmpObject &object,
llvm::StringRef clause, bool checkSubstrings) {
- auto isSubstring{IsSubstring(object, &context_)};
+ auto isSubstring{IsSubstringAndValidate(object, &context_)};
if (!isSubstring) {
return; // Expression analysis has already diagnosed the invalid item.
}
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 3f7e2153792f19..c19f8610cf16b4 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -327,13 +327,18 @@ bool IsVariableListItem(
return false;
}
-std::optional<bool> IsSubstring(
+std::optional<bool> IsSubstringAndValidate(
const parser::OmpObject &object, SemanticsContext *semaCtx) {
// Whole objects and other designators cannot be substrings. Analyzing them
// as expressions can reject otherwise valid OpenMP list items.
if (auto *desg{GetDesignatorFromObj(object)}; desg &&
(std::holds_alternative<parser::Substring>(desg->u) ||
GetArrayElementFromObj(object))) {
+ // This query also validates potential substrings, including in clauses
+ // where they are permitted. Expression analysis can mark symbols erroneous
+ // and rewrite the parse tree, so it is not free of side effects. Retain its
+ // diagnostics: discarding them leaves erroneous symbols without a reported
+ // error and can cause assertion failures when those symbols are used again.
evaluate::ExpressionAnalyzer ea(*semaCtx);
if (MaybeExpr expr{ea.Analyze(*desg)}) {
return ExtractSubstring(*expr).has_value();
More information about the flang-commits
mailing list