[flang] Debugging of ACCESS='STREAM' I/O

[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.

[Meinersbur]

This patch causes the test flang-Unit::ExternalIOTests.TestWriteAfterNonAvancingInput to fail under Windows (https://lab.llvm.org/buildbot/#/builders/172/builds/7664)

[ RUN      ] ExternalIOTests.TestWriteAfterNonAvancingInput
C:\Users\buildbot-worker\minipc-ryzen-win\flang-x86_64-windows\llvm-project\flang\unittests\Runtime\ExternalIOTest.cpp(649): error: Expected equality of these values:
  resultRecord
    Which is: "ABCDEFGHXYZ\r        "
  expectedRecord
    Which is: "ABCDEFGHXYZ         "
Record after non advancing read followed by wrote

[awarzynski]

@klausler Thanks for this patch! I was hoping that I could find a fix for the failure on Windows myself, but no luck. I reverted instead - our Windows buildbot is one of our fastest and reliable CI machines. Also, while this does not work on Windows, our pre-commit CI is unnecessarily noisy.

Please, could take a look and re-submit?

[klausler]

In D118834#3293689, @awarzynski wrote:

@klausler Thanks for this patch! I was hoping that I could find a fix for the failure on Windows myself, but no luck. I reverted instead - our Windows buildbot is one of our fastest and reliable CI machines. Also, while this does not work on Windows, our pre-commit CI is unnecessarily noisy.

Please, could take a look and re-submit?

It seems that this patch exposed a problem; I suspect file truncation on Windows. I may resubmit later.

[Meinersbur]

Thanks for reverting. The stray \r makes me suspect something removed an \n but left over the second byte from Windows CRLF line endings.

If needed, I can look for the problem on Windows.

[klausler]

In D118834#3294452, @Meinersbur wrote:

Thanks for reverting. The stray \r makes me suspect something removed an \n but left over the second byte from Windows CRLF line endings.

If needed, I can look for the problem on Windows.

The f18 runtime doesn't emit \r (yet). So I don't know from whence it came, if it did indeed come from the f18 runtime.

[Meinersbur]

The runtime uses a temporary file in scratch mode that the unittest uses, which is opened in text mode. Under Windows, when writing \n to a file opened in text mode, it will actually write \r\n. However, the internal position counter frameOffsetInFile_ will not take the additional character into account. On Rewind, it will truncate the file to the frameOffsetInFile_ size, keeping the first \r but truncating the second character \n away.

This makes the test pass:

diff --git a/flang/runtime/file.cpp b/flang/runtime/file.cpp
index 4e36978cfb23..787845f78353 100644
--- a/flang/runtime/file.cpp
+++ b/flang/runtime/file.cpp
@@ -46,7 +46,7 @@ static int openfile_mkstemp(IoErrorHandler &handler) {
     return -1;
   }
   int fd{::_open(
-      tempFileName, _O_CREAT | _O_TEMPORARY | _O_RDWR, _S_IREAD | _S_IWRITE)};
+      tempFileName, _O_CREAT | _O_BINARY | _O_TEMPORARY | _O_RDWR, _S_IREAD | _S_IWRITE)};
 #else
   char path[]{"/tmp/Fortran-Scratch-XXXXXX"};
   int fd{::mkstemp(path)};

However, I don't think this is an universal solution, as stdout and stderr are always in text mode.

[klausler]

In D118834#3295507, @Meinersbur wrote:

The runtime uses a temporary file in scratch mode that the unittest uses, which is opened in text mode. Under Windows, when writing \n to a file opened in text mode, it will actually write \r\n. However, the internal position counter frameOffsetInFile_ will not take the additional character into account. On Rewind, it will truncate the file to the frameOffsetInFile_ size, keeping the first \r but truncating the second character \n away.

This makes the test pass:

diff --git a/flang/runtime/file.cpp b/flang/runtime/file.cpp
index 4e36978cfb23..787845f78353 100644
--- a/flang/runtime/file.cpp
+++ b/flang/runtime/file.cpp
@@ -46,7 +46,7 @@ static int openfile_mkstemp(IoErrorHandler &handler) {
     return -1;
   }
   int fd{::_open(
-      tempFileName, _O_CREAT | _O_TEMPORARY | _O_RDWR, _S_IREAD | _S_IWRITE)};
+      tempFileName, _O_CREAT | _O_BINARY | _O_TEMPORARY | _O_RDWR, _S_IREAD | _S_IWRITE)};
 #else
   char path[]{"/tmp/Fortran-Scratch-XXXXXX"};
   int fd{::mkstemp(path)};

However, I don't think this is an universal solution, as stdout and stderr are always in text mode.

Thanks, that's a helpful clue. Unfortunately in Fortran we can't always know whether a file will be formatted or unformatted at the time that the file is opened, so the runtime will probably have to always use binary mode and emit explicit carriage returns. Stdout and stderr being in text mode is not much of a problem -- they don't have to be positionable and are always formatted. I will get back to this in a few weeks after I dig out from under a pile of more important things.

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_; }
   std::optional<FileOffset> knownSize() const { return knownSize_; }
 

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;

     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;
   }
 }

     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)) {

         }
       }
     }
+    ++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
+      ok = ok && Emit("\n", 1, 1, handler); // TODO: Windows CR+LF
     }
     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;