From df38f35acb56b63df23d8645e0f8d791cfe74c83 Mon Sep 17 00:00:00 2001 From: Peter Klausler Date: Wed, 16 Feb 2022 14:55:19 -0800 Subject: [PATCH] [flang] Allow data transfer stmt control list errors to be caught The runtime crashes on several fundamental I/O data transfer statement control list errors, like list I/O on a direct-access unit, or input from a write-only unit, &c. These errors should not be fatal when ERR= or IOSTAT= are present. This patch creates a new ErroneousIoStatementState class and uses it for the state of an I/O statement that is doomed to fail from these errors. If there is no ERR= label or IOSTAT= variable, the error will be raised at the end of the statement. Data transfer operations along the way will be no-op failures. Differential Revision: https://reviews.llvm.org/D120745 --- flang/include/flang/Runtime/iostat.h | 7 ++ flang/runtime/io-api.cpp | 133 ++++++++++++++++----------- flang/runtime/io-stmt.cpp | 7 +- flang/runtime/io-stmt.h | 18 +++- flang/runtime/iostat.cpp | 14 +++ flang/runtime/unit.cpp | 37 +++----- flang/runtime/unit.h | 10 +- 7 files changed, 144 insertions(+), 82 deletions(-) diff --git a/flang/include/flang/Runtime/iostat.h b/flang/include/flang/Runtime/iostat.h index 07a25c420425b..06d6fe7ae3dd4 100644 --- a/flang/include/flang/Runtime/iostat.h +++ b/flang/include/flang/Runtime/iostat.h @@ -56,6 +56,13 @@ enum Iostat { IostatBackspaceAtFirstRecord, IostatRewindNonSequential, IostatWriteAfterEndfile, + IostatFormattedIoOnUnformattedUnit, + IostatUnformattedIoOnFormattedUnit, + IostatListIoOnDirectAccessUnit, + IostatUnformattedChildOnFormattedParent, + IostatFormattedChildOnUnformattedParent, + IostatChildInputFromOutputParent, + IostatChildOutputToInputParent, }; const char *IostatErrorString(int); diff --git a/flang/runtime/io-api.cpp b/flang/runtime/io-api.cpp index 75695af5a1a12..e097867518c73 100644 --- a/flang/runtime/io-api.cpp +++ b/flang/runtime/io-api.cpp @@ -148,8 +148,8 @@ Cookie IONAME(BeginInternalFormattedInput)(const char *internal, } template class STATE, typename... A> -Cookie BeginExternalListIO(const char *what, int unitNumber, - const char *sourceFile, int sourceLine, A &&...xs) { +Cookie BeginExternalListIO( + int unitNumber, const char *sourceFile, int sourceLine, A &&...xs) { Terminator terminator{sourceFile, sourceLine}; if (unitNumber == DefaultUnit) { unitNumber = DIR == Direction::Input ? 5 : 6; @@ -159,38 +159,48 @@ Cookie BeginExternalListIO(const char *what, int unitNumber, if (!unit.isUnformatted.has_value()) { unit.isUnformatted = false; } + Iostat iostat{IostatOk}; if (*unit.isUnformatted) { - terminator.Crash("%s attempted on unformatted file", what); - return nullptr; + iostat = IostatFormattedIoOnUnformattedUnit; } if (ChildIo * child{unit.GetChildIo()}) { - return child->CheckFormattingAndDirection(terminator, what, false, DIR) - ? &child->BeginIoStatement>( - *child, sourceFile, sourceLine) - : nullptr; + if (iostat == IostatOk) { + iostat = child->CheckFormattingAndDirection(false, DIR); + } + if (iostat == IostatOk) { + return &child->BeginIoStatement>( + *child, sourceFile, sourceLine); + } else { + return &child->BeginIoStatement( + iostat, sourceFile, sourceLine); + } } else { - if (unit.access == Access::Direct) { - terminator.Crash("%s attempted on direct access file", what); - return nullptr; + if (iostat == IostatOk && unit.access == Access::Direct) { + iostat = IostatListIoOnDirectAccessUnit; + } + if (iostat == IostatOk) { + iostat = unit.SetDirection(DIR); + } + if (iostat == IostatOk) { + return &unit.BeginIoStatement>( + std::forward(xs)..., unit, sourceFile, sourceLine); + } else { + return &unit.BeginIoStatement( + iostat, sourceFile, sourceLine); } - IoErrorHandler handler{terminator}; - unit.SetDirection(DIR, handler); - IoStatementState &io{unit.BeginIoStatement>( - std::forward(xs)..., unit, sourceFile, sourceLine)}; - return &io; } } Cookie IONAME(BeginExternalListOutput)( ExternalUnit unitNumber, const char *sourceFile, int sourceLine) { return BeginExternalListIO( - "List-directed output", unitNumber, sourceFile, sourceLine); + unitNumber, sourceFile, sourceLine); } Cookie IONAME(BeginExternalListInput)( ExternalUnit unitNumber, const char *sourceFile, int sourceLine) { return BeginExternalListIO( - "List-directed input", unitNumber, sourceFile, sourceLine); + unitNumber, sourceFile, sourceLine); } template @@ -202,28 +212,35 @@ Cookie BeginExternalFormattedIO(const char *format, std::size_t formatLength, } ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous( unitNumber, DIR, false /*!unformatted*/, terminator)}; + Iostat iostat{IostatOk}; if (!unit.isUnformatted.has_value()) { unit.isUnformatted = false; } if (*unit.isUnformatted) { - terminator.Crash("Formatted I/O attempted on unformatted file"); - return nullptr; + iostat = IostatFormattedIoOnUnformattedUnit; } if (ChildIo * child{unit.GetChildIo()}) { - return child->CheckFormattingAndDirection(terminator, - DIR == Direction::Output ? "formatted output" - : "formatted input", - false, DIR) - ? &child->BeginIoStatement>( - *child, format, formatLength, sourceFile, sourceLine) - : nullptr; + if (iostat == IostatOk) { + iostat = child->CheckFormattingAndDirection(false, DIR); + } + if (iostat == IostatOk) { + return &child->BeginIoStatement>( + *child, format, formatLength, sourceFile, sourceLine); + } else { + return &child->BeginIoStatement( + iostat, sourceFile, sourceLine); + } } else { - IoErrorHandler handler{terminator}; - unit.SetDirection(DIR, handler); - IoStatementState &io{ - unit.BeginIoStatement>( - unit, format, formatLength, sourceFile, sourceLine)}; - return &io; + if (iostat == IostatOk) { + iostat = unit.SetDirection(DIR); + } + if (iostat == IostatOk) { + return &unit.BeginIoStatement>( + unit, format, formatLength, sourceFile, sourceLine); + } else { + return &unit.BeginIoStatement( + iostat, sourceFile, sourceLine); + } } } @@ -247,35 +264,45 @@ Cookie BeginUnformattedIO( Terminator terminator{sourceFile, sourceLine}; ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous( unitNumber, DIR, true /*unformatted*/, terminator)}; + Iostat iostat{IostatOk}; if (!unit.isUnformatted.has_value()) { unit.isUnformatted = true; } if (!*unit.isUnformatted) { - terminator.Crash("Unformatted I/O attempted on formatted file"); + iostat = IostatUnformattedIoOnFormattedUnit; } if (ChildIo * child{unit.GetChildIo()}) { - return child->CheckFormattingAndDirection(terminator, - DIR == Direction::Output ? "unformatted output" - : "unformatted input", - true, DIR) - ? &child->BeginIoStatement>( - *child, sourceFile, sourceLine) - : nullptr; + if (iostat == IostatOk) { + iostat = child->CheckFormattingAndDirection(true, DIR); + } + if (iostat == IostatOk) { + return &child->BeginIoStatement>( + *child, sourceFile, sourceLine); + } else { + return &child->BeginIoStatement( + iostat, sourceFile, sourceLine); + } } else { - IoStatementState &io{ - unit.BeginIoStatement>( - unit, sourceFile, sourceLine)}; - IoErrorHandler handler{terminator}; - unit.SetDirection(DIR, handler); - if constexpr (DIR == Direction::Output) { - 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 - io.Emit("\0\0\0\0", 4); // placeholder for record length header + if (iostat == IostatOk) { + iostat = unit.SetDirection(DIR); + } + if (iostat == IostatOk) { + IoStatementState &io{ + unit.BeginIoStatement>( + unit, sourceFile, sourceLine)}; + if constexpr (DIR == Direction::Output) { + 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 + io.Emit("\0\0\0\0", 4); // placeholder for record length header + } } + return &io; + } else { + return &unit.BeginIoStatement( + iostat, sourceFile, sourceLine); } - return &io; } } diff --git a/flang/runtime/io-stmt.cpp b/flang/runtime/io-stmt.cpp index 7d447bafa880e..4f47e36fe78f7 100644 --- a/flang/runtime/io-stmt.cpp +++ b/flang/runtime/io-stmt.cpp @@ -1130,7 +1130,7 @@ bool InquireUnitState::Inquire( } else if (unit().openRecl) { result = *unit().openRecl; } else { - result = std::numeric_limits::max(); + result = std::numeric_limits::max(); } return true; case HashInquiryKeyword("SIZE"): @@ -1360,4 +1360,9 @@ bool InquireIOLengthState::Emit(const char32_t *p, std::size_t n) { return true; } +int ErroneousIoStatementState::EndIoStatement() { + SignalError(iostat_); + return IoStatementBase::EndIoStatement(); +} + } // namespace Fortran::runtime::io diff --git a/flang/runtime/io-stmt.h b/flang/runtime/io-stmt.h index 59d4e50460fba..75cf327f05d7c 100644 --- a/flang/runtime/io-stmt.h +++ b/flang/runtime/io-stmt.h @@ -35,6 +35,7 @@ class InquireIOLengthState; class ExternalMiscIoStatementState; class CloseStatementState; class NoopStatementState; // CLOSE or FLUSH on unknown unit +class ErroneousIoStatementState; template class InternalFormattedIoStatementState; @@ -223,7 +224,8 @@ class IoStatementState { std::reference_wrapper, std::reference_wrapper, std::reference_wrapper, - std::reference_wrapper> + std::reference_wrapper, + std::reference_wrapper> u_; }; @@ -674,5 +676,19 @@ class ExternalMiscIoStatementState : public ExternalIoStatementBase { Which which_; }; +class ErroneousIoStatementState : public IoStatementBase { +public: + explicit ErroneousIoStatementState( + Iostat iostat, const char *sourceFile = nullptr, int sourceLine = 0) + : IoStatementBase{sourceFile, sourceLine}, iostat_{iostat} {} + int EndIoStatement(); + ConnectionState &GetConnectionState() { return connection_; } + MutableModes &mutableModes() { return connection_.modes; } + +private: + Iostat iostat_; + ConnectionState connection_; +}; + } // namespace Fortran::runtime::io #endif // FORTRAN_RUNTIME_IO_STMT_H_ diff --git a/flang/runtime/iostat.cpp b/flang/runtime/iostat.cpp index 58e3f1b387e14..f1d20c1a23b0b 100644 --- a/flang/runtime/iostat.cpp +++ b/flang/runtime/iostat.cpp @@ -55,6 +55,20 @@ const char *IostatErrorString(int iostat) { return "REWIND on non-sequential file"; case IostatWriteAfterEndfile: return "WRITE after ENDFILE"; + case IostatFormattedIoOnUnformattedUnit: + return "Formatted I/O on unformatted file"; + case IostatUnformattedIoOnFormattedUnit: + return "Unformatted I/O on formatted file"; + case IostatListIoOnDirectAccessUnit: + return "List-directed or NAMELIST I/O on direct-access file"; + case IostatUnformattedChildOnFormattedParent: + return "Unformatted child I/O on formatted parent unit"; + case IostatFormattedChildOnUnformattedParent: + return "Formatted child I/O on unformatted parent unit"; + case IostatChildInputFromOutputParent: + return "Child input from output parent unit"; + case IostatChildOutputToInputParent: + return "Child output to input parent unit"; default: return nullptr; } diff --git a/flang/runtime/unit.cpp b/flang/runtime/unit.cpp index b74b3ebf4178d..971909b74e769 100644 --- a/flang/runtime/unit.cpp +++ b/flang/runtime/unit.cpp @@ -181,25 +181,20 @@ void ExternalFileUnit::DestroyClosed() { GetUnitMap().DestroyClosed(*this); // destroys *this } -bool ExternalFileUnit::SetDirection( - Direction direction, IoErrorHandler &handler) { +Iostat ExternalFileUnit::SetDirection(Direction direction) { if (direction == Direction::Input) { if (mayRead()) { direction_ = Direction::Input; - return true; + return IostatOk; } else { - handler.SignalError(IostatReadFromWriteOnly, - "READ(UNIT=%d) with ACTION='WRITE'", unitNumber()); - return false; + return IostatReadFromWriteOnly; } } else { if (mayWrite()) { direction_ = Direction::Output; - return true; + return IostatOk; } else { - handler.SignalError(IostatWriteToReadOnly, - "WRITE(UNIT=%d) with ACTION='READ'", unitNumber()); - return false; + return IostatWriteToReadOnly; } } } @@ -220,21 +215,21 @@ UnitMap &ExternalFileUnit::GetUnitMap() { ExternalFileUnit &out{newUnitMap->LookUpOrCreate(6, terminator, wasExtant)}; RUNTIME_CHECK(terminator, !wasExtant); out.Predefine(1); - out.SetDirection(Direction::Output, handler); + handler.SignalError(out.SetDirection(Direction::Output)); out.isUnformatted = false; defaultOutput = &out; ExternalFileUnit &in{newUnitMap->LookUpOrCreate(5, terminator, wasExtant)}; RUNTIME_CHECK(terminator, !wasExtant); in.Predefine(0); - in.SetDirection(Direction::Input, handler); + handler.SignalError(in.SetDirection(Direction::Input)); in.isUnformatted = false; defaultInput = ∈ ExternalFileUnit &error{newUnitMap->LookUpOrCreate(0, terminator, wasExtant)}; RUNTIME_CHECK(terminator, !wasExtant); error.Predefine(2); - error.SetDirection(Direction::Output, handler); + handler.SignalError(error.SetDirection(Direction::Output)); error.isUnformatted = false; errorOutput = &error; @@ -872,8 +867,8 @@ void ChildIo::EndIoStatement() { u_.emplace(); } -bool ChildIo::CheckFormattingAndDirection(Terminator &terminator, - const char *what, bool unformatted, Direction direction) { +Iostat ChildIo::CheckFormattingAndDirection( + bool unformatted, Direction direction) { bool parentIsInput{!parent_.get_if>()}; bool parentIsFormatted{parentIsInput ? parent_.get_if>() != @@ -882,15 +877,13 @@ bool ChildIo::CheckFormattingAndDirection(Terminator &terminator, nullptr}; bool parentIsUnformatted{!parentIsFormatted}; if (unformatted != parentIsUnformatted) { - terminator.Crash("Child %s attempted on %s parent I/O unit", what, - parentIsUnformatted ? "unformatted" : "formatted"); - return false; + return unformatted ? IostatUnformattedChildOnFormattedParent + : IostatFormattedChildOnUnformattedParent; } else if (parentIsInput != (direction == Direction::Input)) { - terminator.Crash("Child %s attempted on %s parent I/O unit", what, - parentIsInput ? "input" : "output"); - return false; + return parentIsInput ? IostatChildOutputToInputParent + : IostatChildInputFromOutputParent; } else { - return true; + return IostatOk; } } diff --git a/flang/runtime/unit.h b/flang/runtime/unit.h index da6bc4b23d710..c8e65c1f0c4c6 100644 --- a/flang/runtime/unit.h +++ b/flang/runtime/unit.h @@ -62,7 +62,7 @@ class ExternalFileUnit : public ConnectionState, void CloseUnit(CloseStatus, IoErrorHandler &); void DestroyClosed(); - bool SetDirection(Direction, IoErrorHandler &); + Iostat SetDirection(Direction); template IoStatementState &BeginIoStatement(X &&...xs) { @@ -128,7 +128,7 @@ class ExternalFileUnit : public ConnectionState, ExternalListIoStatementState, ExternalUnformattedIoStatementState, ExternalUnformattedIoStatementState, InquireUnitState, - ExternalMiscIoStatementState> + ExternalMiscIoStatementState, ErroneousIoStatementState> u_; // Points to the active alternative (if any) in u_ for use as a Cookie @@ -171,8 +171,7 @@ class ChildIo { OwningPtr AcquirePrevious() { return std::move(previous_); } - bool CheckFormattingAndDirection( - Terminator &, const char *what, bool unformatted, Direction); + Iostat CheckFormattingAndDirection(bool unformatted, Direction); private: IoStatementState &parent_; @@ -183,7 +182,8 @@ class ChildIo { ChildListIoStatementState, ChildListIoStatementState, ChildUnformattedIoStatementState, - ChildUnformattedIoStatementState, InquireUnitState> + ChildUnformattedIoStatementState, InquireUnitState, + ErroneousIoStatementState> u_; std::optional io_; };