Re: [PATCH] PR libstdc++/81395 fix crash when write follows large read

[Jonathan Wakely <jwakely at redhat dot com>]

On 19/07/17 20:15 +0200, Paolo Carlini wrote:
> Hi,
>
> On 19/07/2017 02:17, 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.
> Excellent work on this, thanks Jon. Time permitting, I mean to 
> investigate a bit more your last interesting observation: I suspected 
> that the crazy _M_set_buffer (0) call or the idea itself of calling 
> _M_set_buffer in this case, was coming somehow from previous 
> incarnations of the buffer, which was already "suboptimal" in this 
> respect and didn't make progress... until today ;)

This is the more demanding test I mentioned, which doesn't work with
GCC 4.5 (even when the seekg and flush lines are uncommented).

It works correctly with trunk, allowing switching between reads and
writes without seeking or flushing.

It works correctly with libc++ but only when the seek and flush are
uncommented.

If I find time I want to introduce more variation into the pattern of
reads and write, to exercise difference code paths.

Attachment: fbrand.cc
Description: Text document