[flang] Debugging of ACCESS='STREAM' I/O (take 2)

[klausler]

Summary

Corrects the runtime implementation of I/O on files with
the access mode ACCESS='STREAM'. This is a collection
of edge-case tweaks to ensure that the distinctions between
stream and direct/sequential files, unformatted or formatted,
are respected where appropriate.

Moves NextInField() from io-stmt.h to io-stmt.cpp --
it was getting too big to keep in a header.

The original version of this patch exposed a problem with
the I/O runtime on Windows and it was reverted. This new
version also fixes that problem; files are now opened on Windows
in binary mode to prevent inadvertent insertions of
carriage returns before line feeds, and those line
endings (CR+LF) are now explicitly generated.

[Meinersbur]

flang/runtime/file.cpp

164

I have a concern here: If fd is <=2, (stdout or stderr), it will be in text mode, which expects a single \n for a newline, but with isWindowsTextFile_ = true, we will write \r\n (which the c-runtime expands to \r\r\n).

[klausler]

flang/runtime/file.cpp

164

When isWindowsTextFile_ is true, only the \n will be written. See unit.cpp line 557 -- the emission of the \r is controlled by "if (!isWindowsTextFile())" (note the negation).

[Meinersbur]

LGTM

flang/runtime/file.cpp

164

Correct, I was inverting what the flag means. Hence, effectively with OpenFile::Open the file will be opened in binary mode write \r\n and with OpenFile::Predefine text mode is assumed and only \n is written.

Still means that with OpenFile::Predefine lineEndingBytes is set to 2 even though it effectively writes \r\n, but since stdout/stderr is not generalably seekable I don't think it matters.

flang/include/flang/Runtime/iostat.h

   IostatInternalWriteOverrun,
   IostatErrorInFormat,
   IostatErrorInKeyword,
-  IostatEndfileNonSequential,
+  IostatEndfileDirect,
   IostatEndfileUnwritable,
   IostatOpenBadRecl,
   IostatOpenUnknownSize,

flang/runtime/connection.h

 enum class Direction { Output, Input };
 enum class Access { Sequential, Direct, Stream };
 
-inline bool IsRecordFile(Access a) { return a != Access::Stream; }
-
 // These characteristics of a connection are immutable after being
 // established in an OPEN statement.
 struct ConnectionAttributes {

   std::optional<bool> isUnformatted; // FORM='UNFORMATTED' if true
   bool isUTF8{false}; // ENCODING='UTF-8'
   std::optional<std::int64_t> openRecl; // RECL= on OPEN
+
+  bool IsRecordFile() const {
+    // Formatted stream files are viewed as having records, at least on input
+    return access != Access::Stream || !isUnformatted.value_or(true);
+  }
 };
 
 struct ConnectionState : public ConnectionAttributes {

flang/runtime/edit-input.cpp

   std::optional<int> remaining;
   std::optional<char32_t> next{io.PrepareInput(edit, remaining)};
   common::UnsignedInt128 value{0};
-  for (; next; next = io.NextInField(remaining)) {
+  for (; next; next = io.NextInField(remaining, edit)) {
     char32_t ch{*next};
     if (ch == ' ' || ch == '\t') {
       continue;

     if (negative || *next == '+') {
       io.GotChar();
       io.SkipSpaces(remaining);
-      next = io.NextInField(remaining, GetDecimalPoint(edit));
+      next = io.NextInField(remaining, edit);
     }
   }
   return negative;

   bool negate{ScanNumericPrefix(io, edit, next, remaining)};
   common::UnsignedInt128 value{0};
   bool any{negate};
-  for (; next; next = io.NextInField(remaining)) {
+  for (; next; next = io.NextInField(remaining, edit)) {
     char32_t ch{*next};
     if (ch == ' ' || ch == '\t') {
       if (edit.modes.editingFlags & blankZero) {

     // Subtle: a blank field of digits could be followed by 'E' or 'D',
     for (; next &&
          ((*next >= 'a' && *next <= 'z') || (*next >= 'A' && *next <= 'Z'));
-         next = io.NextInField(remaining)) {
+         next = io.NextInField(remaining, edit)) {
       if (*next >= 'a' && *next <= 'z') {
         Put(*next - 'a' + 'A');
       } else {

     }
     if (next && *next == '(') { // NaN(...)
       while (next && *next != ')') {
-        next = io.NextInField(remaining);
+        next = io.NextInField(remaining, edit);
       }
     }
     exponent = 0;

     Put('.'); // input field is normalized to a fraction
     auto start{got};
     bool bzMode{(edit.modes.editingFlags & blankZero) != 0};
-    for (; next; next = io.NextInField(remaining, decimal)) {
+    for (; next; next = io.NextInField(remaining, edit)) {
       char32_t ch{*next};
       if (ch == ' ' || ch == '\t') {
         if (bzMode) {

       // Optional exponent letter.  Blanks are allowed between the
       // optional exponent letter and the exponent value.
       io.SkipSpaces(remaining);
-      next = io.NextInField(remaining);
+      next = io.NextInField(remaining, edit);
     }
     // The default exponent is -kP, but the scale factor doesn't affect
     // an explicit exponent.

             (bzMode && (*next == ' ' || *next == '\t')))) {
       bool negExpo{*next == '-'};
       if (negExpo || *next == '+') {
-        next = io.NextInField(remaining);
+        next = io.NextInField(remaining, edit);
       }
-      for (exponent = 0; next; next = io.NextInField(remaining)) {
+      for (exponent = 0; next; next = io.NextInField(remaining, edit)) {
         if (*next >= '0' && *next <= '9') {
           exponent = 10 * exponent + *next - '0';
         } else if (bzMode && (*next == ' ' || *next == '\t')) {

   // input value.
   if (edit.descriptor == DataEdit::ListDirectedImaginaryPart) {
     if (next && (*next == ' ' || *next == '\t')) {
-      next = io.NextInField(remaining);
+      next = io.NextInField(remaining, edit);
     }
     if (!next) { // NextInField fails on separators like ')'
       next = io.GetCurrentChar();

     }
   } else if (remaining) {
     while (next && (*next == ' ' || *next == '\t')) {
-      next = io.NextInField(remaining);
+      next = io.NextInField(remaining, edit);
     }
     if (next) {
       return 0; // error: unused nonblank character in fixed-width field

   std::optional<int> remaining;
   std::optional<char32_t> next{io.PrepareInput(edit, remaining)};
   if (next && *next == '.') { // skip optional period
-    next = io.NextInField(remaining);
+    next = io.NextInField(remaining, edit);
   }
   if (!next) {
     io.GetIoErrorHandler().SignalError("Empty LOGICAL input field");

   if (remaining) { // ignore the rest of the field
     io.HandleRelativePosition(*remaining);
   } else if (edit.descriptor == DataEdit::ListDirected) {
-    while (io.NextInField(remaining)) { // discard rest of field
+    while (io.NextInField(remaining, edit)) { // discard rest of field
     }
   }
   return true;

 }
 
 static bool EditListDirectedDefaultCharacterInput(
-    IoStatementState &io, char *x, std::size_t length) {
+    IoStatementState &io, char *x, std::size_t length, const DataEdit &edit) {
   auto ch{io.GetCurrentChar()};
   if (ch && (*ch == '\'' || *ch == '"')) {
     io.HandleRelativePosition(1);

   // Undelimited list-directed character input: stop at a value separator
   // or the end of the current record.
   std::optional<int> remaining{length};
-  for (std::optional<char32_t> next{io.NextInField(remaining)}; next;
-       next = io.NextInField(remaining)) {
+  while (std::optional<char32_t> next{io.NextInField(remaining, edit)}) {
     switch (*next) {
     case ' ':
     case '\t':

     IoStatementState &io, const DataEdit &edit, char *x, std::size_t length) {
   switch (edit.descriptor) {
   case DataEdit::ListDirected:
-    return EditListDirectedDefaultCharacterInput(io, x, length);
+    return EditListDirectedDefaultCharacterInput(io, x, length, edit);
   case 'A':
   case 'G':
     break;

   // characters.  When the variable is wider than the field, there's
   // trailing padding.
   std::int64_t skip{*remaining - static_cast<std::int64_t>(length)};
-  for (std::optional<char32_t> next{io.NextInField(remaining)}; next;
-       next = io.NextInField(remaining)) {
+  while (std::optional<char32_t> next{io.NextInField(remaining, edit)}) {
     if (skip > 0) {
       --skip;
       io.GotChar(-1);

flang/runtime/file.h

   bool mayPosition() const { return mayPosition_; }
   bool mayAsynchronous() const { return mayAsynchronous_; }
   void set_mayAsynchronous(bool yes) { mayAsynchronous_ = yes; }
-  FileOffset position() const { return position_; }
   bool isTerminal() const { return isTerminal_; }
+  bool isWindowsTextFile() const { return isWindowsTextFile_; }
   std::optional<FileOffset> knownSize() const { return knownSize_; }
 
   bool IsConnected() const { return fd_ >= 0; }

   FileOffset position_{0};
   std::optional<FileOffset> knownSize_;
   bool isTerminal_{false};
+  bool isWindowsTextFile_{false}; // expands LF to CR+LF on write
 
   int nextId_;
   OwningPtr<Pending> pending_;

flang/runtime/file.cpp

   if (::GetTempFileNameA(tempDirName, "Fortran", uUnique, tempFileName) == 0) {
     return -1;
   }
-  int fd{::_open(
-      tempFileName, _O_CREAT | _O_TEMPORARY | _O_RDWR, _S_IREAD | _S_IWRITE)};
+  int fd{::_open(tempFileName, _O_CREAT | _O_BINARY | _O_TEMPORARY | _O_RDWR,
+      _S_IREAD | _S_IWRITE)};
 #else
   char path[]{"/tmp/Fortran-Scratch-XXXXXX"};
   int fd{::mkstemp(path)};

       return;
     }
     int flags{0};
+#ifdef _WIN32
+    // We emit explicit CR+LF line endings and cope with them on input
+    // for formatted files, since we can't yet always know now at OPEN
+    // time whether the file is formatted or not.
+    flags |= O_BINARY;
+#endif
     if (status != OpenStatus::Old) {
       flags |= O_CREAT;
     }

   mayRead_ = fd == 0;
   mayWrite_ = fd != 0;
   mayPosition_ = false;
+#ifdef _WIN32
+  isWindowsTextFile_ = true;
+#endif
 }
 
 void OpenFile::Close(CloseStatus status, IoErrorHandler &handler) {

flang/runtime/io-api.cpp

 bool IONAME(SetPos)(Cookie cookie, std::int64_t pos) {
   IoStatementState &io{*cookie};
   ConnectionState &connection{io.GetConnectionState()};
+  IoErrorHandler &handler{io.GetIoErrorHandler()};
   if (connection.access != Access::Stream) {
-    io.GetIoErrorHandler().SignalError(
-        "POS= may not appear unless ACCESS='STREAM'");
+    handler.SignalError("POS= may not appear unless ACCESS='STREAM'");
     return false;
   }
-  if (pos < 1) {
-    io.GetIoErrorHandler().SignalError(
-        "POS=%zd is invalid", static_cast<std::intmax_t>(pos));
+  if (pos < 1) { // POS=1 is beginning of file (12.6.2.11)
+    handler.SignalError("POS=%zd is invalid", static_cast<std::intmax_t>(pos));
     return false;
   }
   if (auto *unit{io.GetExternalFileUnit()}) {
-    unit->SetPosition(pos);
+    unit->SetPosition(pos - 1, handler);
     return true;
   }
   io.GetIoErrorHandler().Crash("SetPos() on internal unit");

 bool IONAME(SetRec)(Cookie cookie, std::int64_t rec) {
   IoStatementState &io{*cookie};
   ConnectionState &connection{io.GetConnectionState()};
+  IoErrorHandler &handler{io.GetIoErrorHandler()};
   if (connection.access != Access::Direct) {
-    io.GetIoErrorHandler().SignalError(
-        "REC= may not appear unless ACCESS='DIRECT'");
+    handler.SignalError("REC= may not appear unless ACCESS='DIRECT'");
     return false;
   }
   if (!connection.openRecl) {
-    io.GetIoErrorHandler().SignalError("RECL= was not specified");
+    handler.SignalError("RECL= was not specified");
     return false;
   }
   if (rec < 1) {
-    io.GetIoErrorHandler().SignalError(
-        "REC=%zd is invalid", static_cast<std::intmax_t>(rec));
+    handler.SignalError("REC=%zd is invalid", static_cast<std::intmax_t>(rec));
     return false;
   }
   connection.currentRecordNumber = rec;
   if (auto *unit{io.GetExternalFileUnit()}) {
-    unit->SetPosition((rec - 1) * *connection.openRecl);
+    unit->SetPosition((rec - 1) * *connection.openRecl, handler);
   }
   return true;
 }

flang/runtime/io-stmt.h

       }
       SkipSpaces(remaining);
     }
-    return NextInField(remaining);
+    return NextInField(remaining, edit);
   }
 
   std::optional<char32_t> SkipSpaces(std::optional<int> &remaining) {

     return std::nullopt;
   }
 
+  // Acquires the next input character, respecting any applicable field width
+  // or separator character.
   std::optional<char32_t> NextInField(
-      std::optional<int> &remaining, char32_t decimal = '.') {
-    if (!remaining) { // list-directed or NAMELIST: check for separators
-      if (auto next{GetCurrentChar()}) {
-        if (*next == decimal) { // can be ','
-          HandleRelativePosition(1);
-          return next;
-        }
-        switch (*next) {
-        case ' ':
-        case '\t':
-        case ',':
-        case ';':
-        case '/':
-        case '(':
-        case ')':
-        case '\'':
-        case '"':
-        case '*':
-        case '\n': // for stream access
-          break;
-        default:
-          HandleRelativePosition(1);
-          return next;
-        }
-      }
-    } else if (*remaining > 0) {
-      if (auto next{GetCurrentChar()}) {
-        --*remaining;
-        HandleRelativePosition(1);
-        GotChar();
-        return next;
-      }
-      const ConnectionState &connection{GetConnectionState()};
-      if (!connection.IsAtEOF()) {
-        if (auto length{connection.EffectiveRecordLength()}) {
-          if (connection.positionInRecord >= *length) {
-            IoErrorHandler &handler{GetIoErrorHandler()};
-            if (mutableModes().nonAdvancing) {
-              handler.SignalEor();
-            } else if (connection.openRecl && !connection.modes.pad) {
-              handler.SignalError(IostatRecordReadOverrun);
-            }
-            if (connection.modes.pad) { // PAD='YES'
-              --*remaining;
-              return std::optional<char32_t>{' '};
-            }
-          }
-        }
-      }
-    }
-    return std::nullopt;
-  }
+      std::optional<int> &remaining, const DataEdit &);
 
   // Skips spaces, advances records, and ignores NAMELIST comments
   std::optional<char32_t> GetNextNonBlank() {

flang/runtime/io-stmt.cpp

     : ExternalIoStatementBase{unit, sourceFile, sourceLine}, mutableModes_{
                                                                  unit.modes} {
   if constexpr (DIR == Direction::Output) {
-    // If the last statement was a non advancing IO input statement, the unit
+    // If the last statement was a non-advancing IO input statement, the unit
     // furthestPositionInRecord was not advanced, but the positionInRecord may
     // have been advanced. Advance furthestPositionInRecord here to avoid
     // overwriting the part of the record that has been read with blanks.

   }
 }
 
+std::optional<char32_t> IoStatementState::NextInField(
+    std::optional<int> &remaining, const DataEdit &edit) {
+  if (!remaining) { // Stream, list-directed, or NAMELIST
+    if (auto next{GetCurrentChar()}) {
+      if (edit.IsListDirected()) {
+        // list-directed or NAMELIST: check for separators
+        switch (*next) {
+        case ' ':
+        case '\t':
+        case ';':
+        case '/':
+        case '(':
+        case ')':
+        case '\'':
+        case '"':
+        case '*':
+        case '\n': // for stream access
+          return std::nullopt;
+        case ',':
+          if (edit.modes.editingFlags & decimalComma) {
+            break;
+          } else {
+            return std::nullopt;
+          }
+        default:
+          break;
+        }
+      }
+      HandleRelativePosition(1);
+      GotChar();
+      return next;
+    }
+  } else if (*remaining > 0) {
+    if (auto next{GetCurrentChar()}) {
+      --*remaining;
+      HandleRelativePosition(1);
+      GotChar();
+      return next;
+    }
+    const ConnectionState &connection{GetConnectionState()};
+    if (!connection.IsAtEOF()) {
+      if (auto length{connection.EffectiveRecordLength()}) {
+        if (connection.positionInRecord >= *length) {
+          IoErrorHandler &handler{GetIoErrorHandler()};
+          if (mutableModes().nonAdvancing) {
+            handler.SignalEor();
+          } else if (connection.openRecl && !connection.modes.pad) {
+            handler.SignalError(IostatRecordReadOverrun);
+          }
+          if (connection.modes.pad) { // PAD='YES'
+            --*remaining;
+            return std::optional<char32_t>{' '};
+          }
+        }
+      }
+    }
+  }
+  return std::nullopt;
+}
+
 bool IoStatementState::Inquire(
     InquiryKeywordHash inquiry, char *out, std::size_t chars) {
   return std::visit(

     result = unit().IsConnected() ? unit().unitNumber() : -1;
     return true;
   case HashInquiryKeyword("POS"):
-    result = unit().position();
+    result = unit().InquirePos();
     return true;
   case HashInquiryKeyword("RECL"):
     if (!unit().IsConnected()) {

flang/runtime/iostat.cpp

     return "Invalid FORMAT";
   case IostatErrorInKeyword:
     return "Bad keyword argument value";
-  case IostatEndfileNonSequential:
-    return "ENDFILE on non-sequential file";
+  case IostatEndfileDirect:
+    return "ENDFILE on direct-access file";
   case IostatEndfileUnwritable:
     return "ENDFILE on read-only file";
   case IostatOpenBadRecl:

flang/runtime/unit.h

   void Endfile(IoErrorHandler &);
   void Rewind(IoErrorHandler &);
   void EndIoStatement();
-  void SetPosition(std::int64_t pos) {
-    frameOffsetInFile_ = pos;
-    recordOffsetInFrame_ = 0;
-    BeginRecord();
+  void SetPosition(std::int64_t, IoErrorHandler &); // zero-based
+  std::int64_t InquirePos() const {
+    // 12.6.2.11 defines POS=1 as the beginning of file
+    return frameOffsetInFile_ + 1;
   }
 
   ChildIo *GetChildIo() { return child_.get(); }

   static UnitMap &GetUnitMap();
   const char *FrameNextInput(IoErrorHandler &, std::size_t);
   void BeginSequentialVariableUnformattedInputRecord(IoErrorHandler &);
-  void BeginSequentialVariableFormattedInputRecord(IoErrorHandler &);
+  void BeginVariableFormattedInputRecord(IoErrorHandler &);
   void BackspaceFixedRecord(IoErrorHandler &);
   void BackspaceVariableUnformattedRecord(IoErrorHandler &);
   void BackspaceVariableFormattedRecord(IoErrorHandler &);
-  bool SetSequentialVariableFormattedRecordLength();
+  bool SetVariableFormattedRecordLength();
   void DoImpliedEndfile(IoErrorHandler &);
   void DoEndfile(IoErrorHandler &);
   void CommitWrites();
 
   int unitNumber_{-1};
   Direction direction_{Direction::Output};
-  bool impliedEndfile_{false}; // seq. output has taken place
+  bool impliedEndfile_{false}; // sequential/stream output has taken place
   bool beganReadingRecord_{false};
 
   Lock lock_;

flang/runtime/unit.cpp

   if (totalBytes && access == Access::Direct && openRecl.value_or(0) > 0) {
     endfileRecordNumber = 1 + (*totalBytes / *openRecl);
   }
-  if (position == Position::Append) {
+  if (position == Position::Append && access != Access::Stream) {
     if (!endfileRecordNumber) {
       // Fake it so that we can backspace relative from the end
       endfileRecordNumber = std::numeric_limits<std::int64_t>::max() - 2;

         header = static_cast<int>(sizeof(std::uint32_t));
         extra = 2 * header;
       } else {
-        extra = 1; // newline
+#ifdef _WIN32
+        if (!isWindowsTextFile()) {
+          ++extra; // carriage return (CR)
+        }
+#endif
+        ++extra; // newline (LF)
       }
     }
     if (furthestAfter > extra + *openRecl) {

     furthestPositionInRecord = furthestAfter;
     return true;
   } else {
-    // EOF or error: can be handled & has been signaled
-    endfileRecordNumber = currentRecordNumber;
+    handler.SignalEnd();
+    if (access == Access::Sequential) {
+      endfileRecordNumber = currentRecordNumber;
+    }
     return false;
   }
 }

 std::size_t ExternalFileUnit::GetNextInputBytes(
     const char *&p, IoErrorHandler &handler) {
   RUNTIME_CHECK(handler, direction_ == Direction::Input);
-  p = FrameNextInput(handler, 1);
-  return p ? EffectiveRecordLength().value_or(positionInRecord + 1) -
-          positionInRecord
-           : 0;
+  std::size_t length{1};
+  if (auto recl{EffectiveRecordLength()}) {
+    if (positionInRecord < *recl) {
+      length = *recl - positionInRecord;
+    } else {
+      p = nullptr;
+      return 0;
+    }
+  }
+  p = FrameNextInput(handler, length);
+  return p ? length : 0;
 }
 
 std::optional<char32_t> ExternalFileUnit::GetCurrentChar(

     auto at{recordOffsetInFrame_ + positionInRecord};
     auto need{static_cast<std::size_t>(at + bytes)};
     auto got{ReadFrame(frameOffsetInFile_, need, handler)};
-    SetSequentialVariableFormattedRecordLength();
+    SetVariableFormattedRecordLength();
     if (got >= need) {
       return Frame() + at;
     }
     handler.SignalEnd();
-    endfileRecordNumber = currentRecordNumber;
+    if (access == Access::Sequential) {
+      endfileRecordNumber = currentRecordNumber;
+    }
   }
   return nullptr;
 }
 
-bool ExternalFileUnit::SetSequentialVariableFormattedRecordLength() {
-  if (recordLength || access != Access::Sequential) {
+bool ExternalFileUnit::SetVariableFormattedRecordLength() {
+  if (recordLength || access == Access::Direct) {
     return true;
   } else if (FrameLength() > recordOffsetInFrame_) {
     const char *record{Frame() + recordOffsetInFrame_};

         recordLength.reset();
         handler.SignalEnd();
       }
-    } else if (access == Access::Sequential) {
+    } else {
       recordLength.reset();
       if (IsAtEOF()) {
         handler.SignalEnd();
       } else {
         RUNTIME_CHECK(handler, isUnformatted.has_value());
-        if (isUnformatted.value_or(false)) {
-          BeginSequentialVariableUnformattedInputRecord(handler);
-        } else { // formatted
-          BeginSequentialVariableFormattedInputRecord(handler);
+        if (*isUnformatted) {
+          if (access == Access::Sequential) {
+            BeginSequentialVariableUnformattedInputRecord(handler);
+          }
+        } else { // formatted sequential or stream
+          BeginVariableFormattedInputRecord(handler);
         }
       }
     }
   }
   RUNTIME_CHECK(handler,
-      recordLength.has_value() || !IsRecordFile(access) || handler.InError());
+      recordLength.has_value() || !IsRecordFile() || handler.InError());
   return !handler.InError();
 }
 

   RUNTIME_CHECK(handler, direction_ == Direction::Input && beganReadingRecord_);
   beganReadingRecord_ = false;
   if (handler.InError() && handler.GetIoStat() != IostatEor) {
-    // avoid bogus crashes in END/ERR circumstances
-  } else if (access == Access::Sequential) {
+    // Avoid bogus crashes in END/ERR circumstances; but
+    // still increment the current record number so that
+    // an attempted read of an endfile record, followed by
+    // a BACKSPACE, will still be at EOF.
+    ++currentRecordNumber;
+  } else if (IsRecordFile()) {
     RUNTIME_CHECK(handler, recordLength.has_value());
     recordOffsetInFrame_ += *recordLength;
-    if (openRecl && access == Access::Direct) {
-      frameOffsetInFile_ += recordOffsetInFrame_;
-      recordOffsetInFrame_ = 0;
-    } else {
+    if (access != Access::Direct) {
       RUNTIME_CHECK(handler, isUnformatted.has_value());
       recordLength.reset();
       if (isUnformatted.value_or(false)) {

             Frame()[recordOffsetInFrame_] == '\r') {
           ++recordOffsetInFrame_;
         }
-        if (FrameLength() >= recordOffsetInFrame_ &&
+        if (FrameLength() > recordOffsetInFrame_ &&
             Frame()[recordOffsetInFrame_] == '\n') {
           ++recordOffsetInFrame_;
         }

         }
       }
     }
+    ++currentRecordNumber;
+  } else { // unformatted stream
+    furthestPositionInRecord =
+        std::max(furthestPositionInRecord, positionInRecord);
+    frameOffsetInFile_ += recordOffsetInFrame_ + furthestPositionInRecord;
   }
-  ++currentRecordNumber;
   BeginRecord();
 }
 

   } else { // Direction::Output
     bool ok{true};
     RUNTIME_CHECK(handler, isUnformatted.has_value());
-    if (openRecl && access == Access::Direct) {
-      if (furthestPositionInRecord < *openRecl) {
+    positionInRecord = furthestPositionInRecord;
+    if (access == Access::Direct) {
+      if (furthestPositionInRecord <
+          openRecl.value_or(furthestPositionInRecord)) {
         // Pad remainder of fixed length record
         WriteFrame(
             frameOffsetInFile_, recordOffsetInFrame_ + *openRecl, handler);

             *openRecl - furthestPositionInRecord);
         furthestPositionInRecord = *openRecl;
       }
-    } else {
-      positionInRecord = furthestPositionInRecord;
-      if (isUnformatted.value_or(false)) {
+    } else if (*isUnformatted) {
+      if (access == Access::Sequential) {
         // Append the length of a sequential unformatted variable-length record
         // as its footer, then overwrite the reserved first four bytes of the
         // record with its length as its header.  These four bytes were skipped

             Emit(reinterpret_cast<const char *>(&length), sizeof length,
                 sizeof length, handler);
       } else {
-        // Terminate formatted variable length record
-        ok = ok && Emit("\n", 1, 1, handler); // TODO: Windows CR+LF
+        // Unformatted stream: nothing to do
       }
+    } else {
+      // Terminate formatted variable length record
+      const char *lineEnding{"\n"};
+      std::size_t lineEndingBytes{1};
+#ifdef _WIN32
+      if (!isWindowsTextFile()) {
+        lineEnding = "\r\n";
+        lineEndingBytes = 2;
+      }
+#endif
+      ok = ok && Emit(lineEnding, lineEndingBytes, 1, handler);
     }
     if (IsAfterEndfile()) {
       return false;
     }
     CommitWrites();
-    impliedEndfile_ = true;
     ++currentRecordNumber;
-    if (IsAtEOF()) {
-      endfileRecordNumber.reset();
+    if (access != Access::Direct) {
+      impliedEndfile_ = IsRecordFile();
+      if (IsAtEOF()) {
+        endfileRecordNumber.reset();
+      }
     }
     return ok;
   }
 }
 
 void ExternalFileUnit::BackspaceRecord(IoErrorHandler &handler) {
-  if (access != Access::Sequential) {
+  if (access == Access::Direct || !IsRecordFile()) {
     handler.SignalError(IostatBackspaceNonSequential,
-        "BACKSPACE(UNIT=%d) on non-sequential file", unitNumber());
+        "BACKSPACE(UNIT=%d) on direct-access file or unformatted stream",
+        unitNumber());
   } else {
     if (IsAfterEndfile()) {
       // BACKSPACE after explicit ENDFILE

 }
 
 void ExternalFileUnit::Endfile(IoErrorHandler &handler) {
-  if (access != Access::Sequential) {
-    handler.SignalError(IostatEndfileNonSequential,
-        "ENDFILE(UNIT=%d) on non-sequential file", unitNumber());
+  if (access == Access::Direct) {
+    handler.SignalError(IostatEndfileDirect,
+        "ENDFILE(UNIT=%d) on direct-access file", unitNumber());
   } else if (!mayWrite()) {
     handler.SignalError(IostatEndfileUnwritable,
         "ENDFILE(UNIT=%d) on read-only file", unitNumber());

     // ENDFILE after ENDFILE
   } else {
     DoEndfile(handler);
-    // Explicit ENDFILE leaves position *after* the endfile record
-    RUNTIME_CHECK(handler, endfileRecordNumber.has_value());
-    currentRecordNumber = *endfileRecordNumber + 1;
+    if (access == Access::Sequential) {
+      // Explicit ENDFILE leaves position *after* the endfile record
+      RUNTIME_CHECK(handler, endfileRecordNumber.has_value());
+      currentRecordNumber = *endfileRecordNumber + 1;
+    }
   }
 }
 

     handler.SignalError(IostatRewindNonSequential,
         "REWIND(UNIT=%d) on non-sequential file", unitNumber());
   } else {
-    DoImpliedEndfile(handler);
-    SetPosition(0);
+    SetPosition(0, handler);
     currentRecordNumber = 1;
   }
 }
 
+void ExternalFileUnit::SetPosition(std::int64_t pos, IoErrorHandler &handler) {
+  DoImpliedEndfile(handler);
+  frameOffsetInFile_ = pos;
+  recordOffsetInFrame_ = 0;
+  BeginRecord();
+}
+
 void ExternalFileUnit::EndIoStatement() {
   io_.reset();
   u_.emplace<std::monostate>();

   positionInRecord = sizeof header;
 }
 
-void ExternalFileUnit::BeginSequentialVariableFormattedInputRecord(
+void ExternalFileUnit::BeginVariableFormattedInputRecord(
     IoErrorHandler &handler) {
   if (this == defaultInput) {
     if (defaultOutput) {

       }
       break;
     }
-  } while (!SetSequentialVariableFormattedRecordLength());
+  } while (!SetVariableFormattedRecordLength());
 }
 
 void ExternalFileUnit::BackspaceFixedRecord(IoErrorHandler &handler) {

 void ExternalFileUnit::DoImpliedEndfile(IoErrorHandler &handler) {
   if (impliedEndfile_) {
     impliedEndfile_ = false;
-    if (access == Access::Sequential && mayPosition()) {
+    if (access != Access::Direct && IsRecordFile() && mayPosition()) {
       DoEndfile(handler);
     }
   }
 }
 
 void ExternalFileUnit::DoEndfile(IoErrorHandler &handler) {
-  endfileRecordNumber = currentRecordNumber;
+  if (access == Access::Sequential) {
+    endfileRecordNumber = currentRecordNumber;
+  }
+  FlushOutput(handler);
   Truncate(frameOffsetInFile_ + recordOffsetInFrame_, handler);
   BeginRecord();
   impliedEndfile_ = false;

flang/unittests/Runtime/ExternalIOTest.cpp

   ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
       << "EndIoStatement() for OutputAscii";
 
-  // Verify that the output was written in the record read in non avdancing
+  // Verify that the output was written in the record read in non advancing
   // mode, after the read part, and that the end was truncated.
 
   // REWIND(UNIT=unit)

       << "EndIoStatement() for Read ";
 
   ASSERT_EQ(resultRecord, expectedRecord)
-      << "Record after non advancing read followed by wrote";
+      << "Record after non advancing read followed by write";
 }
 
 TEST(ExternalIOTests, TestWriteAfterEndfile) {