[flang-commits] [flang] [flang][runtime] Don't use endfile record number for EOF detection on… (PR #74640)

[Peter Klausler via flang-commits]

https://github.com/klausler updated https://github.com/llvm/llvm-project/pull/74640

>From c8cc7b04ceb4c53268ca3d628b0d7d47b81d336c Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler at nvidia.com>
Date: Wed, 6 Dec 2023 10:35:47 -0800
Subject: [PATCH] [flang][runtime] Don't use endfile record number for EOF
 detection on input

The current EOF detection method (IsAtEOF()) depends on comparing
the current record number with the record number of the endfile
record, if it is known, which it is for units that have been
written and then rewound for input.  For formatted input, this is wrong
in the case of a unit written with in-band newline characters.
Rather than scan output data to count newlines, it's best to just
organically determine EOF by detecting a failed or short read(),
as we would have done anyway had the endfile record number not been
known.  (I considered resetting the endfile record number at the
point of a REWIND, but not all rewinds are followed by input; it
seems wiser to defer the resetting until an actual READ takes place.)

Fixes llvm-test-suite/Fortran/gfortran/regression/backslash_2.f90
---
 flang/runtime/unit.cpp | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/flang/runtime/unit.cpp b/flang/runtime/unit.cpp
index 995656b9480c4..1ef9c66b0583f 100644
--- a/flang/runtime/unit.cpp
+++ b/flang/runtime/unit.cpp
@@ -436,6 +436,14 @@ bool ExternalFileUnit::BeginReadingRecord(IoErrorHandler &handler) {
   RUNTIME_CHECK(handler, direction_ == Direction::Input);
   if (!beganReadingRecord_) {
     beganReadingRecord_ = true;
+    // Don't use IsAtEOF() to check for an EOF condition here, just detect
+    // it from a failed or short read from the file.  IsAtEOF() could be
+    // wrong for formatted input if actual newline characters had been
+    // written in-band by previous WRITEs before a REWIND.  In fact,
+    // now that we know that the unit is being used for input (again),
+    // it's best to reset endfileRecordNumber and ensure IsAtEOF() will
+    // now be true on return only if it gets set by HitEndOnRead().
+    endfileRecordNumber.reset();
     if (access == Access::Direct) {
       CheckDirectAccess(handler);
       auto need{static_cast<std::size_t>(recordOffsetInFrame_ + *openRecl)};
@@ -448,17 +456,13 @@ bool ExternalFileUnit::BeginReadingRecord(IoErrorHandler &handler) {
       }
     } else {
       recordLength.reset();
-      if (IsAtEOF()) {
-        handler.SignalEnd();
-      } else {
-        RUNTIME_CHECK(handler, isUnformatted.has_value());
-        if (*isUnformatted) {
-          if (access == Access::Sequential) {
-            BeginSequentialVariableUnformattedInputRecord(handler);
-          }
-        } else { // formatted sequential or stream
-          BeginVariableFormattedInputRecord(handler);
+      RUNTIME_CHECK(handler, isUnformatted.has_value());
+      if (*isUnformatted) {
+        if (access == Access::Sequential) {
+          BeginSequentialVariableUnformattedInputRecord(handler);
         }
+      } else { // formatted sequential or stream
+        BeginVariableFormattedInputRecord(handler);
       }
     }
   }
@@ -723,6 +727,7 @@ void ExternalFileUnit::EndIoStatement() {
 
 void ExternalFileUnit::BeginSequentialVariableUnformattedInputRecord(
     IoErrorHandler &handler) {
+  RUNTIME_CHECK(handler, access == Access::Sequential);
   std::int32_t header{0}, footer{0};
   std::size_t need{recordOffsetInFrame_ + sizeof header};
   std::size_t got{ReadFrame(frameOffsetInFile_, need, handler)};