[flang][runtime] Catch more (all?) negative unit number errors

[klausler]

Summary

Fortran does have negative unit numbers -- they show up in child I/O
subroutines for defined I/O and for OPEN(NEWUNIT=) -- but the runtime
needs to catch the cases where a negative unit number that wasn't
generated by the runtime is passed in for OPEN or for an I/O statement
that would ordinarily create an anonymous "fort.NNN" file for a
hitherto unseen unit.

[klausler]

Rebase to current HEAD.

flang/include/flang/Runtime/iostat.h

enum Iostat {
   IostatBOZInputOverflow,
   IostatIntegerInputOverflow,
   IostatRealInputOverflow,
   IostatOpenAlreadyConnected,
   IostatCannotReposition,
   IostatBadWaitId,
   IostatTooManyAsyncOps,
   IostatBadBackspaceUnit,
+  IostatBadUnitNumber,
 };
 
 const char *IostatErrorString(int);
 
 } // namespace Fortran::runtime::io
 #endif // FORTRAN_RUNTIME_IOSTAT_H_

flang/runtime/io-api.cpp

 Cookie IONAME(BeginInternalFormattedInput)(const char *internal,
     std::size_t internalLength, const char *format, std::size_t formatLength,
     void **scratchArea, std::size_t scratchBytes, const char *sourceFile,
     int sourceLine) {
   return BeginInternalFormattedIO<Direction::Input>(internal, internalLength,
       format, formatLength, scratchArea, scratchBytes, sourceFile, sourceLine);
 }
 
+static ExternalFileUnit *GetOrCreateUnit(int unitNumber, Direction direction,
+    std::optional<bool> isUnformatted, const Terminator &terminator,
+    Cookie &errorCookie) {
+  if (ExternalFileUnit *
+      unit{ExternalFileUnit::LookUpOrCreateAnonymous(
+          unitNumber, direction, isUnformatted, terminator)}) {
+    errorCookie = nullptr;
+    return unit;
+  } else {
+    errorCookie = &New<NoopStatementState>{terminator}(
+        terminator.sourceFileName(), terminator.sourceLine(), unitNumber)
+                       .release()
+                       ->ioStatementState();
+    errorCookie->GetIoErrorHandler().SetPendingError(IostatBadUnitNumber);
+    return nullptr;
+  }
+}
+
 template <Direction DIR, template <Direction> class STATE, typename... A>
 Cookie BeginExternalListIO(
     int unitNumber, const char *sourceFile, int sourceLine, A &&...xs) {
   Terminator terminator{sourceFile, sourceLine};
   if (unitNumber == DefaultUnit) {
     unitNumber = DIR == Direction::Input ? 5 : 6;
   }
-  ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
-      unitNumber, DIR, false /*!unformatted*/, terminator)};
-  if (!unit.isUnformatted.has_value()) {
-    unit.isUnformatted = false;
+  Cookie errorCookie{nullptr};
+  ExternalFileUnit *unit{GetOrCreateUnit(
+      unitNumber, DIR, false /*!unformatted*/, terminator, errorCookie)};
+  if (!unit) {
+    return errorCookie;
+  }
+  if (!unit->isUnformatted.has_value()) {
+    unit->isUnformatted = false;
   }
   Iostat iostat{IostatOk};
-  if (*unit.isUnformatted) {
+  if (*unit->isUnformatted) {
     iostat = IostatFormattedIoOnUnformattedUnit;
   }
-  if (ChildIo * child{unit.GetChildIo()}) {
+  if (ChildIo * child{unit->GetChildIo()}) {
     if (iostat == IostatOk) {
       iostat = child->CheckFormattingAndDirection(false, DIR);
     }
     if (iostat == IostatOk) {
       return &child->BeginIoStatement<ChildListIoStatementState<DIR>>(
           *child, sourceFile, sourceLine);
     } else {
       return &child->BeginIoStatement<ErroneousIoStatementState>(
           iostat, nullptr /* no unit */, sourceFile, sourceLine);
     }
   } else {
-    if (iostat == IostatOk && unit.access == Access::Direct) {
+    if (iostat == IostatOk && unit->access == Access::Direct) {
       iostat = IostatListIoOnDirectAccessUnit;
     }
     if (iostat == IostatOk) {
-      iostat = unit.SetDirection(DIR);
+      iostat = unit->SetDirection(DIR);
     }
     if (iostat == IostatOk) {
-      return &unit.BeginIoStatement<STATE<DIR>>(
-          std::forward<A>(xs)..., unit, sourceFile, sourceLine);
+      return &unit->BeginIoStatement<STATE<DIR>>(
+          std::forward<A>(xs)..., *unit, sourceFile, sourceLine);
     } else {
-      return &unit.BeginIoStatement<ErroneousIoStatementState>(
-          iostat, &unit, sourceFile, sourceLine);
+      return &unit->BeginIoStatement<ErroneousIoStatementState>(
+          iostat, unit, sourceFile, sourceLine);
     }
   }
 }
 
 Cookie IONAME(BeginExternalListOutput)(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   return BeginExternalListIO<Direction::Output, ExternalListIoStatementState>(
       unitNumber, sourceFile, sourceLine);
 }
 
 Cookie IONAME(BeginExternalListInput)(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   return BeginExternalListIO<Direction::Input, ExternalListIoStatementState>(
       unitNumber, sourceFile, sourceLine);
 }
 
 template <Direction DIR>
 Cookie BeginExternalFormattedIO(const char *format, std::size_t formatLength,
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   Terminator terminator{sourceFile, sourceLine};
   if (unitNumber == DefaultUnit) {
     unitNumber = DIR == Direction::Input ? 5 : 6;
   }
-  ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
-      unitNumber, DIR, false /*!unformatted*/, terminator)};
+  Cookie errorCookie{nullptr};
+  ExternalFileUnit *unit{GetOrCreateUnit(
+      unitNumber, DIR, false /*!unformatted*/, terminator, errorCookie)};
+  if (!unit) {
+    return errorCookie;
+  }
   Iostat iostat{IostatOk};
-  if (!unit.isUnformatted.has_value()) {
-    unit.isUnformatted = false;
+  if (!unit->isUnformatted.has_value()) {
+    unit->isUnformatted = false;
   }
-  if (*unit.isUnformatted) {
+  if (*unit->isUnformatted) {
     iostat = IostatFormattedIoOnUnformattedUnit;
   }
-  if (ChildIo * child{unit.GetChildIo()}) {
+  if (ChildIo * child{unit->GetChildIo()}) {
     if (iostat == IostatOk) {
       iostat = child->CheckFormattingAndDirection(false, DIR);
     }
     if (iostat == IostatOk) {
       return &child->BeginIoStatement<ChildFormattedIoStatementState<DIR>>(
           *child, format, formatLength, sourceFile, sourceLine);
     } else {
       return &child->BeginIoStatement<ErroneousIoStatementState>(
           iostat, nullptr /* no unit */, sourceFile, sourceLine);
     }
   } else {
     if (iostat == IostatOk) {
-      iostat = unit.SetDirection(DIR);
+      iostat = unit->SetDirection(DIR);
     }
     if (iostat == IostatOk) {
-      return &unit.BeginIoStatement<ExternalFormattedIoStatementState<DIR>>(
-          unit, format, formatLength, sourceFile, sourceLine);
+      return &unit->BeginIoStatement<ExternalFormattedIoStatementState<DIR>>(
+          *unit, format, formatLength, sourceFile, sourceLine);
     } else {
-      return &unit.BeginIoStatement<ErroneousIoStatementState>(
-          iostat, &unit, sourceFile, sourceLine);
+      return &unit->BeginIoStatement<ErroneousIoStatementState>(
+          iostat, unit, sourceFile, sourceLine);
     }
   }
 }
 
 Cookie IONAME(BeginExternalFormattedOutput)(const char *format,
     std::size_t formatLength, ExternalUnit unitNumber, const char *sourceFile,
     int sourceLine) {
   return BeginExternalFormattedIO<Direction::Output>(
       format, formatLength, unitNumber, sourceFile, sourceLine);
 }
 
 Cookie IONAME(BeginExternalFormattedInput)(const char *format,
     std::size_t formatLength, ExternalUnit unitNumber, const char *sourceFile,
     int sourceLine) {
   return BeginExternalFormattedIO<Direction::Input>(
       format, formatLength, unitNumber, sourceFile, sourceLine);
 }
 
 template <Direction DIR>
 Cookie BeginUnformattedIO(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   Terminator terminator{sourceFile, sourceLine};
-  ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
-      unitNumber, DIR, true /*unformatted*/, terminator)};
+  Cookie errorCookie{nullptr};
+  ExternalFileUnit *unit{GetOrCreateUnit(
+      unitNumber, DIR, true /*unformatted*/, terminator, errorCookie)};
+  if (!unit) {
+    return errorCookie;
+  }
   Iostat iostat{IostatOk};
-  if (!unit.isUnformatted.has_value()) {
-    unit.isUnformatted = true;
+  if (!unit->isUnformatted.has_value()) {
+    unit->isUnformatted = true;
   }
-  if (!*unit.isUnformatted) {
+  if (!*unit->isUnformatted) {
     iostat = IostatUnformattedIoOnFormattedUnit;
   }
-  if (ChildIo * child{unit.GetChildIo()}) {
+  if (ChildIo * child{unit->GetChildIo()}) {
     if (iostat == IostatOk) {
       iostat = child->CheckFormattingAndDirection(true, DIR);
     }
     if (iostat == IostatOk) {
       return &child->BeginIoStatement<ChildUnformattedIoStatementState<DIR>>(
           *child, sourceFile, sourceLine);
     } else {
       return &child->BeginIoStatement<ErroneousIoStatementState>(
           iostat, nullptr /* no unit */, sourceFile, sourceLine);
     }
   } else {
     if (iostat == IostatOk) {
-      iostat = unit.SetDirection(DIR);
+      iostat = unit->SetDirection(DIR);
     }
     if (iostat == IostatOk) {
       IoStatementState &io{
-          unit.BeginIoStatement<ExternalUnformattedIoStatementState<DIR>>(
-              unit, sourceFile, sourceLine)};
+          unit->BeginIoStatement<ExternalUnformattedIoStatementState<DIR>>(
+              *unit, sourceFile, sourceLine)};
       if constexpr (DIR == Direction::Output) {
-        if (unit.access == Access::Sequential) {
+        if (unit->access == Access::Sequential) {
           // Create space for (sub)record header to be completed by
           // ExternalFileUnit::AdvanceRecord()
-          unit.recordLength.reset(); // in case of prior BACKSPACE
+          unit->recordLength.reset(); // in case of prior BACKSPACE
           io.Emit("\0\0\0\0", 4); // placeholder for record length header
         }
       }
       return &io;
     } else {
-      return &unit.BeginIoStatement<ErroneousIoStatementState>(
-          iostat, &unit, sourceFile, sourceLine);
+      return &unit->BeginIoStatement<ErroneousIoStatementState>(
+          iostat, unit, sourceFile, sourceLine);
     }
   }
 }
 
 Cookie IONAME(BeginUnformattedOutput)(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   return BeginUnformattedIO<Direction::Output>(
       unitNumber, sourceFile, sourceLine);
 }
 
 Cookie IONAME(BeginUnformattedInput)(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   return BeginUnformattedIO<Direction::Input>(
       unitNumber, sourceFile, sourceLine);
 }
 
 Cookie IONAME(BeginOpenUnit)( // OPEN(without NEWUNIT=)
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
-  bool wasExtant{false};
   Terminator terminator{sourceFile, sourceLine};
-  ExternalFileUnit &unit{
-      ExternalFileUnit::LookUpOrCreate(unitNumber, terminator, wasExtant)};
-  return &unit.BeginIoStatement<OpenStatementState>(
-      unit, wasExtant, sourceFile, sourceLine);
+  bool wasExtant{false};
+  if (ExternalFileUnit *
+      unit{ExternalFileUnit::LookUpOrCreate(
+          unitNumber, terminator, wasExtant)}) {
+    return &unit->BeginIoStatement<OpenStatementState>(
+        *unit, wasExtant, sourceFile, sourceLine);
+  } else {
+    auto &io{
+        New<NoopStatementState>{terminator}(sourceFile, sourceLine, unitNumber)
+            .release()
+            ->ioStatementState()};
+    io.GetIoErrorHandler().SetPendingError(IostatBadUnitNumber);
+    return &io;
+  }
 }
 
 Cookie IONAME(BeginOpenNewUnit)( // OPEN(NEWUNIT=j)
     const char *sourceFile, int sourceLine) {
   Terminator terminator{sourceFile, sourceLine};
   ExternalFileUnit &unit{
       ExternalFileUnit::NewUnit(terminator, false /*not child I/O*/)};
   return &unit.BeginIoStatement<OpenStatementState>(
if (ExternalFileUnit * unit{ExternalFileUnit::LookUp(unitNumber)}) {
     io.GetIoErrorHandler().SetPendingError(IostatBadBackspaceUnit);
     return &io;
   }
 }
 
 Cookie IONAME(BeginEndfile)(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   Terminator terminator{sourceFile, sourceLine};
-  ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
-      unitNumber, Direction::Output, std::nullopt, terminator)};
-  return &unit.BeginIoStatement<ExternalMiscIoStatementState>(
-      unit, ExternalMiscIoStatementState::Endfile, sourceFile, sourceLine);
+  Cookie errorCookie{nullptr};
+  if (ExternalFileUnit *
+      unit{GetOrCreateUnit(unitNumber, Direction::Output, std::nullopt,
+          terminator, errorCookie)}) {
+    return &unit->BeginIoStatement<ExternalMiscIoStatementState>(
+        *unit, ExternalMiscIoStatementState::Endfile, sourceFile, sourceLine);
+  } else {
+    return errorCookie;
+  }
 }
 
 Cookie IONAME(BeginRewind)(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   Terminator terminator{sourceFile, sourceLine};
-  ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
-      unitNumber, Direction::Input, std::nullopt, terminator)};
-  return &unit.BeginIoStatement<ExternalMiscIoStatementState>(
-      unit, ExternalMiscIoStatementState::Rewind, sourceFile, sourceLine);
+  Cookie errorCookie{nullptr};
+  if (ExternalFileUnit *
+      unit{GetOrCreateUnit(unitNumber, Direction::Input, std::nullopt,
+          terminator, errorCookie)}) {
+    return &unit->BeginIoStatement<ExternalMiscIoStatementState>(
+        *unit, ExternalMiscIoStatementState::Rewind, sourceFile, sourceLine);
+  } else {
+    return errorCookie;
+  }
 }
 
 Cookie IONAME(BeginInquireUnit)(
     ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
   if (ExternalFileUnit * unit{ExternalFileUnit::LookUp(unitNumber)}) {
     if (ChildIo * child{unit->GetChildIo()}) {
       return &child->BeginIoStatement<InquireUnitState>(
           *unit, sourceFile, sourceLine);

flang/runtime/iostat.cpp

const char *IostatErrorString(int iostat) {
   case IostatOpenAlreadyConnected:
     return "OPEN of file already connected to another unit";
   case IostatBadWaitId:
     return "WAIT(ID=nonzero) for an ID value that is not a pending operation";
   case IostatTooManyAsyncOps:
     return "Too many asynchronous operations pending on unit";
   case IostatBadBackspaceUnit:
     return "BACKSPACE on unconnected unit";
+  case IostatBadUnitNumber:
+    return "Negative unit number is not allowed";
   default:
     return nullptr;
   }
 }
 
 } // namespace Fortran::runtime::io

flang/runtime/unit-map.h

 
 class UnitMap {
 public:
   ExternalFileUnit *LookUp(int n) {
     CriticalSection critical{lock_};
     return Find(n);
   }
 
-  ExternalFileUnit &LookUpOrCreate(
+  ExternalFileUnit *LookUpOrCreate(
       int n, const Terminator &terminator, bool &wasExtant) {
     CriticalSection critical{lock_};
-    auto *p{Find(n)};
-    wasExtant = p != nullptr;
-    return p ? *p : Create(n, terminator);
+    if (auto *p{Find(n)}) {
+      wasExtant = true;
+      return p;
+    } else {
+      wasExtant = false;
+      return n >= 0 ? &Create(n, terminator) : nullptr;
+    }
   }
 
   // Unit look-up by name is needed for INQUIRE(FILE="...")
   ExternalFileUnit *LookUp(const char *path, std::size_t pathLen) {
     CriticalSection critical{lock_};
     return Find(path, pathLen);
   }
 

flang/runtime/unit.h

public:
   }
   ~ExternalFileUnit() {}
 
   int unitNumber() const { return unitNumber_; }
   bool swapEndianness() const { return swapEndianness_; }
   bool createdForInternalChildIo() const { return createdForInternalChildIo_; }
 
   static ExternalFileUnit *LookUp(int unit);
-  static ExternalFileUnit &LookUpOrCreate(
+  static ExternalFileUnit *LookUpOrCreate(
       int unit, const Terminator &, bool &wasExtant);
-  static ExternalFileUnit &LookUpOrCreateAnonymous(int unit, Direction,
+  static ExternalFileUnit *LookUpOrCreateAnonymous(int unit, Direction,
       std::optional<bool> isUnformatted, const Terminator &);
   static ExternalFileUnit *LookUp(const char *path, std::size_t pathLen);
   static ExternalFileUnit &CreateNew(int unit, const Terminator &);
   static ExternalFileUnit *LookUpForClose(int unit);
   static ExternalFileUnit &NewUnit(const Terminator &, bool forChildIo);
   static void CloseAll(IoErrorHandler &);
   static void FlushAll(IoErrorHandler &);
 

flang/runtime/unit.cpp

if (errorOutput) {
     errorOutput->FlushOutput(handler);
   }
 }
 
 ExternalFileUnit *ExternalFileUnit::LookUp(int unit) {
   return GetUnitMap().LookUp(unit);
 }
 
-ExternalFileUnit &ExternalFileUnit::LookUpOrCreate(
+ExternalFileUnit *ExternalFileUnit::LookUpOrCreate(
     int unit, const Terminator &terminator, bool &wasExtant) {
   return GetUnitMap().LookUpOrCreate(unit, terminator, wasExtant);
 }
 
-ExternalFileUnit &ExternalFileUnit::LookUpOrCreateAnonymous(int unit,
+ExternalFileUnit *ExternalFileUnit::LookUpOrCreateAnonymous(int unit,
     Direction dir, std::optional<bool> isUnformatted,
     const Terminator &terminator) {
   bool exists{false};
-  ExternalFileUnit &result{
+  ExternalFileUnit *result{
       GetUnitMap().LookUpOrCreate(unit, terminator, exists)};
-  if (!exists) {
+  if (result && !exists) {
     IoErrorHandler handler{terminator};
-    result.OpenAnonymousUnit(
+    result->OpenAnonymousUnit(
         dir == Direction::Input ? OpenStatus::Unknown : OpenStatus::Replace,
         Action::ReadWrite, Position::Rewind, Convert::Native, handler);
-    result.isUnformatted = isUnformatted;
+    result->isUnformatted = isUnformatted;
   }
   return result;
 }
 
 ExternalFileUnit *ExternalFileUnit::LookUp(
     const char *path, std::size_t pathLen) {
   return GetUnitMap().LookUp(path, pathLen);
 }
 
 ExternalFileUnit &ExternalFileUnit::CreateNew(
     int unit, const Terminator &terminator) {
   bool wasExtant{false};
-  ExternalFileUnit &result{
+  ExternalFileUnit *result{
       GetUnitMap().LookUpOrCreate(unit, terminator, wasExtant)};
-  RUNTIME_CHECK(terminator, !wasExtant);
-  return result;
+  RUNTIME_CHECK(terminator, result && !wasExtant);
+  return *result;
 }
 
 ExternalFileUnit *ExternalFileUnit::LookUpForClose(int unit) {
   return GetUnitMap().LookUpForClose(unit);
 }
 
 ExternalFileUnit &ExternalFileUnit::NewUnit(
     const Terminator &terminator, bool forChildIo) {
UnitMap &ExternalFileUnit::GetUnitMap() {
   if (unitMap) {
     return *unitMap;
   }
   Terminator terminator{__FILE__, __LINE__};
   IoErrorHandler handler{terminator};
   UnitMap *newUnitMap{New<UnitMap>{terminator}().release()};
 
   bool wasExtant{false};
-  ExternalFileUnit &out{newUnitMap->LookUpOrCreate(6, terminator, wasExtant)};
+  ExternalFileUnit &out{*newUnitMap->LookUpOrCreate(6, terminator, wasExtant)};
   RUNTIME_CHECK(terminator, !wasExtant);
   out.Predefine(1);
   handler.SignalError(out.SetDirection(Direction::Output));
   out.isUnformatted = false;
   defaultOutput = &out;
 
-  ExternalFileUnit &in{newUnitMap->LookUpOrCreate(5, terminator, wasExtant)};
+  ExternalFileUnit &in{*newUnitMap->LookUpOrCreate(5, terminator, wasExtant)};
   RUNTIME_CHECK(terminator, !wasExtant);
   in.Predefine(0);
   handler.SignalError(in.SetDirection(Direction::Input));
   in.isUnformatted = false;
   defaultInput = &in;
 
-  ExternalFileUnit &error{newUnitMap->LookUpOrCreate(0, terminator, wasExtant)};
+  ExternalFileUnit &error{
+      *newUnitMap->LookUpOrCreate(0, terminator, wasExtant)};
   RUNTIME_CHECK(terminator, !wasExtant);
   error.Predefine(2);
   handler.SignalError(error.SetDirection(Direction::Output));
   error.isUnformatted = false;
   errorOutput = &error;
 
   unitMap = newUnitMap;
   return *unitMap;