Bug 1463706 - Inconsistency when switching from read to write in std::fstream
Inconsistency when switching from read to write in std::fstream
Status: ASSIGNED
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: gcc (Show other bugs)
7.2
All Linux
high Severity medium
: rc
: ---
Assigned To: Jonathan Wakely
Michael Petlan
:
Depends On:
Blocks: 1420851 1471969
  Show dependency treegraph
 
Reported: 2017-06-21 10:20 EDT by Paulo Andrade
Modified: 2017-08-07 17:25 EDT (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
GNU Compiler Collection 81395 None None None 2017-07-11 11:45 EDT

  None (edit)
Description Paulo Andrade 2017-06-21 10:20:37 EDT
A test program crashes with a stack overflow, for example on
Fedora 25:
#0  0x00007ffff7b3ec70 in std::basic_filebuf<char, std::char_traits<char> >::_M_terminate_output (this=this@entry=0x7fffffffdc38)
    at /usr/src/debug/gcc-6.3.1-20161221/obj-x86_64-redhat-linux/x86_64-redhat-linux/libstdc++-v3/include/bits/fstream.tcc:925
#1  0x00007ffff7b3eccb in std::basic_filebuf<char, std::char_traits<char> >::_M_seek (this=this@entry=0x7fffffffdc38, __off=0, __way=__way@entry=std::_S_cur, __state=...)
    at /usr/src/debug/gcc-6.3.1-20161221/obj-x86_64-redhat-linux/x86_64-redhat-linux/libstdc++-v3/include/bits/fstream.tcc:878
#2  0x00007ffff7b3ee11 in std::basic_filebuf<char, std::char_traits<char> >::overflow (this=0x7fffffffdc38, __c=-1)
    at /usr/src/debug/gcc-6.3.1-20161221/obj-x86_64-redhat-linux/x86_64-redhat-linux/libstdc++-v3/include/bits/fstream.tcc:519
#3  0x00007ffff7b3ec73 in std::basic_filebuf<char, std::char_traits<char> >::_M_terminate_output (this=this@entry=0x7fffffffdc38)
    at /usr/src/debug/gcc-6.3.1-20161221/obj-x86_64-redhat-linux/x86_64-redhat-linux/libstdc++-v3/include/bits/fstream.tcc:925

  The C++ version is :
"""
#include <iostream>
#include <cstring>

int main(int, char**) {
	std::fstream s;
	s.open("test.txt", std::ios_base::in | std::ios_base::out | std::ios_base::binary);

	char data[8 * 1024];
	memset(data, 'A', sizeof(data));
	s.write(data, sizeof(data));

	s.seekg(0);

	char buf[8 * 1024];
	memset(buf, 0, sizeof(buf));
	s.read(buf, sizeof(buf));

	char data2[8 * 1024];
	memset(data2, 'B', sizeof(data2));
	s.write(data2, sizeof(data2));

	return 0;
}
"""
(might need to first create an empty test.txt file)

  But the equivalent C version works as expected:
"""
#include <stdio.h>
#include <string.h>

int main(int argc, char *argv[]) {
	FILE	*s;
	s = fopen("ctest.txt", "w+");
	char data[8 * 1024];
	memset(data, 'A', sizeof(data));
	fwrite(data, sizeof(data), 1, s);

	rewind(s);

	char buf[8 * 1024];
	memset(buf, 0, sizeof(buf));
	fread(buf, sizeof(buf), 1, s);

	char data2[8 * 1024];
	memset(data2, 'B', sizeof(data2));
	fwrite(data2, sizeof(data2), 1, s);
	fclose(s);

	return 0;
}
"""

  The C++ version requires keeping track of the "implicit" offset,
and doing an absolute seek (and calculating it for reads) when
switching from read to write, to avoid the crash.
Comment 2 Christian Horn 2017-06-21 20:55:00 EDT
For me, the C++ code in the description does not compile.
I used this:
"""
#include <fstream>
#include <cstring>
  
int main () {
  std::fstream s;
  s.open ("test.txt", std::ios_base::in | std::ios_base::out | std::ios_base::binary);
  
  char data[8 * 1024];
  memset(data, 'A', sizeof(data));
  s.write(data, sizeof(data));
  
  s.seekg(0);
  
  char buf[8 * 1024];
  memset(buf, 0, sizeof(buf));
  s.read(buf, sizeof(buf));

  // s.seekg(8 * 1024);
  char data2[8 * 1024];
  memset(data2, 'B', sizeof(data2));
  s.write(data2, sizeof(data2));
  
  s.close();
  
  return 0;
}
"""

When removing the comment from the second s.seekg(), then the code does no longer segfault.
Comment 3 Jos Collin 2017-06-22 05:07:05 EDT
An additional information:
Read a character less and it won't segfault.
s.read(buf, sizeof(buf)-1);

As far as I have researched about this, Bidirection file streams require setting the output position indicator for the write after a read to get the desired results.
Comment 13 Jonathan Wakely 2017-07-10 16:52:49 EDT
(In reply to Jos Collin from comment #3)
> As far as I have researched about this, Bidirection file streams require
> setting the output position indicator for the write after a read to get the
> desired results.

That's correct. Mixing reads and writes without intervening seeks or flushes is undefined, which is why the fstream gets into a corrupt state.

I have a prototype patch that prevents the recursion, and should only affect behaviour in undefined cases like this. It won't make the program correct, but it will prevent the infinite recursion that overflows the stack.
Comment 14 Christian Horn 2017-07-10 21:08:14 EDT
(In reply to Jonathan Wakely from comment #13)
> I have a prototype patch that prevents the recursion, and should only affect
> behaviour in undefined cases like this. It won't make the program correct,
> but it will prevent the infinite recursion that overflows the stack.

From a user perspective, I think the most desirable behaviour would be to detect the situation already at compile time and bail out.  Not sure if that detection is possible (or allowed by conventions/standards).
Comment 17 Jonathan Wakely 2017-07-11 04:47:42 EDT
(In reply to Christian Horn from comment #14)
> From a user perspective, I think the most desirable behaviour would be to
> detect the situation already at compile time and bail out.  Not sure if that
> detection is possible (or allowed by conventions/standards).

That's impossible.
Comment 18 Jonathan Wakely 2017-07-18 20:33:10 EDT
Although the C++ standard says that the seeks are required for correct behaviour, changes were to GCC's fstream code several years ago to insert the seeks automatically as needed. Although relying on this isn't necessarily portable to other implementations, GCC tries to make it work. However, the customer's testcase hits an edge case where the automatic seeking isn't done correctly and leads to a crash.

This is now fixed in upstream GCC, in a way that makes the original testcase work as expected: no intervening seek is needed between the read and write.

Note You need to log in before you can comment on or make changes to this bug.