Skip to content

Commit

Permalink
[flang] Add runtime API to catch unit number out of range
Browse files Browse the repository at this point in the history
Unit numbers must fit on a default integer. It is however possible that
the user provides the unit number in UNIT with a wider integer type.
In such case, lowering was previously silently narrowing
the value and passing the result to the BeginXXX runtime entry points.
Cases where the conversion caused overflow were not reported/caught.
Most existing compilers catch these errors and raise an IO error.
Add a CheckUnitNumberInRange runtime API to do the same in f18.

This runtime API has its own error management interface (i.e., does not
use GetIoMsg, EndIo, and EnableHandlers) because the usual error
management requires BeginXXX to be called to set up the error
management. But in this case, the BeginXXX cannot be called since
the bad unit number that would be provided to it overflew (and in the worst
case scenario, the narrowed value could point to a different valid unit
already in use). Hence I decided to make an API that must be called
before the BeginXXX and should trigger the whole BeginXXX/.../EndIoStatement
to be skipped in case the unit number is too big and the user enabled
error recovery.

Note that CheckUnitNumberInRange accepts negative numbers (as long as
they can fit on a default integer), because unit numbers may be negative
if they were created by NEWUNIT.

Differential Revision: https://reviews.llvm.org/D123157
  • Loading branch information
jeanPerier committed Apr 6, 2022
1 parent f4661b5 commit c58c64d
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 0 deletions.
18 changes: 18 additions & 0 deletions flang/include/flang/Runtime/io-api.h
Expand Up @@ -111,6 +111,24 @@ Cookie IONAME(BeginInternalFormattedInput)(const char *internal,
void **scratchArea = nullptr, std::size_t scratchBytes = 0,
const char *sourceFile = nullptr, int sourceLine = 0);

// External unit numbers must fit in default integers. When the integer
// provided as UNIT is of a wider type than the default integer, it could
// overflow when converted to a default integer.
// CheckUnitNumberInRange should be called to verify that a unit number of a
// wide integer type can fit in a default integer. Since it should be called
// before the BeginXXX(unit, ...) call, it has its own error handling interface.
// If handleError is false, and the unit number is out of range, the program
// will be terminated. Otherwise, if unit is out of range, a nonzero Iostat
// code is returned and ioMsg is set if it is not a nullptr.
enum Iostat IONAME(CheckUnitNumberInRange64)(std::int64_t unit,
bool handleError, char *ioMsg = nullptr, std::size_t ioMsgLength = 0,
const char *sourceFile = nullptr, int sourceLine = 0);
#ifdef __SIZEOF_INT128__
enum Iostat IONAME(CheckUnitNumberInRange128)(common::int128_t unit,
bool handleError, char *ioMsg = nullptr, std::size_t ioMsgLength = 0,
const char *sourceFile = nullptr, int sourceLine = 0);
#endif

// External synchronous I/O initiation
Cookie IONAME(BeginExternalListOutput)(ExternalUnit = DefaultUnit,
const char *sourceFile = nullptr, int sourceLine = 0);
Expand Down
1 change: 1 addition & 0 deletions flang/include/flang/Runtime/iostat.h
Expand Up @@ -67,6 +67,7 @@ enum Iostat {
IostatMissingTerminator,
IostatBadUnformattedRecord,
IostatUTF8Decoding,
IostatUnitOverflow,
};

const char *IostatErrorString(int);
Expand Down
47 changes: 47 additions & 0 deletions flang/runtime/io-api.cpp
Expand Up @@ -1275,4 +1275,51 @@ enum Iostat IONAME(EndIoStatement)(Cookie cookie) {
IoStatementState &io{*cookie};
return static_cast<enum Iostat>(io.EndIoStatement());
}

template <typename INT>
static enum Iostat CheckUnitNumberInRangeImpl(INT unit, bool handleError,
char *ioMsg, std::size_t ioMsgLength, const char *sourceFile,
int sourceLine) {
if (unit != static_cast<ExternalUnit>(unit)) {
Terminator oom{sourceFile, sourceLine};
IoErrorHandler errorHandler{oom};
if (handleError) {
errorHandler.HasIoStat();
if (ioMsg) {
errorHandler.HasIoMsg();
}
}
// Only provide the bad unit number in the message if SignalError can print
// it accurately. Otherwise, the generic IostatUnitOverflow message will be
// used.
if (static_cast<std::intmax_t>(unit) == unit) {
errorHandler.SignalError(IostatUnitOverflow,
"UNIT number %jd is out of range", static_cast<std::intmax_t>(unit));
} else {
errorHandler.SignalError(IostatUnitOverflow);
}
if (ioMsg) {
errorHandler.GetIoMsg(ioMsg, ioMsgLength);
}
return static_cast<enum Iostat>(errorHandler.GetIoStat());
}
return IostatOk;
}

enum Iostat IONAME(CheckUnitNumberInRange64)(std::int64_t unit,
bool handleError, char *ioMsg, std::size_t ioMsgLength,
const char *sourceFile, int sourceLine) {
return CheckUnitNumberInRangeImpl(
unit, handleError, ioMsg, ioMsgLength, sourceFile, sourceLine);
}

#ifdef __SIZEOF_INT128__
enum Iostat IONAME(CheckUnitNumberInRange128)(common::int128_t unit,
bool handleError, char *ioMsg, std::size_t ioMsgLength,
const char *sourceFile, int sourceLine) {
return CheckUnitNumberInRangeImpl(
unit, handleError, ioMsg, ioMsgLength, sourceFile, sourceLine);
}
#endif

} // namespace Fortran::runtime::io
2 changes: 2 additions & 0 deletions flang/runtime/iostat.cpp
Expand Up @@ -77,6 +77,8 @@ const char *IostatErrorString(int iostat) {
return "Erroneous unformatted sequential file record structure";
case IostatUTF8Decoding:
return "UTF-8 decoding error";
case IostatUnitOverflow:
return "UNIT number is out of range";
default:
return nullptr;
}
Expand Down
35 changes: 35 additions & 0 deletions flang/unittests/Runtime/ExternalIOTest.cpp
Expand Up @@ -895,3 +895,38 @@ TEST(ExternalIOTests, TestUCS) {
ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
<< "EndIoStatement() for second CLOSE";
}

TEST(ExternalIOTests, BigUnitNumbers) {
if (std::numeric_limits<ExternalUnit>::max() <
std::numeric_limits<std::int64_t>::max()) {
std::int64_t unit64Ok = std::numeric_limits<ExternalUnit>::max();
std::int64_t unit64Bad = unit64Ok + 1;
std::int64_t unit64Bad2 =
static_cast<std::int64_t>(std::numeric_limits<ExternalUnit>::min()) - 1;
EXPECT_EQ(IONAME(CheckUnitNumberInRange64)(unit64Ok, true), IostatOk);
EXPECT_EQ(IONAME(CheckUnitNumberInRange64)(unit64Ok, false), IostatOk);
EXPECT_EQ(
IONAME(CheckUnitNumberInRange64)(unit64Bad, true), IostatUnitOverflow);
EXPECT_EQ(
IONAME(CheckUnitNumberInRange64)(unit64Bad2, true), IostatUnitOverflow);
EXPECT_EQ(
IONAME(CheckUnitNumberInRange64)(unit64Bad, true), IostatUnitOverflow);
EXPECT_EQ(
IONAME(CheckUnitNumberInRange64)(unit64Bad2, true), IostatUnitOverflow);
constexpr std::size_t n{80};
char expectedMsg[n + 1];
expectedMsg[n] = '\0';
std::snprintf(expectedMsg, n, "UNIT number %jd is out of range",
static_cast<std::intmax_t>(unit64Bad));
EXPECT_DEATH(
IONAME(CheckUnitNumberInRange64)(2147483648, false), expectedMsg);
for (auto i{std::strlen(expectedMsg)}; i < n; ++i) {
expectedMsg[i] = ' ';
}
char msg[n + 1];
msg[n] = '\0';
EXPECT_EQ(IONAME(CheckUnitNumberInRange64)(unit64Bad, true, msg, n),
IostatUnitOverflow);
EXPECT_EQ(std::strncmp(msg, expectedMsg, n), 0);
}
}

0 comments on commit c58c64d

Please sign in to comment.