Skip to content

Conversation

@klausler
Copy link
Contributor

@klausler klausler commented Oct 7, 2024

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 #111404.

@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2024

@llvm/pr-subscribers-flang-runtime

Author: Peter Klausler (klausler)

Changes

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 #111404.


Full diff: https://github.com/llvm/llvm-project/pull/111454.diff

2 Files Affected:

  • (modified) flang/runtime/io-stmt.cpp (+9-6)
  • (modified) flang/runtime/io-stmt.h (+2)
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>

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 llvm#111404.
@klausler klausler merged commit c893e3d into llvm:main Oct 10, 2024
9 checks passed
@klausler klausler deleted the bug111404 branch October 10, 2024 17:25
ericastor pushed a commit to ericastor/llvm-project that referenced this pull request Oct 10, 2024
…11454)

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 llvm#111404.
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…11454)

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 llvm#111404.
edoapra added a commit to edoapra/nwchem that referenced this pull request Nov 6, 2024
edoapra added a commit to edoapra/nwchem that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:runtime flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Flang] flang-new-20 Segmentation fault with open(,status=old) of non existing file when setting MALLOC_PERTURB

3 participants