[PATCH] libstdc++: Make basic_stringbuf use initial SSO capacity [PR80676]

[Jonathan Wakely]

On Fri, 31 Oct 2025 at 18:00 +0000, Jonathan Wakely wrote:
>On Mon, 27 Oct 2025 at 22:46 -0400, Patrick Palka wrote:
>>From: Jonathan Wakely <jwakely@redhat.com>
>>
>>Tested on x86_64-pc-linux-gnu.  Patch originally written by Jonathan and
>>completed by me (mainly testsuite adjustments).
>
>Thanks for taking on the task of figuring out what needing changing in
>the tests. IIRC I made the changes to stringbuf and when tests failed
>in unexpected ways I got confused and gave up.
>
>>I don't understand the stringbuf API enough to determine what the
>>correct value of egptr() - eback() (i.e. the get area size) should be in
>>the 26250.cc tests aftering calling overflow().  It's currently zero
>>after this patch, which I'm pretty sure is wrong?
>>
>>[stringbuf.virtuals]/8 says: If ios_base::in is set in mode, the
>>function alters the read end pointer egptr() to point just past the new
>>write position.
>>
>>In light of that I guess egptr() - eback() should still be 1 after this
>>patch then?  That'd require making stringbuf::overflow() more
>>consistently set egptr() I believe.
>
>As written, I think the "alters the read end pointer egptr()" wording
>only applies in the case where overflow needs to make a new write
>position available, i.e. needs to reallocate.
>
>When we've been initialized with the SSO buffer the initial capacity
>means that the single call to overflow in 26250.cc doesn't need to
>reallocate.  So I think it's correct that after your change, the size
>of the get area isn't change by the overflow('x') call. Because it
>doesn't reallocate.
>
>>But then what about for:
>>
>> buf.sputn("01234567890abcdef", 16); // Exceed SSO buffer
>> buf.overflow('x');
>> VERIFY( buf.egptr() - buf.eback() == ? );
>>
>>I would think the get area size should be 17? But other implementations
>>including us before this patch say 16, so I'm pretty confused.
>
>Again, I think this depends on whether overflow('x') reallocates. If
>the capacity after sputn means a write position is available, then my
>reading of the spec says we don't change egptr().
>
>But that doesn't really make sense to me. Surely the point of moving
>the egptr is that a character has been written to the put area and
>that character should now be available for reading, so the get area
>should expand to include the just-written character. But that should
>happen any time a character is written to the put area, not only when
>reallocation was needed?

Ah, but the newly-written character is made available, because the
high_mark is moved when writing a new character. The egptr() only
needs to be adjusted when reallocating.

I think what we're doing is correct, and your changes to the tests are
the right ones to adjust to the new expected behaviour.

It might be good if the 26250.cc tests were adjusted to still test
that egptr is moved after reallocating, since that's what it is
intended to test. It's just that with your changes, the test no longer
reallocates.

So something like this (untested!)

#if _GLIBCXX_USE_CXX11_ABI
   // No reallocation happened on overflow above.
   VERIFY( buf.egptr() - buf.eback() == 0 );

   // Fill the write area and then overflow again:
   std::string fill(write_positions, 'x');
   buf.sputn(fill.c_str(), fill.size());
   std::ptrdiff_t get_size = buf.egptr() - buf.eback();
   buf.overflow('x');
   VERIFY( buf.egptr() - buf.eback() == get_size + 1 );
#else
   VERIFY( buf.egptr() - buf.eback() == 1 );
#endif


This assumes the following change earlier in the test:

    // not required but good for efficiency
    // NB: we are implementing DR 169 and DR 432
-  const int write_positions = buf.epptr() - buf.pbase();
+  const int write_positions = buf.epptr() - buf.pptr();
    VERIFY( write_positions > 1 );

    // 27.7.1.3, p8:

Because surely the number of available write positions is the number
of positions after pptr(), not the number of positions after pbase()?