Skip to content
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

[flang][runtime] Fix BACKSPACE-WRITE on variable-length unformatted file #72732

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

klausler
Copy link
Contributor

A subtle bug in buffer management is being caused by a WRITE on an unformatted file with variable-length records after one or more BACKSPACEs and some READs. An attempt at a minor optimization in BACKSPACE kept the footer of the previous record in the unit's buffer, in case more BACKSPACEs were to follow. If a later WRITE takes place instead, the buffer's frame would still cover that footer, but its content would be lost when the buffer became dirty on its first modification, and the footer in the file would be overwritten with stale buffer contents. As WriteFrame() implies the intent to define all the bytes in the identified range, the trick being used in BACKSPACE with its adjustment to frameOffsetInFile_ breaks any later WRITE... and so we just can't do it.

Fixes #72599.

A subtle bug in buffer management is being caused by a WRITE
on an unformatted file with variable-length records after one
or more BACKSPACEs and some READs.  An attempt at a minor
optimization in BACKSPACE kept the footer of the previous
record in the unit's buffer, in case more BACKSPACEs were
to follow.  If a later WRITE takes place instead, the buffer's
frame would still cover that footer, but its content would be
lost when the buffer became dirty on its first modification,
and the footer in the file would be overwritten with stale
buffer contents.  As WriteFrame() implies the intent to define
all the bytes in the identified range, the trick being used in
BACKSPACE with its adjustment to frameOffsetInFile_ breaks any
later WRITE... and so we just can't do it.

Fixes llvm#72599.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 18, 2023

@llvm/pr-subscribers-flang-runtime

Author: Peter Klausler (klausler)

Changes

A subtle bug in buffer management is being caused by a WRITE on an unformatted file with variable-length records after one or more BACKSPACEs and some READs. An attempt at a minor optimization in BACKSPACE kept the footer of the previous record in the unit's buffer, in case more BACKSPACEs were to follow. If a later WRITE takes place instead, the buffer's frame would still cover that footer, but its content would be lost when the buffer became dirty on its first modification, and the footer in the file would be overwritten with stale buffer contents. As WriteFrame() implies the intent to define all the bytes in the identified range, the trick being used in BACKSPACE with its adjustment to frameOffsetInFile_ breaks any later WRITE... and so we just can't do it.

Fixes #72599.


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

1 Files Affected:

  • (modified) flang/runtime/unit.cpp (-4)
diff --git a/flang/runtime/unit.cpp b/flang/runtime/unit.cpp
index dec8d8032f6155e..995656b9480c414 100644
--- a/flang/runtime/unit.cpp
+++ b/flang/runtime/unit.cpp
@@ -825,10 +825,6 @@ void ExternalFileUnit::BackspaceVariableUnformattedRecord(
     return;
   }
   frameOffsetInFile_ -= *recordLength + 2 * headerBytes;
-  if (frameOffsetInFile_ >= headerBytes) {
-    frameOffsetInFile_ -= headerBytes;
-    recordOffsetInFrame_ = headerBytes;
-  }
   auto need{static_cast<std::size_t>(
       recordOffsetInFrame_ + sizeof header + *recordLength)};
   got = ReadFrame(frameOffsetInFile_, need, handler);

@klausler klausler merged commit bddf5d2 into llvm:main Nov 30, 2023
5 checks passed
@klausler klausler deleted the bug72599 branch November 30, 2023 20:34
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
3 participants