[flang-commits] [flang] c58c64d - [flang] Add runtime API to catch unit number out of range

Jean Perier via flang-commits flang-commits at lists.llvm.org
Wed Apr 6 06:39:10 PDT 2022


Author: Jean Perier
Date: 2022-04-06T15:38:13+02:00
New Revision: c58c64d05c51b75df3d803d293d386d8e7ae02b4

URL: https://github.com/llvm/llvm-project/commit/c58c64d05c51b75df3d803d293d386d8e7ae02b4
DIFF: https://github.com/llvm/llvm-project/commit/c58c64d05c51b75df3d803d293d386d8e7ae02b4.diff

LOG: [flang] Add runtime API to catch unit number out of range

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

Added: 
    

Modified: 
    flang/include/flang/Runtime/io-api.h
    flang/include/flang/Runtime/iostat.h
    flang/runtime/io-api.cpp
    flang/runtime/iostat.cpp
    flang/unittests/Runtime/ExternalIOTest.cpp

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Runtime/io-api.h b/flang/include/flang/Runtime/io-api.h
index 800244c29884d..2b0005f467399 100644
--- a/flang/include/flang/Runtime/io-api.h
+++ b/flang/include/flang/Runtime/io-api.h
@@ -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);

diff  --git a/flang/include/flang/Runtime/iostat.h b/flang/include/flang/Runtime/iostat.h
index d0e8ea7d65747..15985b74dd3ce 100644
--- a/flang/include/flang/Runtime/iostat.h
+++ b/flang/include/flang/Runtime/iostat.h
@@ -67,6 +67,7 @@ enum Iostat {
   IostatMissingTerminator,
   IostatBadUnformattedRecord,
   IostatUTF8Decoding,
+  IostatUnitOverflow,
 };
 
 const char *IostatErrorString(int);

diff  --git a/flang/runtime/io-api.cpp b/flang/runtime/io-api.cpp
index 70bd1480a2662..80f4583f42d63 100644
--- a/flang/runtime/io-api.cpp
+++ b/flang/runtime/io-api.cpp
@@ -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

diff  --git a/flang/runtime/iostat.cpp b/flang/runtime/iostat.cpp
index 73cf2b4e58002..5b761369206cb 100644
--- a/flang/runtime/iostat.cpp
+++ b/flang/runtime/iostat.cpp
@@ -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;
   }

diff  --git a/flang/unittests/Runtime/ExternalIOTest.cpp b/flang/unittests/Runtime/ExternalIOTest.cpp
index d88a0e11d87d0..87b1ef91addd1 100644
--- a/flang/unittests/Runtime/ExternalIOTest.cpp
+++ b/flang/unittests/Runtime/ExternalIOTest.cpp
@@ -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);
+  }
+}


        


More information about the flang-commits mailing list