Skip to content

Commit 8db4dc8

Browse files
klauslerklausler
committed
[flang] Error recovery improvement in runtime (IOMSG=)
Some refactoring and related fixes for more accurate user program error recovery in the I/O runtime, especially for error recovery with IOMSG= character values. 1) Move any work in an EndIoStatement() implementation that may raise an error into a new CompleteOperation() member function. This allows error handling APIs like GetIoMsg() to complete a pending I/O statement and harvest any errors that may result. 2) Move the pending error code from ErroneousIoStatementState to a new pendingError_ data member in IoErrorHandler. This allows IoErrorHandler::InError() to return a correct result when there is a pending error that will be recovered from so that I/O list data transfers don't crash in the meantime. 3) Don't create and leak a unit for a failed OPEN(NEWUNIT=n) with error recovery, and don't modify 'n'. (Depends on changes to API call ordering in lowering, in a separate patch; code was added to ensure that OPEN statement control list specifiers, e.g. SetFile(), must be passed before GetNewUnit().) 4) Fix the code that calls a form of strerror to fill an IOMSG= variable so that it actually works for Fortran's character type: blank fill with no null or newline termination. Differential Revision: https://reviews.llvm.org/D122036
1 parent b8f029c commit 8db4dc8

File tree

6 files changed

+225
-40
lines changed

6 files changed

+225
-40
lines changed

‎flang/runtime/descriptor-io.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,10 @@ static bool UnformattedDescriptorIO(
372372

373373
template <Direction DIR>
374374
static bool DescriptorIO(IoStatementState &io, const Descriptor &descriptor) {
375+
IoErrorHandler &handler{io.GetIoErrorHandler()};
376+
if (handler.InError()) {
377+
return false;
378+
}
375379
if (!io.get_if<IoDirectionState<DIR>>()) {
376380
io.GetIoErrorHandler().Crash(
377381
"DescriptorIO() called for wrong I/O direction");
@@ -385,7 +389,6 @@ static bool DescriptorIO(IoStatementState &io, const Descriptor &descriptor) {
385389
if (!io.get_if<FormattedIoStatementState<DIR>>()) {
386390
return UnformattedDescriptorIO<DIR>(io, descriptor);
387391
}
388-
IoErrorHandler &handler{io.GetIoErrorHandler()};
389392
if (auto catAndKind{descriptor.type().GetCategoryAndKind()}) {
390393
TypeCategory cat{catAndKind->first};
391394
int kind{catAndKind->second};

‎flang/runtime/io-api.cpp

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,9 @@ bool IONAME(SetAccess)(Cookie cookie, const char *keyword, std::size_t length) {
646646
if (!open) {
647647
io.GetIoErrorHandler().Crash(
648648
"SetAccess() called when not in an OPEN statement");
649+
} else if (open->completedOperation()) {
650+
io.GetIoErrorHandler().Crash(
651+
"SetAccess() called after GetNewUnit() for an OPEN statement");
649652
}
650653
static const char *keywords[]{
651654
"SEQUENTIAL", "DIRECT", "STREAM", "APPEND", nullptr};
@@ -675,6 +678,9 @@ bool IONAME(SetAction)(Cookie cookie, const char *keyword, std::size_t length) {
675678
if (!open) {
676679
io.GetIoErrorHandler().Crash(
677680
"SetAction() called when not in an OPEN statement");
681+
} else if (open->completedOperation()) {
682+
io.GetIoErrorHandler().Crash(
683+
"SetAction() called after GetNewUnit() for an OPEN statement");
678684
}
679685
std::optional<Action> action;
680686
static const char *keywords[]{"READ", "WRITE", "READWRITE", nullptr};
@@ -711,6 +717,9 @@ bool IONAME(SetAsynchronous)(
711717
if (!open) {
712718
io.GetIoErrorHandler().Crash(
713719
"SetAsynchronous() called when not in an OPEN statement");
720+
} else if (open->completedOperation()) {
721+
io.GetIoErrorHandler().Crash(
722+
"SetAsynchronous() called after GetNewUnit() for an OPEN statement");
714723
}
715724
static const char *keywords[]{"YES", "NO", nullptr};
716725
switch (IdentifyValue(keyword, length, keywords)) {
@@ -734,6 +743,9 @@ bool IONAME(SetCarriagecontrol)(
734743
if (!open) {
735744
io.GetIoErrorHandler().Crash(
736745
"SetCarriageControl() called when not in an OPEN statement");
746+
} else if (open->completedOperation()) {
747+
io.GetIoErrorHandler().Crash(
748+
"SetCarriageControl() called after GetNewUnit() for an OPEN statement");
737749
}
738750
static const char *keywords[]{"LIST", "FORTRAN", "NONE", nullptr};
739751
switch (IdentifyValue(keyword, length, keywords)) {
@@ -759,6 +771,9 @@ bool IONAME(SetConvert)(
759771
if (!open) {
760772
io.GetIoErrorHandler().Crash(
761773
"SetConvert() called when not in an OPEN statement");
774+
} else if (open->completedOperation()) {
775+
io.GetIoErrorHandler().Crash(
776+
"SetConvert() called after GetNewUnit() for an OPEN statement");
762777
}
763778
if (auto convert{GetConvertFromString(keyword, length)}) {
764779
open->set_convert(*convert);
@@ -777,6 +792,9 @@ bool IONAME(SetEncoding)(
777792
if (!open) {
778793
io.GetIoErrorHandler().Crash(
779794
"SetEncoding() called when not in an OPEN statement");
795+
} else if (open->completedOperation()) {
796+
io.GetIoErrorHandler().Crash(
797+
"SetEncoding() called after GetNewUnit() for an OPEN statement");
780798
}
781799
bool isUTF8{false};
782800
static const char *keywords[]{"UTF-8", "DEFAULT", nullptr};
@@ -806,6 +824,9 @@ bool IONAME(SetForm)(Cookie cookie, const char *keyword, std::size_t length) {
806824
if (!open) {
807825
io.GetIoErrorHandler().Crash(
808826
"SetForm() called when not in an OPEN statement");
827+
} else if (open->completedOperation()) {
828+
io.GetIoErrorHandler().Crash(
829+
"SetForm() called after GetNewUnit() for an OPEN statement");
809830
}
810831
static const char *keywords[]{"FORMATTED", "UNFORMATTED", nullptr};
811832
switch (IdentifyValue(keyword, length, keywords)) {
@@ -829,6 +850,9 @@ bool IONAME(SetPosition)(
829850
if (!open) {
830851
io.GetIoErrorHandler().Crash(
831852
"SetPosition() called when not in an OPEN statement");
853+
} else if (open->completedOperation()) {
854+
io.GetIoErrorHandler().Crash(
855+
"SetPosition() called after GetNewUnit() for an OPEN statement");
832856
}
833857
static const char *positions[]{"ASIS", "REWIND", "APPEND", nullptr};
834858
switch (IdentifyValue(keyword, length, positions)) {
@@ -854,6 +878,9 @@ bool IONAME(SetRecl)(Cookie cookie, std::size_t n) {
854878
if (!open) {
855879
io.GetIoErrorHandler().Crash(
856880
"SetRecl() called when not in an OPEN statement");
881+
} else if (open->completedOperation()) {
882+
io.GetIoErrorHandler().Crash(
883+
"SetRecl() called after GetNewUnit() for an OPEN statement");
857884
}
858885
if (n <= 0) {
859886
io.GetIoErrorHandler().SignalError("RECL= must be greater than zero");
@@ -871,6 +898,10 @@ bool IONAME(SetRecl)(Cookie cookie, std::size_t n) {
871898
bool IONAME(SetStatus)(Cookie cookie, const char *keyword, std::size_t length) {
872899
IoStatementState &io{*cookie};
873900
if (auto *open{io.get_if<OpenStatementState>()}) {
901+
if (open->completedOperation()) {
902+
io.GetIoErrorHandler().Crash(
903+
"SetStatus() called after GetNewUnit() for an OPEN statement");
904+
}
874905
static const char *statuses[]{
875906
"OLD", "NEW", "SCRATCH", "REPLACE", "UNKNOWN", nullptr};
876907
switch (IdentifyValue(keyword, length, statuses)) {
@@ -920,6 +951,10 @@ bool IONAME(SetStatus)(Cookie cookie, const char *keyword, std::size_t length) {
920951
bool IONAME(SetFile)(Cookie cookie, const char *path, std::size_t chars) {
921952
IoStatementState &io{*cookie};
922953
if (auto *open{io.get_if<OpenStatementState>()}) {
954+
if (open->completedOperation()) {
955+
io.GetIoErrorHandler().Crash(
956+
"SetFile() called after GetNewUnit() for an OPEN statement");
957+
}
923958
open->set_path(path, chars);
924959
return true;
925960
}
@@ -934,6 +969,12 @@ bool IONAME(GetNewUnit)(Cookie cookie, int &unit, int kind) {
934969
if (!open) {
935970
io.GetIoErrorHandler().Crash(
936971
"GetNewUnit() called when not in an OPEN statement");
972+
} else if (!open->InError()) {
973+
open->CompleteOperation();
974+
}
975+
if (open->InError()) {
976+
// A failed OPEN(NEWUNIT=n) does not modify 'n'
977+
return false;
937978
}
938979
std::int64_t result{open->unit().unitNumber()};
939980
if (!SetInteger(unit, kind, result)) {
@@ -971,16 +1012,17 @@ bool IONAME(OutputUnformattedBlock)(Cookie cookie, const char *x,
9711012
bool IONAME(InputUnformattedBlock)(
9721013
Cookie cookie, char *x, std::size_t length, std::size_t elementBytes) {
9731014
IoStatementState &io{*cookie};
1015+
IoErrorHandler &handler{io.GetIoErrorHandler()};
9741016
io.BeginReadingRecord();
975-
if (io.GetIoErrorHandler().InError()) {
1017+
if (handler.InError()) {
9761018
return false;
9771019
}
9781020
if (auto *unf{
9791021
io.get_if<ExternalUnformattedIoStatementState<Direction::Input>>()}) {
9801022
return unf->Receive(x, length, elementBytes);
9811023
}
982-
io.GetIoErrorHandler().Crash("InputUnformattedBlock() called for an I/O "
983-
"statement that is not unformatted output");
1024+
handler.Crash("InputUnformattedBlock() called for an I/O statement that is "
1025+
"not unformatted input");
9841026
return false;
9851027
}
9861028

@@ -1157,27 +1199,39 @@ bool IONAME(InputLogical)(Cookie cookie, bool &truth) {
11571199

11581200
std::size_t IONAME(GetSize)(Cookie cookie) {
11591201
IoStatementState &io{*cookie};
1202+
IoErrorHandler &handler{io.GetIoErrorHandler()};
1203+
if (!handler.InError()) {
1204+
io.CompleteOperation();
1205+
}
11601206
if (const auto *formatted{
11611207
io.get_if<FormattedIoStatementState<Direction::Input>>()}) {
11621208
return formatted->GetEditDescriptorChars();
11631209
}
1164-
io.GetIoErrorHandler().Crash(
1210+
handler.Crash(
11651211
"GetIoSize() called for an I/O statement that is not a formatted READ()");
11661212
return 0;
11671213
}
11681214

11691215
std::size_t IONAME(GetIoLength)(Cookie cookie) {
11701216
IoStatementState &io{*cookie};
1217+
IoErrorHandler &handler{io.GetIoErrorHandler()};
1218+
if (!handler.InError()) {
1219+
io.CompleteOperation();
1220+
}
11711221
if (const auto *inq{io.get_if<InquireIOLengthState>()}) {
11721222
return inq->bytes();
11731223
}
1174-
io.GetIoErrorHandler().Crash("GetIoLength() called for an I/O statement that "
1175-
"is not INQUIRE(IOLENGTH=)");
1224+
handler.Crash("GetIoLength() called for an I/O statement that is not "
1225+
"INQUIRE(IOLENGTH=)");
11761226
return 0;
11771227
}
11781228

11791229
void IONAME(GetIoMsg)(Cookie cookie, char *msg, std::size_t length) {
1180-
IoErrorHandler &handler{cookie->GetIoErrorHandler()};
1230+
IoStatementState &io{*cookie};
1231+
IoErrorHandler &handler{io.GetIoErrorHandler()};
1232+
if (!handler.InError()) {
1233+
io.CompleteOperation();
1234+
}
11811235
if (handler.InError()) { // leave "msg" alone when no error
11821236
handler.GetIoMsg(msg, length);
11831237
}

‎flang/runtime/io-error.cpp

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ void IoErrorHandler::SignalError(int iostatOrErrno, const char *msg, ...) {
2727
ioStat_ = IostatEor; // least priority
2828
}
2929
} else if (iostatOrErrno != IostatOk) {
30-
if (flags_ & (hasIoStat | hasErr)) {
30+
if (flags_ & (hasIoStat | hasIoMsg | hasErr)) {
3131
if (ioStat_ <= 0) {
3232
ioStat_ = iostatOrErrno; // priority over END=/EOR=
3333
if (msg && (flags_ & hasIoMsg)) {
@@ -75,41 +75,57 @@ void IoErrorHandler::SignalEnd() { SignalError(IostatEnd); }
7575

7676
void IoErrorHandler::SignalEor() { SignalError(IostatEor); }
7777

78+
void IoErrorHandler::SignalPendingError() {
79+
int error{pendingError_};
80+
pendingError_ = IostatOk;
81+
SignalError(error);
82+
}
83+
7884
bool IoErrorHandler::GetIoMsg(char *buffer, std::size_t bufferLength) {
7985
const char *msg{ioMsg_.get()};
8086
if (!msg) {
81-
msg = IostatErrorString(ioStat_);
87+
msg = IostatErrorString(ioStat_ == IostatOk ? pendingError_ : ioStat_);
8288
}
8389
if (msg) {
8490
ToFortranDefaultCharacter(buffer, bufferLength, msg);
8591
return true;
8692
}
8793

88-
char *newBuf;
8994
// Following code is taken from llvm/lib/Support/Errno.cpp
90-
// in LLVM v9.0.1
95+
// in LLVM v9.0.1 with inadequate modification for Fortran,
96+
// since rectified.
97+
bool ok{false};
9198
#if HAVE_STRERROR_R
9299
// strerror_r is thread-safe.
93100
#if defined(__GLIBC__) && defined(_GNU_SOURCE)
94101
// glibc defines its own incompatible version of strerror_r
95102
// which may not use the buffer supplied.
96-
newBuf = ::strerror_r(ioStat_, buffer, bufferLength);
103+
msg = ::strerror_r(ioStat_, buffer, bufferLength);
97104
#else
98-
return ::strerror_r(ioStat_, buffer, bufferLength) == 0;
105+
ok = ::strerror_r(ioStat_, buffer, bufferLength) == 0;
99106
#endif
100107
#elif HAVE_DECL_STRERROR_S // "Windows Secure API"
101-
return ::strerror_s(buffer, bufferLength, ioStat_) == 0;
108+
ok = ::strerror_s(buffer, bufferLength, ioStat_) == 0;
102109
#elif HAVE_STRERROR
103110
// Copy the thread un-safe result of strerror into
104111
// the buffer as fast as possible to minimize impact
105112
// of collision of strerror in multiple threads.
106-
newBuf = strerror(ioStat_);
113+
msg = strerror(ioStat_);
107114
#else
108115
// Strange that this system doesn't even have strerror
109116
return false;
110117
#endif
111-
::strncpy(buffer, newBuf, bufferLength - 1);
112-
buffer[bufferLength - 1] = '\n';
113-
return true;
118+
if (msg) {
119+
ToFortranDefaultCharacter(buffer, bufferLength, msg);
120+
return true;
121+
} else if (ok) {
122+
std::size_t copied{std::strlen(buffer)};
123+
if (copied < bufferLength) {
124+
std::memset(buffer + copied, ' ', bufferLength - copied);
125+
}
126+
return true;
127+
} else {
128+
return false;
129+
}
114130
}
115131
} // namespace Fortran::runtime::io

‎flang/runtime/io-error.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,15 @@ class IoErrorHandler : public Terminator {
3636
flags_ = hasIoStat | hasErr | hasEnd | hasEor | hasIoMsg;
3737
}
3838

39-
bool InError() const { return ioStat_ != IostatOk; }
39+
bool InError() const {
40+
return ioStat_ != IostatOk || pendingError_ != IostatOk;
41+
}
42+
43+
// For I/O statements that detect fatal errors in their
44+
// Begin...() API routines before it is known whether they
45+
// have error handling control list items. Such statements
46+
// have an ErroneousIoStatementState with a pending error.
47+
void SetPendingError(int iostat) { pendingError_ = iostat; }
4048

4149
void SignalError(int iostatOrErrno, const char *msg, ...);
4250
void SignalError(int iostatOrErrno);
@@ -49,6 +57,7 @@ class IoErrorHandler : public Terminator {
4957
void SignalErrno(); // SignalError(errno)
5058
void SignalEnd(); // input only; EOF on internal write is an error
5159
void SignalEor(); // non-advancing input only; EOR on write is an error
60+
void SignalPendingError();
5261

5362
int GetIoStat() const { return ioStat_; }
5463
bool GetIoMsg(char *, std::size_t);
@@ -64,6 +73,7 @@ class IoErrorHandler : public Terminator {
6473
std::uint8_t flags_{0};
6574
int ioStat_{IostatOk};
6675
OwningPtr<char> ioMsg_;
76+
int pendingError_{IostatOk};
6777
};
6878

6979
} // namespace Fortran::runtime::io

0 commit comments

Comments
 (0)