[flang] Rework read/write permission management for runtime file opening

Anonymous Fortran unit files (e.g., "./fort.7") need to be created
O_RDWR so that they can be written, rewound, and read.  Other
files opened with no ACTION= specifier need to set read/write
permissions based on the file, if it exists.

Reviewed By: sscalpone

Differential Revision: https://reviews.llvm.org/D84063

flang/runtime/file.cpp

@@ -57,63 +57,86 @@ static int openfile_mkstemp(IoErrorHandler &handler) {
   return fd;
 }
 
-void OpenFile::Open(
-    OpenStatus status, Position position, IoErrorHandler &handler) {
-  int flags{mayRead_ ? mayWrite_ ? O_RDWR : O_RDONLY : O_WRONLY};
-  switch (status) {
-  case OpenStatus::Old:
-    if (fd_ >= 0) {
-      return;
+void OpenFile::Open(OpenStatus status, std::optional<Action> action,
+    Position position, IoErrorHandler &handler) {
+  if (fd_ >= 0 &&
+      (status == OpenStatus::Old || status == OpenStatus::Unknown)) {
+    return;
+  }
+  if (fd_ >= 0) {
+    if (fd_ <= 2) {
+      // don't actually close a standard file descriptor, we might need it
+    } else {
+      if (::close(fd_) != 0) {
+        handler.SignalErrno();
+      }
     }
-    knownSize_.reset();
-    break;
-  case OpenStatus::New:
-    flags |= O_CREAT | O_EXCL;
-    knownSize_ = 0;
-    break;
-  case OpenStatus::Scratch:
+    fd_ = -1;
+  }
+  if (status == OpenStatus::Scratch) {
     if (path_.get()) {
       handler.SignalError("FILE= must not appear with STATUS='SCRATCH'");
       path_.reset();
     }
+    if (!action) {
+      action = Action::ReadWrite;
+    }
     fd_ = openfile_mkstemp(handler);
-    knownSize_ = 0;
-    return;
-  case OpenStatus::Replace:
-    flags |= O_CREAT | O_TRUNC;
-    knownSize_ = 0;
-    break;
-  case OpenStatus::Unknown:
-    if (fd_ >= 0) {
+  } else {
+    if (!path_.get()) {
+      handler.SignalError(
+          "FILE= is required unless STATUS='OLD' and unit is connected");
       return;
     }
-    flags |= O_CREAT;
-    knownSize_.reset();
-    break;
-  }
-  // If we reach this point, we're opening a new file.
-  // TODO: Fortran shouldn't create a new file until the first WRITE.
-  if (fd_ >= 0) {
-    if (fd_ <= 2) {
-      // don't actually close a standard file descriptor, we might need it
-    } else if (::close(fd_) != 0) {
-      handler.SignalErrno();
+    int flags{0};
+    if (status != OpenStatus::Old) {
+      flags |= O_CREAT;
+    }
+    if (status == OpenStatus::New) {
+      flags |= O_EXCL;
+    } else if (status == OpenStatus::Replace) {
+      flags |= O_TRUNC;
+    }
+    if (!action) {
+      // Try to open read/write, back off to read-only on failure
+      fd_ = ::open(path_.get(), flags | O_RDWR, 0600);
+      if (fd_ >= 0) {
+        action = Action::ReadWrite;
+      } else {
+        action = Action::Read;
+      }
+    }
+    if (fd_ < 0) {
+      switch (*action) {
+      case Action::Read:
+        flags |= O_RDONLY;
+        break;
+      case Action::Write:
+        flags |= O_WRONLY;
+        break;
+      case Action::ReadWrite:
+        flags |= O_RDWR;
+        break;
+      }
+      fd_ = ::open(path_.get(), flags, 0600);
+      if (fd_ < 0) {
+        handler.SignalErrno();
+      }
     }
   }
-  if (!path_.get()) {
-    handler.SignalError(
-        "FILE= is required unless STATUS='OLD' and unit is connected");
-    return;
-  }
-  fd_ = ::open(path_.get(), flags, 0600);
-  if (fd_ < 0) {
-    handler.SignalErrno();
-  }
+  RUNTIME_CHECK(handler, action.has_value());
   pending_.reset();
   if (position == Position::Append && !RawSeekToEnd()) {
     handler.SignalErrno();
   }
   isTerminal_ = ::isatty(fd_) == 1;
+  mayRead_ = *action != Action::Write;
+  mayWrite_ = *action != Action::Read;
+  if (status == OpenStatus::Old || status == OpenStatus::Unknown) {
+    knownSize_.reset();
+  } else {
+    knownSize_ = 0;
+  }
 }
 
 void OpenFile::Predefine(int fd) {
@@ -124,6 +147,9 @@ void OpenFile::Predefine(int fd) {
   knownSize_.reset();
   nextId_ = 0;
   pending_.reset();
+  mayRead_ = fd == 0;
+  mayWrite_ = fd != 0;
+  mayPosition_ = false;
 }
 
 void OpenFile::Close(CloseStatus status, IoErrorHandler &handler) {

flang/runtime/file.h

@@ -21,6 +21,7 @@ namespace Fortran::runtime::io {
 enum class OpenStatus { Old, New, Scratch, Replace, Unknown };
 enum class CloseStatus { Keep, Delete };
 enum class Position { AsIs, Rewind, Append };
+enum class Action { Read, Write, ReadWrite };
 
 class OpenFile {
 public:
@@ -30,19 +31,16 @@ public:
   void set_path(OwningPtr<char> &&, std::size_t bytes);
   std::size_t pathLength() const { return pathLength_; }
   bool mayRead() const { return mayRead_; }
-  void set_mayRead(bool yes) { mayRead_ = yes; }
   bool mayWrite() const { return mayWrite_; }
-  void set_mayWrite(bool yes) { mayWrite_ = yes; }
+  bool mayPosition() const { return mayPosition_; }
   bool mayAsynchronous() const { return mayAsynchronous_; }
   void set_mayAsynchronous(bool yes) { mayAsynchronous_ = yes; }
-  bool mayPosition() const { return mayPosition_; }
-  void set_mayPosition(bool yes) { mayPosition_ = yes; }
   FileOffset position() const { return position_; }
   bool isTerminal() const { return isTerminal_; }
   std::optional<FileOffset> knownSize() const { return knownSize_; }
 
   bool IsOpen() const { return fd_ >= 0; }
-  void Open(OpenStatus, Position, IoErrorHandler &);
+  void Open(OpenStatus, std::optional<Action>, Position, IoErrorHandler &);
   void Predefine(int fd);
   void Close(CloseStatus, IoErrorHandler &);
 

flang/runtime/io-api.cpp

@@ -563,31 +563,31 @@ bool IONAME(SetAction)(Cookie cookie, const char *keyword, std::size_t length) {
     io.GetIoErrorHandler().Crash(
         "SetAction() called when not in an OPEN statement");
   }
-  bool mayRead{true};
-  bool mayWrite{true};
+  std::optional<Action> action;
   static const char *keywords[]{"READ", "WRITE", "READWRITE", nullptr};
   switch (IdentifyValue(keyword, length, keywords)) {
   case 0:
-    mayWrite = false;
+    action = Action::Read;
     break;
   case 1:
-    mayRead = false;
+    action = Action::Write;
     break;
   case 2:
+    action = Action::ReadWrite;
     break;
   default:
     open->SignalError(IostatErrorInKeyword, "Invalid ACTION='%.*s'",
         static_cast<int>(length), keyword);
     return false;
   }
-  if (mayRead != open->unit().mayRead() ||
-      mayWrite != open->unit().mayWrite()) {
-    if (open->wasExtant()) {
+  RUNTIME_CHECK(io.GetIoErrorHandler(), action.has_value());
+  if (open->wasExtant()) {
+    if ((*action != Action::Write) != open->unit().mayRead() ||
+        (*action != Action::Read) != open->unit().mayWrite()) {
       open->SignalError("ACTION= may not be changed on an open unit");
     }
-    open->unit().set_mayRead(mayRead);
-    open->unit().set_mayWrite(mayWrite);
   }
+  open->set_action(*action);
   return true;
 }
 

flang/runtime/io-stmt.cpp

@@ -165,7 +165,8 @@ int OpenStatementState::EndIoStatement() {
   if (wasExtant_ && status_ != OpenStatus::Old) {
     SignalError("OPEN statement for connected unit must have STATUS='OLD'");
   }
-  unit().OpenUnit(status_, position_, std::move(path_), pathLength_, *this);
+  unit().OpenUnit(
+      status_, action_, position_, std::move(path_), pathLength_, *this);
   return ExternalIoStatementBase::EndIoStatement();
 }
 

flang/runtime/io-stmt.h

@@ -294,15 +294,17 @@ public:
       : ExternalIoStatementBase{unit, sourceFile, sourceLine}, wasExtant_{
                                                                    wasExtant} {}
   bool wasExtant() const { return wasExtant_; }
-  void set_status(OpenStatus status) { status_ = status; }
+  void set_status(OpenStatus status) { status_ = status; } // STATUS=
   void set_path(const char *, std::size_t, int kind); // FILE=
   void set_position(Position position) { position_ = position; } // POSITION=
+  void set_action(Action action) { action_ = action; } // ACTION=
   int EndIoStatement();
 
 private:
   bool wasExtant_;
   OpenStatus status_{OpenStatus::Unknown};
   Position position_{Position::AsIs};
+  std::optional<Action> action_;
   OwningPtr<char> path_;
   std::size_t pathLength_;
 };

flang/runtime/unit.cpp

@@ -64,7 +64,8 @@ ExternalFileUnit &ExternalFileUnit::LookUpOrCreateAnonymous(
     IoErrorHandler handler{terminator};
     result.OpenUnit(
         dir == Direction::Input ? OpenStatus::Old : OpenStatus::Replace,
-        Position::Rewind, std::move(path), std::strlen(path.get()), handler);
+        Action::ReadWrite, Position::Rewind, std::move(path),
+        std::strlen(path.get()), handler);
     result.isUnformatted = isUnformatted;
   }
   return result;
@@ -87,8 +88,8 @@ int ExternalFileUnit::NewUnit(const Terminator &terminator) {
   return GetUnitMap().NewUnit(terminator).unitNumber();
 }
 
-void ExternalFileUnit::OpenUnit(OpenStatus status, Position position,
-    OwningPtr<char> &&newPath, std::size_t newPathLength,
+void ExternalFileUnit::OpenUnit(OpenStatus status, std::optional<Action> action,
+    Position position, OwningPtr<char> &&newPath, std::size_t newPathLength,
     IoErrorHandler &handler) {
   if (IsOpen()) {
     if (status == OpenStatus::Old &&
@@ -105,7 +106,7 @@ void ExternalFileUnit::OpenUnit(OpenStatus status, Position position,
     Close(CloseStatus::Keep, handler);
   }
   set_path(std::move(newPath), newPathLength);
-  Open(status, position, handler);
+  Open(status, action, position, handler);
   auto totalBytes{knownSize()};
   if (access == Access::Direct) {
     if (!isFixedRecordLength || !recordLength) {
@@ -186,16 +187,10 @@ UnitMap &ExternalFileUnit::GetUnitMap() {
   unitMap = New<UnitMap>{terminator}().release();
   ExternalFileUnit &out{ExternalFileUnit::CreateNew(6, terminator)};
   out.Predefine(1);
-  out.set_mayRead(false);
-  out.set_mayWrite(true);
-  out.set_mayPosition(false);
   out.SetDirection(Direction::Output, handler);
   defaultOutput = &out;
   ExternalFileUnit &in{ExternalFileUnit::CreateNew(5, terminator)};
   in.Predefine(0);
-  in.set_mayRead(true);
-  in.set_mayWrite(false);
-  in.set_mayPosition(false);
   in.SetDirection(Direction::Input, handler);
   defaultInput = &in;
   // TODO: Set UTF-8 mode from the environment

flang/runtime/unit.h

@@ -48,8 +48,8 @@ public:
   static void CloseAll(IoErrorHandler &);
   static void FlushAll(IoErrorHandler &);
 
-  void OpenUnit(OpenStatus, Position, OwningPtr<char> &&path,
-      std::size_t pathLength, IoErrorHandler &);
+  void OpenUnit(OpenStatus, std::optional<Action>, Position,
+      OwningPtr<char> &&path, std::size_t pathLength, IoErrorHandler &);
   void CloseUnit(CloseStatus, IoErrorHandler &);
   void DestroyClosed();