Skip to content

[flang][runtime] Fix runtime crash after bad recoverable OPEN #111454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 10, 2024

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