-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[flang] Do not error on constant nonzero UB in substring of ZLA #174511
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
Conversation
|
@llvm/pr-subscribers-flang-semantics Author: TMJ (tmjbios) ChangesA substring reference where the lower bound is higher than the upper bound is defined in 9.4.1 to be zero-length. Thus, a reference to a substring of a CHARACTER*(0) string such as cannot be a compile-time error since we do not know the return value of foo(). We also should not error if the lbound > ubound at compile time. Full diff: https://github.com/llvm/llvm-project/pull/174511.diff 2 Files Affected:
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index 402e88252fc20..89bc7264707de 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -1221,9 +1221,9 @@ MaybeExpr ExpressionAnalyzer::Analyze(const parser::Substring &ss) {
Say("Substring must begin at 1 or later, not %jd"_err_en_US,
static_cast<std::intmax_t>(*lbValue));
return std::nullopt;
- } else if (ubValue && len && *ubValue > *len &&
+ } else if (ubValue && len && *len > 0 && *ubValue > *len &&
(lbValue || !first)) {
- Say("Substring must end at %zd or earlier, not %jd"_err_en_US,
+ Say("Substring must end at %jd or earlier, not %jd"_err_en_US,
static_cast<std::intmax_t>(*len),
static_cast<std::intmax_t>(*ubValue));
return std::nullopt;
diff --git a/flang/test/Semantics/zero_len_char_array.f90 b/flang/test/Semantics/zero_len_char_array.f90
new file mode 100644
index 0000000000000..ee9f10fdd6d67
--- /dev/null
+++ b/flang/test/Semantics/zero_len_char_array.f90
@@ -0,0 +1,30 @@
+! RUN: %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s
+! CHECK-NOT: error:
+! Test that zero-length character substrings with lbound>ubounds indices
+! do not produce spurious errors when the character array is zero length.
+
+integer function init(i)
+ integer i
+ init=i
+end
+
+program flang_t7052
+ character*(*), parameter :: param_char = ""
+ character*(0) :: zero_len_char
+
+ if ( param_char(init(5):init(3)) > zero_len_char(1:-2) ) then
+ print *,"Test failed"
+ endif
+
+ if ( param_char(init(5):init(3)) > zero_len_char(10:2) ) then
+ print *,"Test failed"
+ endif
+
+ if ( param_char(init(5):init(3)) > zero_len_char(init(10):2) ) then
+ print *,"Test failed"
+ endif
+
+ if ( param_char(init(5):init(3)) > zero_len_char(init(10):-2) ) then
+ print *,"Test failed"
+ endif
+end program
|
|
Please add a prefix to your commit message so we know it is related to flang ( |
|
Bah! Sorry about that, Val. |
akuhlens
left a comment
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.
This looks great to me.
eugeneepshteyn
left a comment
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.
Thank you for the PR.
Could you please also run Fortran tests from llvm-test-suite?
A substring reference where the lower bound is higher
than the upper bound is defined in 9.4.1 to be zero-length.
Thus, a reference to a substring of a CHARACTER*(0)
string such as
string(foo():2)
cannot be a compile-time error since we do not know
the return value of foo().
We also should not error if the lbound > ubound at compile time.
* Cleaned up the test * Moved the zero-len check to the empty array check condition * Ran "Fortran" llvm-test-suite in addition to check-flang
7b7810e to
a674969
Compare
eugeneepshteyn
left a comment
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.
Thank you for the changes. LGTM!
|
Thanks! I don't have permission yet to merge my own PRs, so I'll need someone else to squash & merge. |
|
@tmjbios , did you run Fortran tests from llvm-test-suite with this change? |
|
Yes. I'm still getting used to the various test tools and methods for Flang; in addition to our internal test suite for CCE (where applicable) and the flang "make" lit test targets for flang and flang-rt, I follow this link's instructions for the llvm-test-suite for everything I'm doing: https://github.com/llvm/llvm-test-suite/blob/main/Fortran/gfortran/README.md#usage If there is additional pre-commit testing you would like to see us doing religiously, please let me know. |
…#174511) A substring reference where the lower bound is higher than the upper bound is defined in 9.4.1 to be zero-length. Thus, a reference to a substring of a CHARACTER*(0) string such as string(foo():2) cannot be a compile-time error since we do not know the return value of foo(). We also should not error if the lbound > ubound at compile time.
A substring reference where the lower bound is higher than the upper bound is defined in 9.4.1 to be zero-length.
Thus, a reference to a substring of a CHARACTER*(0) string such as
cannot be a compile-time error since we do not know the return value of foo().
We also should not error if the lbound > ubound at compile time.