diff mbox

PR libstdc++/81395 fix crash when write follows large read

Message ID 20170719002256.GD15340@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely July 19, 2017, 12:22 a.m. UTC 
On 19/07/17 01:17 +0100, Jonathan Wakely wrote:
>This fixes a crash that happens in std::filebuf when a large read
>consumes the entire get area and is followed by a write, which is then
>synced to the file by a call to overflow.
>
>The problem is that xsgetn calls _M_set_buffer(0) after reading from
>the file (i.e. when in 'read' mode). As the comments on _M_set_buffer
>say, an argument of 0 is used for 'write' mode. This causes the
>filebuf to have an active put area while in 'read' mode, so that the
>next write inserts straight into that put area, rather than performing
>the required seek to leave 'read' mode.
>
>The next overflow then tries to leave 'read' mode by doing a seek, but
>that then tries to flush the non-empty put area by calling overflow,
>which goes into a loop until we overflow the stack.
>
>The solution is to simply remove the call to _M_set_buffer(0). It's
>not needed because the buffers are already set up appropriately after
>xsgetn has read from the file: there's no active putback, no put area,
>and setg(eback(), egptr(), egptr()) has been called so there's nothing
>available in the get area. All we need to do is set _M_reading = true
>so that a following write knows it needs to perform a seek.
>
>The new testcase passes with GCC 4.5, so this is technically a
>regression. However, I have a more demanding test that fails even with
>GCC 4.5, so I don't think mixing reads and writes without intervening
>seeks was ever working completely. I hope it is now.
>
>I spent a LOT of time checking the make check-performance results
>before and after this patch (and with various other attempted fixes)
>and any difference seemed to be noise.
>
>	PR libstdc++/81395
>	* include/bits/fstream.tcc (basic_filebuf::xsgetn): Don't set buffer
>	pointers for write mode after reading.
>	* testsuite/27_io/basic_filebuf/sgetn/char/81395.cc: New.

The new test needs this dg-require so it doesn't FAIL on target boards
with no file I/O, and the dg-do is redundant.

Committed to trunk.
diff mbox

Patch

commit e868d4e4a67faa9b889720b5fcdd10f5eb0f4fa8
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jul 19 01:19:20 2017 +0100

    Use dg-require-fileio in new test
    
    	* testsuite/27_io/basic_filebuf/sgetn/char/81395.cc: Add dg-require.

diff --git a/libstdc++-v3/testsuite/27_io/basic_filebuf/sgetn/char/81395.cc b/libstdc++-v3/testsuite/27_io/basic_filebuf/sgetn/char/81395.cc
index 4985628..ea8dbc1 100644
--- a/libstdc++-v3/testsuite/27_io/basic_filebuf/sgetn/char/81395.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_filebuf/sgetn/char/81395.cc
@@ -15,7 +15,7 @@ 
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// { dg-do run }
+// { dg-require-fileio "" }
 
 // PR libstdc++/81395