[flang-commits] [flang] cfbde71 - [flang][runtime] Catch more (all?) negative unit number errors
Peter Klausler via flang-commits
flang-commits at lists.llvm.org
Wed Jun 15 17:34:49 PDT 2022
Author: Peter Klausler
Date: 2022-06-15T17:29:50-07:00
New Revision: cfbde7149d8647a4ff2eef892a30ffc55605df2c
URL: https://github.com/llvm/llvm-project/commit/cfbde7149d8647a4ff2eef892a30ffc55605df2c
DIFF: https://github.com/llvm/llvm-project/commit/cfbde7149d8647a4ff2eef892a30ffc55605df2c.diff
LOG: [flang][runtime] Catch more (all?) negative unit number errors
Fortran does have negative unit numbers -- they show up in child I/O
subroutines for defined I/O and for OPEN(NEWUNIT=) -- but the runtime
needs to catch the cases where a negative unit number that wasn't
generated by the runtime is passed in for OPEN or for an I/O statement
that would ordinarily create an anonymous "fort.NNN" file for a
hitherto unseen unit.
Differential Revision: https://reviews.llvm.org/D127788
Added:
Modified:
flang/include/flang/Runtime/iostat.h
flang/runtime/io-api.cpp
flang/runtime/iostat.cpp
flang/runtime/unit-map.h
flang/runtime/unit.cpp
flang/runtime/unit.h
Removed:
################################################################################
diff --git a/flang/include/flang/Runtime/iostat.h b/flang/include/flang/Runtime/iostat.h
index 1ae6890678b4e..bd38e748e4a92 100644
--- a/flang/include/flang/Runtime/iostat.h
+++ b/flang/include/flang/Runtime/iostat.h
@@ -80,6 +80,7 @@ enum Iostat {
IostatBadWaitId,
IostatTooManyAsyncOps,
IostatBadBackspaceUnit,
+ IostatBadUnitNumber,
};
const char *IostatErrorString(int);
diff --git a/flang/runtime/io-api.cpp b/flang/runtime/io-api.cpp
index af79757c2add7..c11e4271b20eb 100644
--- a/flang/runtime/io-api.cpp
+++ b/flang/runtime/io-api.cpp
@@ -147,6 +147,24 @@ Cookie IONAME(BeginInternalFormattedInput)(const char *internal,
format, formatLength, scratchArea, scratchBytes, sourceFile, sourceLine);
}
+static ExternalFileUnit *GetOrCreateUnit(int unitNumber, Direction direction,
+ std::optional<bool> isUnformatted, const Terminator &terminator,
+ Cookie &errorCookie) {
+ if (ExternalFileUnit *
+ unit{ExternalFileUnit::LookUpOrCreateAnonymous(
+ unitNumber, direction, isUnformatted, terminator)}) {
+ errorCookie = nullptr;
+ return unit;
+ } else {
+ errorCookie = &New<NoopStatementState>{terminator}(
+ terminator.sourceFileName(), terminator.sourceLine(), unitNumber)
+ .release()
+ ->ioStatementState();
+ errorCookie->GetIoErrorHandler().SetPendingError(IostatBadUnitNumber);
+ return nullptr;
+ }
+}
+
template <Direction DIR, template <Direction> class STATE, typename... A>
Cookie BeginExternalListIO(
int unitNumber, const char *sourceFile, int sourceLine, A &&...xs) {
@@ -154,16 +172,20 @@ Cookie BeginExternalListIO(
if (unitNumber == DefaultUnit) {
unitNumber = DIR == Direction::Input ? 5 : 6;
}
- ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
- unitNumber, DIR, false /*!unformatted*/, terminator)};
- if (!unit.isUnformatted.has_value()) {
- unit.isUnformatted = false;
+ Cookie errorCookie{nullptr};
+ ExternalFileUnit *unit{GetOrCreateUnit(
+ unitNumber, DIR, false /*!unformatted*/, terminator, errorCookie)};
+ if (!unit) {
+ return errorCookie;
+ }
+ if (!unit->isUnformatted.has_value()) {
+ unit->isUnformatted = false;
}
Iostat iostat{IostatOk};
- if (*unit.isUnformatted) {
+ if (*unit->isUnformatted) {
iostat = IostatFormattedIoOnUnformattedUnit;
}
- if (ChildIo * child{unit.GetChildIo()}) {
+ if (ChildIo * child{unit->GetChildIo()}) {
if (iostat == IostatOk) {
iostat = child->CheckFormattingAndDirection(false, DIR);
}
@@ -175,18 +197,18 @@ Cookie BeginExternalListIO(
iostat, nullptr /* no unit */, sourceFile, sourceLine);
}
} else {
- if (iostat == IostatOk && unit.access == Access::Direct) {
+ if (iostat == IostatOk && unit->access == Access::Direct) {
iostat = IostatListIoOnDirectAccessUnit;
}
if (iostat == IostatOk) {
- iostat = unit.SetDirection(DIR);
+ iostat = unit->SetDirection(DIR);
}
if (iostat == IostatOk) {
- return &unit.BeginIoStatement<STATE<DIR>>(
- std::forward<A>(xs)..., unit, sourceFile, sourceLine);
+ return &unit->BeginIoStatement<STATE<DIR>>(
+ std::forward<A>(xs)..., *unit, sourceFile, sourceLine);
} else {
- return &unit.BeginIoStatement<ErroneousIoStatementState>(
- iostat, &unit, sourceFile, sourceLine);
+ return &unit->BeginIoStatement<ErroneousIoStatementState>(
+ iostat, unit, sourceFile, sourceLine);
}
}
}
@@ -210,16 +232,20 @@ Cookie BeginExternalFormattedIO(const char *format, std::size_t formatLength,
if (unitNumber == DefaultUnit) {
unitNumber = DIR == Direction::Input ? 5 : 6;
}
- ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
- unitNumber, DIR, false /*!unformatted*/, terminator)};
+ Cookie errorCookie{nullptr};
+ ExternalFileUnit *unit{GetOrCreateUnit(
+ unitNumber, DIR, false /*!unformatted*/, terminator, errorCookie)};
+ if (!unit) {
+ return errorCookie;
+ }
Iostat iostat{IostatOk};
- if (!unit.isUnformatted.has_value()) {
- unit.isUnformatted = false;
+ if (!unit->isUnformatted.has_value()) {
+ unit->isUnformatted = false;
}
- if (*unit.isUnformatted) {
+ if (*unit->isUnformatted) {
iostat = IostatFormattedIoOnUnformattedUnit;
}
- if (ChildIo * child{unit.GetChildIo()}) {
+ if (ChildIo * child{unit->GetChildIo()}) {
if (iostat == IostatOk) {
iostat = child->CheckFormattingAndDirection(false, DIR);
}
@@ -232,14 +258,14 @@ Cookie BeginExternalFormattedIO(const char *format, std::size_t formatLength,
}
} else {
if (iostat == IostatOk) {
- iostat = unit.SetDirection(DIR);
+ iostat = unit->SetDirection(DIR);
}
if (iostat == IostatOk) {
- return &unit.BeginIoStatement<ExternalFormattedIoStatementState<DIR>>(
- unit, format, formatLength, sourceFile, sourceLine);
+ return &unit->BeginIoStatement<ExternalFormattedIoStatementState<DIR>>(
+ *unit, format, formatLength, sourceFile, sourceLine);
} else {
- return &unit.BeginIoStatement<ErroneousIoStatementState>(
- iostat, &unit, sourceFile, sourceLine);
+ return &unit->BeginIoStatement<ErroneousIoStatementState>(
+ iostat, unit, sourceFile, sourceLine);
}
}
}
@@ -262,16 +288,20 @@ template <Direction DIR>
Cookie BeginUnformattedIO(
ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
Terminator terminator{sourceFile, sourceLine};
- ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
- unitNumber, DIR, true /*unformatted*/, terminator)};
+ Cookie errorCookie{nullptr};
+ ExternalFileUnit *unit{GetOrCreateUnit(
+ unitNumber, DIR, true /*unformatted*/, terminator, errorCookie)};
+ if (!unit) {
+ return errorCookie;
+ }
Iostat iostat{IostatOk};
- if (!unit.isUnformatted.has_value()) {
- unit.isUnformatted = true;
+ if (!unit->isUnformatted.has_value()) {
+ unit->isUnformatted = true;
}
- if (!*unit.isUnformatted) {
+ if (!*unit->isUnformatted) {
iostat = IostatUnformattedIoOnFormattedUnit;
}
- if (ChildIo * child{unit.GetChildIo()}) {
+ if (ChildIo * child{unit->GetChildIo()}) {
if (iostat == IostatOk) {
iostat = child->CheckFormattingAndDirection(true, DIR);
}
@@ -284,24 +314,24 @@ Cookie BeginUnformattedIO(
}
} else {
if (iostat == IostatOk) {
- iostat = unit.SetDirection(DIR);
+ iostat = unit->SetDirection(DIR);
}
if (iostat == IostatOk) {
IoStatementState &io{
- unit.BeginIoStatement<ExternalUnformattedIoStatementState<DIR>>(
- unit, sourceFile, sourceLine)};
+ unit->BeginIoStatement<ExternalUnformattedIoStatementState<DIR>>(
+ *unit, sourceFile, sourceLine)};
if constexpr (DIR == Direction::Output) {
- if (unit.access == Access::Sequential) {
+ 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
+ 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<ErroneousIoStatementState>(
- iostat, &unit, sourceFile, sourceLine);
+ return &unit->BeginIoStatement<ErroneousIoStatementState>(
+ iostat, unit, sourceFile, sourceLine);
}
}
}
@@ -320,12 +350,21 @@ Cookie IONAME(BeginUnformattedInput)(
Cookie IONAME(BeginOpenUnit)( // OPEN(without NEWUNIT=)
ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
- bool wasExtant{false};
Terminator terminator{sourceFile, sourceLine};
- ExternalFileUnit &unit{
- ExternalFileUnit::LookUpOrCreate(unitNumber, terminator, wasExtant)};
- return &unit.BeginIoStatement<OpenStatementState>(
- unit, wasExtant, sourceFile, sourceLine);
+ bool wasExtant{false};
+ if (ExternalFileUnit *
+ unit{ExternalFileUnit::LookUpOrCreate(
+ unitNumber, terminator, wasExtant)}) {
+ return &unit->BeginIoStatement<OpenStatementState>(
+ *unit, wasExtant, sourceFile, sourceLine);
+ } else {
+ auto &io{
+ New<NoopStatementState>{terminator}(sourceFile, sourceLine, unitNumber)
+ .release()
+ ->ioStatementState()};
+ io.GetIoErrorHandler().SetPendingError(IostatBadUnitNumber);
+ return &io;
+ }
}
Cookie IONAME(BeginOpenNewUnit)( // OPEN(NEWUNIT=j)
@@ -411,19 +450,29 @@ Cookie IONAME(BeginBackspace)(
Cookie IONAME(BeginEndfile)(
ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
Terminator terminator{sourceFile, sourceLine};
- ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
- unitNumber, Direction::Output, std::nullopt, terminator)};
- return &unit.BeginIoStatement<ExternalMiscIoStatementState>(
- unit, ExternalMiscIoStatementState::Endfile, sourceFile, sourceLine);
+ Cookie errorCookie{nullptr};
+ if (ExternalFileUnit *
+ unit{GetOrCreateUnit(unitNumber, Direction::Output, std::nullopt,
+ terminator, errorCookie)}) {
+ return &unit->BeginIoStatement<ExternalMiscIoStatementState>(
+ *unit, ExternalMiscIoStatementState::Endfile, sourceFile, sourceLine);
+ } else {
+ return errorCookie;
+ }
}
Cookie IONAME(BeginRewind)(
ExternalUnit unitNumber, const char *sourceFile, int sourceLine) {
Terminator terminator{sourceFile, sourceLine};
- ExternalFileUnit &unit{ExternalFileUnit::LookUpOrCreateAnonymous(
- unitNumber, Direction::Input, std::nullopt, terminator)};
- return &unit.BeginIoStatement<ExternalMiscIoStatementState>(
- unit, ExternalMiscIoStatementState::Rewind, sourceFile, sourceLine);
+ Cookie errorCookie{nullptr};
+ if (ExternalFileUnit *
+ unit{GetOrCreateUnit(unitNumber, Direction::Input, std::nullopt,
+ terminator, errorCookie)}) {
+ return &unit->BeginIoStatement<ExternalMiscIoStatementState>(
+ *unit, ExternalMiscIoStatementState::Rewind, sourceFile, sourceLine);
+ } else {
+ return errorCookie;
+ }
}
Cookie IONAME(BeginInquireUnit)(
diff --git a/flang/runtime/iostat.cpp b/flang/runtime/iostat.cpp
index a1df37a7647c2..d39f9b64f5e65 100644
--- a/flang/runtime/iostat.cpp
+++ b/flang/runtime/iostat.cpp
@@ -105,6 +105,8 @@ const char *IostatErrorString(int iostat) {
return "Too many asynchronous operations pending on unit";
case IostatBadBackspaceUnit:
return "BACKSPACE on unconnected unit";
+ case IostatBadUnitNumber:
+ return "Negative unit number is not allowed";
default:
return nullptr;
}
diff --git a/flang/runtime/unit-map.h b/flang/runtime/unit-map.h
index 7211892ef66eb..6f1e01bb1e64a 100644
--- a/flang/runtime/unit-map.h
+++ b/flang/runtime/unit-map.h
@@ -28,12 +28,16 @@ class UnitMap {
return Find(n);
}
- ExternalFileUnit &LookUpOrCreate(
+ ExternalFileUnit *LookUpOrCreate(
int n, const Terminator &terminator, bool &wasExtant) {
CriticalSection critical{lock_};
- auto *p{Find(n)};
- wasExtant = p != nullptr;
- return p ? *p : Create(n, terminator);
+ if (auto *p{Find(n)}) {
+ wasExtant = true;
+ return p;
+ } else {
+ wasExtant = false;
+ return n >= 0 ? &Create(n, terminator) : nullptr;
+ }
}
// Unit look-up by name is needed for INQUIRE(FILE="...")
diff --git a/flang/runtime/unit.cpp b/flang/runtime/unit.cpp
index 2d1beb5b5e643..9cef0f5cb0afd 100644
--- a/flang/runtime/unit.cpp
+++ b/flang/runtime/unit.cpp
@@ -43,23 +43,23 @@ ExternalFileUnit *ExternalFileUnit::LookUp(int unit) {
return GetUnitMap().LookUp(unit);
}
-ExternalFileUnit &ExternalFileUnit::LookUpOrCreate(
+ExternalFileUnit *ExternalFileUnit::LookUpOrCreate(
int unit, const Terminator &terminator, bool &wasExtant) {
return GetUnitMap().LookUpOrCreate(unit, terminator, wasExtant);
}
-ExternalFileUnit &ExternalFileUnit::LookUpOrCreateAnonymous(int unit,
+ExternalFileUnit *ExternalFileUnit::LookUpOrCreateAnonymous(int unit,
Direction dir, std::optional<bool> isUnformatted,
const Terminator &terminator) {
bool exists{false};
- ExternalFileUnit &result{
+ ExternalFileUnit *result{
GetUnitMap().LookUpOrCreate(unit, terminator, exists)};
- if (!exists) {
+ if (result && !exists) {
IoErrorHandler handler{terminator};
- result.OpenAnonymousUnit(
+ result->OpenAnonymousUnit(
dir == Direction::Input ? OpenStatus::Unknown : OpenStatus::Replace,
Action::ReadWrite, Position::Rewind, Convert::Native, handler);
- result.isUnformatted = isUnformatted;
+ result->isUnformatted = isUnformatted;
}
return result;
}
@@ -72,10 +72,10 @@ ExternalFileUnit *ExternalFileUnit::LookUp(
ExternalFileUnit &ExternalFileUnit::CreateNew(
int unit, const Terminator &terminator) {
bool wasExtant{false};
- ExternalFileUnit &result{
+ ExternalFileUnit *result{
GetUnitMap().LookUpOrCreate(unit, terminator, wasExtant)};
- RUNTIME_CHECK(terminator, !wasExtant);
- return result;
+ RUNTIME_CHECK(terminator, result && !wasExtant);
+ return *result;
}
ExternalFileUnit *ExternalFileUnit::LookUpForClose(int unit) {
@@ -218,21 +218,22 @@ UnitMap &ExternalFileUnit::GetUnitMap() {
UnitMap *newUnitMap{New<UnitMap>{terminator}().release()};
bool wasExtant{false};
- ExternalFileUnit &out{newUnitMap->LookUpOrCreate(6, terminator, wasExtant)};
+ ExternalFileUnit &out{*newUnitMap->LookUpOrCreate(6, terminator, wasExtant)};
RUNTIME_CHECK(terminator, !wasExtant);
out.Predefine(1);
handler.SignalError(out.SetDirection(Direction::Output));
out.isUnformatted = false;
defaultOutput = &out;
- ExternalFileUnit &in{newUnitMap->LookUpOrCreate(5, terminator, wasExtant)};
+ ExternalFileUnit &in{*newUnitMap->LookUpOrCreate(5, terminator, wasExtant)};
RUNTIME_CHECK(terminator, !wasExtant);
in.Predefine(0);
handler.SignalError(in.SetDirection(Direction::Input));
in.isUnformatted = false;
defaultInput = ∈
- ExternalFileUnit &error{newUnitMap->LookUpOrCreate(0, terminator, wasExtant)};
+ ExternalFileUnit &error{
+ *newUnitMap->LookUpOrCreate(0, terminator, wasExtant)};
RUNTIME_CHECK(terminator, !wasExtant);
error.Predefine(2);
handler.SignalError(error.SetDirection(Direction::Output));
diff --git a/flang/runtime/unit.h b/flang/runtime/unit.h
index 3498a5e9682f2..c58f71c00186f 100644
--- a/flang/runtime/unit.h
+++ b/flang/runtime/unit.h
@@ -47,9 +47,9 @@ class ExternalFileUnit : public ConnectionState,
bool createdForInternalChildIo() const { return createdForInternalChildIo_; }
static ExternalFileUnit *LookUp(int unit);
- static ExternalFileUnit &LookUpOrCreate(
+ static ExternalFileUnit *LookUpOrCreate(
int unit, const Terminator &, bool &wasExtant);
- static ExternalFileUnit &LookUpOrCreateAnonymous(int unit, Direction,
+ static ExternalFileUnit *LookUpOrCreateAnonymous(int unit, Direction,
std::optional<bool> isUnformatted, const Terminator &);
static ExternalFileUnit *LookUp(const char *path, std::size_t pathLen);
static ExternalFileUnit &CreateNew(int unit, const Terminator &);
More information about the flang-commits
mailing list