[flang-commits] [flang] [flang][runtime] Fix runtime crash after bad recoverable OPEN (PR #111454)

[Peter Klausler via flang-commits]

https://github.com/klausler created https://github.com/llvm/llvm-project/pull/111454

When an OPEN statement with a unit number fails in a recoverable manner, the runtime needs to delete the ExternalFileUnit instance that was created in the unit map.  And we do this too soon -- that instance still holds some of the I/O statement state that will be used by a later call into the runtime for EndIoStatement.

Move the code that deletes the unit after a failed but recoverable OPEN into ExternalIoStatementBase::EndIoStatement, and don't do things afterwards that would need the I/O statement state that has been destroyed.

Fixes https://github.com/llvm/llvm-project/issues/111404.

>From 405d006f568c1f5e65bd3642b6e7a0d72fc5cfd9 Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler at nvidia.com>
Date: Mon, 7 Oct 2024 16:27:25 -0700
Subject: [PATCH] [flang][runtime] Fix runtime crash after bad recoverable OPEN

When an OPEN statement with a unit number fails in a recoverable
manner, the runtime needs to delete the ExternalFileUnit instance
that was created in the unit map.  And we do this too soon -- that
instance still holds some of the I/O statement state that will be
used by a later call into the runtime for EndIoStatement.

Move the code that deletes the unit after a failed but recoverable
OPEN into ExternalIoStatementBase::EndIoStatement, and don't do
things afterwards that would need the I/O statement state that has
been destroyed.

Fixes https://github.com/llvm/llvm-project/issues/111404.
---
 flang/runtime/io-stmt.cpp | 15 +++++++++------
 flang/runtime/io-stmt.h   |  2 ++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/flang/runtime/io-stmt.cpp b/flang/runtime/io-stmt.cpp
index cd7a196335d31e..b59ad45b16ddea 100644
--- a/flang/runtime/io-stmt.cpp
+++ b/flang/runtime/io-stmt.cpp
@@ -243,7 +243,14 @@ int ExternalIoStatementBase::EndIoStatement() {
   CompleteOperation();
   auto result{IoStatementBase::EndIoStatement()};
 #if !defined(RT_USE_PSEUDO_FILE_UNIT)
-  unit_.EndIoStatement(); // annihilates *this in unit_.u_
+  if (destroy_) {
+    if (ExternalFileUnit * toClose{unit_.LookUpForClose(unit_.unitNumber())}) {
+      toClose->Close(CloseStatus::Delete, *this);
+      toClose->DestroyClosed();
+    }
+  } else {
+    unit_.EndIoStatement(); // annihilates *this in unit_.u_
+  }
 #else
   // Fetch the unit pointer before *this disappears.
   ExternalFileUnit *unitPtr{&unit_};
@@ -329,11 +336,7 @@ void OpenStatementState::CompleteOperation() {
   }
   if (!wasExtant_ && InError()) {
     // Release the new unit on failure
-    if (ExternalFileUnit *
-        toClose{unit().LookUpForClose(unit().unitNumber())}) {
-      toClose->Close(CloseStatus::Delete, *this);
-      toClose->DestroyClosed();
-    }
+    set_destroy();
   }
   IoStatementBase::CompleteOperation();
 }
diff --git a/flang/runtime/io-stmt.h b/flang/runtime/io-stmt.h
index 2e0ca46078ecdc..1f1419b249e5e5 100644
--- a/flang/runtime/io-stmt.h
+++ b/flang/runtime/io-stmt.h
@@ -455,6 +455,7 @@ class ExternalIoStatementBase : public IoStatementBase {
   RT_API_ATTRS MutableModes &mutableModes();
   RT_API_ATTRS ConnectionState &GetConnectionState();
   RT_API_ATTRS int asynchronousID() const { return asynchronousID_; }
+  RT_API_ATTRS void set_destroy(bool yes = true) { destroy_ = yes; }
   RT_API_ATTRS int EndIoStatement();
   RT_API_ATTRS ExternalFileUnit *GetExternalFileUnit() const { return &unit_; }
   RT_API_ATTRS void SetAsynchronous();
@@ -463,6 +464,7 @@ class ExternalIoStatementBase : public IoStatementBase {
 private:
   ExternalFileUnit &unit_;
   int asynchronousID_{-1};
+  bool destroy_{false};
 };
 
 template <Direction DIR>