diff mbox series

libstdc++: Fix std::basic_stringbuf::str()&& [PR123100]

Message ID 20260107115258.1822816-1-jwakely@redhat.com
State New
Headers
Series libstdc++: Fix std::basic_stringbuf::str()&& [PR123100] |

Commit Message

Jonathan Wakely Jan. 7, 2026, 11:52 a.m. UTC 
  When basic_stringbuf::setbuf has been called we need to copy the
contents of the buffer into _M_string first, before returning that.

libstdc++-v3/ChangeLog:

	PR libstdc++/123100
	* include/std/sstream (basic_stringbuf::str()&&): Handle the
	case where _M_string is not being used for the buffer.
	* testsuite/27_io/basic_stringbuf/str/char/123100.cc: New test.
---

Tested x86_64-linux.

 libstdc++-v3/include/std/sstream              |  9 ++-
 .../27_io/basic_stringbuf/str/char/123100.cc  | 58 +++++++++++++++++++
 2 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc

Comments

Jonathan Wakely Jan. 7, 2026, 11:58 a.m. UTC | #1 
On Wed, 7 Jan 2026 at 11:56, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> When basic_stringbuf::setbuf has been called we need to copy the
> contents of the buffer into _M_string first, before returning that.
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/123100
>         * include/std/sstream (basic_stringbuf::str()&&): Handle the
>         case where _M_string is not being used for the buffer.
>         * testsuite/27_io/basic_stringbuf/str/char/123100.cc: New test.
> ---
>
> Tested x86_64-linux.
>
>  libstdc++-v3/include/std/sstream              |  9 ++-
>  .../27_io/basic_stringbuf/str/char/123100.cc  | 58 +++++++++++++++++++
>  2 files changed, 64 insertions(+), 3 deletions(-)
>  create mode 100644 libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
>
> diff --git a/libstdc++-v3/include/std/sstream b/libstdc++-v3/include/std/sstream
> index 4a5c090f97ba..f036165909df 100644
> --- a/libstdc++-v3/include/std/sstream
> +++ b/libstdc++-v3/include/std/sstream
> @@ -298,7 +298,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>  #if __cplusplus > 201703L
>  #if _GLIBCXX_USE_CXX11_ABI
>  #if __cpp_concepts
> -       // P0407 Allocator-aware basic_streambuf
> +      // P0407 Allocator-aware basic_streambuf

This is just a whitespace fix in the vicinity of the bug fix.


>        template<__allocator_like _SAlloc>
>         _GLIBCXX_NODISCARD
>         basic_string<_CharT, _Traits, _SAlloc>
> @@ -315,8 +315,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        {
>         if (char_type* __hi = _M_high_mark())
>           {
> -           // Set length to end of character sequence and add null terminator.
> -           _M_string._M_set_length(_M_high_mark() - this->pbase());
> +           if (_M_string.c_str() == this->pbase()) [[likely]]
> +             // Set length to end of sequence and add null terminator.
> +             _M_string._M_set_length(__hi - this->pbase());
> +           else
> +             _M_string.assign(this->pbase(), __hi);
>           }
>         auto __str = std::move(_M_string);
>         _M_string.clear();
> diff --git a/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
> new file mode 100644
> index 000000000000..174fe0ece1d6
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
> @@ -0,0 +1,58 @@
> +// { dg-do run }
> +
> +#include <sstream>
> +#include <testsuite_hooks.h>
> +
> +void
> +test01()
> +{
> +  const int n = 20;
> +  const char data[n] = "abcde";
> +  char buf[n] = "0123456789";
> +  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
> +
> +  std::ostringstream out;
> +  out.rdbuf()->pubsetbuf(buf, n);
> +  out << data;
> +  VERIFY( out.str() == expected );
> +  VERIFY( out.str() == expected );
> +#if __cplusplus >= 201103L
> +  VERIFY( std::move(out).str() == expected );
> +#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
> +  expected.clear();
> +#endif
> +  VERIFY( out.str() == expected );
> +  VERIFY( std::move(out).str() == expected );
> +#endif
> +}
> +
> +void
> +test02()
> +{
> +  const int n = 20;
> +  const char data[n] = "abcde";
> +  char buf[n] = "0123456789";
> +  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
> +
> +  std::ostringstream out;
> +  out << std::string(n * 2, 'a');
> +  VERIFY( out.str() == std::string(n * 2, 'a') );
> +  out.rdbuf()->pubsetbuf(buf, n);
> +  out << data; // writes 6 chars
> +  VERIFY( out.str() == expected );
> +  VERIFY( out.str() == expected );
> +#if __cplusplus >= 201103L
> +  VERIFY( std::move(out).str() == expected );
> +#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
> +  expected.clear();
> +#endif
> +  VERIFY( out.str() == expected );
> +  VERIFY( std::move(out).str() == expected );
> +#endif
> +}
> +
> +int main()
> +{
> +  test01();
> +  test02();
> +}
> --
> 2.52.0
>
Tomasz Kaminski Jan. 7, 2026, 1:50 p.m. UTC | #2 
On Wed, Jan 7, 2026 at 12:56 PM Jonathan Wakely <jwakely@redhat.com> wrote:

> When basic_stringbuf::setbuf has been called we need to copy the
> contents of the buffer into _M_string first, before returning that.
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/123100
>         * include/std/sstream (basic_stringbuf::str()&&): Handle the
>         case where _M_string is not being used for the buffer.
>         * testsuite/27_io/basic_stringbuf/str/char/123100.cc: New test.
> ---
>
> Tested x86_64-linux.
>
LGTM. One small subjective and stylistic suggestion.

>
>  libstdc++-v3/include/std/sstream              |  9 ++-
>  .../27_io/basic_stringbuf/str/char/123100.cc  | 58 +++++++++++++++++++
>  2 files changed, 64 insertions(+), 3 deletions(-)
>  create mode 100644
> libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
>
> diff --git a/libstdc++-v3/include/std/sstream
> b/libstdc++-v3/include/std/sstream
> index 4a5c090f97ba..f036165909df 100644
> --- a/libstdc++-v3/include/std/sstream
> +++ b/libstdc++-v3/include/std/sstream
> @@ -298,7 +298,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>  #if __cplusplus > 201703L
>  #if _GLIBCXX_USE_CXX11_ABI
>  #if __cpp_concepts
> -       // P0407 Allocator-aware basic_streambuf
> +      // P0407 Allocator-aware basic_streambuf
>        template<__allocator_like _SAlloc>
>         _GLIBCXX_NODISCARD
>         basic_string<_CharT, _Traits, _SAlloc>
> @@ -315,8 +315,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        {
>         if (char_type* __hi = _M_high_mark())
>           {
> -           // Set length to end of character sequence and add null
> terminator.
> -           _M_string._M_set_length(_M_high_mark() - this->pbase());
> +           if (_M_string.c_str() == this->pbase()) [[likely]]
>
I would preffer _M_string.data(), as we are already in C++17 mode.

> +             // Set length to end of sequence and add null terminator.
> +             _M_string._M_set_length(__hi - this->pbase());
> +           else
> +             _M_string.assign(this->pbase(), __hi);
>           }
>         auto __str = std::move(_M_string);
>         _M_string.clear();
> diff --git
> a/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
> b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
> new file mode 100644
> index 000000000000..174fe0ece1d6
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
> @@ -0,0 +1,58 @@
> +// { dg-do run }
> +
> +#include <sstream>
> +#include <testsuite_hooks.h>
> +
> +void
> +test01()
> +{
> +  const int n = 20;
> +  const char data[n] = "abcde";
> +  char buf[n] = "0123456789";
> +  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
> +
> +  std::ostringstream out;
> +  out.rdbuf()->pubsetbuf(buf, n);
> +  out << data;
> +  VERIFY( out.str() == expected );
> +  VERIFY( out.str() == expected );
> +#if __cplusplus >= 201103L
> +  VERIFY( std::move(out).str() == expected );
> +#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
> +  expected.clear();
> +#endif
> +  VERIFY( out.str() == expected );
> +  VERIFY( std::move(out).str() == expected );
> +#endif
> +}
> +
> +void
> +test02()
> +{
> +  const int n = 20;
> +  const char data[n] = "abcde";
> +  char buf[n] = "0123456789";
> +  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
> +
> +  std::ostringstream out;
> +  out << std::string(n * 2, 'a');
> +  VERIFY( out.str() == std::string(n * 2, 'a') );
> +  out.rdbuf()->pubsetbuf(buf, n);
> +  out << data; // writes 6 chars
> +  VERIFY( out.str() == expected );
> +  VERIFY( out.str() == expected );
> +#if __cplusplus >= 201103L
> +  VERIFY( std::move(out).str() == expected );
> +#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
> +  expected.clear();
> +#endif
> +  VERIFY( out.str() == expected );
> +  VERIFY( std::move(out).str() == expected );
> +#endif
> +}
> +
> +int main()
> +{
> +  test01();
> +  test02();
> +}
> --
> 2.52.0
>
>
Jonathan Wakely Jan. 7, 2026, 2:10 p.m. UTC | #3 
On Wed, 7 Jan 2026 at 13:51, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>
>
>
> On Wed, Jan 7, 2026 at 12:56 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> When basic_stringbuf::setbuf has been called we need to copy the
>> contents of the buffer into _M_string first, before returning that.
>>
>> libstdc++-v3/ChangeLog:
>>
>>         PR libstdc++/123100
>>         * include/std/sstream (basic_stringbuf::str()&&): Handle the
>>         case where _M_string is not being used for the buffer.
>>         * testsuite/27_io/basic_stringbuf/str/char/123100.cc: New test.
>> ---
>>
>> Tested x86_64-linux.
>
> LGTM. One small subjective and stylistic suggestion.
>>
>>
>>  libstdc++-v3/include/std/sstream              |  9 ++-
>>  .../27_io/basic_stringbuf/str/char/123100.cc  | 58 +++++++++++++++++++
>>  2 files changed, 64 insertions(+), 3 deletions(-)
>>  create mode 100644 libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
>>
>> diff --git a/libstdc++-v3/include/std/sstream b/libstdc++-v3/include/std/sstream
>> index 4a5c090f97ba..f036165909df 100644
>> --- a/libstdc++-v3/include/std/sstream
>> +++ b/libstdc++-v3/include/std/sstream
>> @@ -298,7 +298,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>>  #if __cplusplus > 201703L
>>  #if _GLIBCXX_USE_CXX11_ABI
>>  #if __cpp_concepts
>> -       // P0407 Allocator-aware basic_streambuf
>> +      // P0407 Allocator-aware basic_streambuf
>>        template<__allocator_like _SAlloc>
>>         _GLIBCXX_NODISCARD
>>         basic_string<_CharT, _Traits, _SAlloc>
>> @@ -315,8 +315,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>>        {
>>         if (char_type* __hi = _M_high_mark())
>>           {
>> -           // Set length to end of character sequence and add null terminator.
>> -           _M_string._M_set_length(_M_high_mark() - this->pbase());
>> +           if (_M_string.c_str() == this->pbase()) [[likely]]
>
> I would preffer _M_string.data(), as we are already in C++17 mode.

Good idea. That's what's used later in the function too.

>>
>> +             // Set length to end of sequence and add null terminator.
>> +             _M_string._M_set_length(__hi - this->pbase());
>> +           else
>> +             _M_string.assign(this->pbase(), __hi);
>>           }
>>         auto __str = std::move(_M_string);
>>         _M_string.clear();
>> diff --git a/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
>> new file mode 100644
>> index 000000000000..174fe0ece1d6
>> --- /dev/null
>> +++ b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
>> @@ -0,0 +1,58 @@
>> +// { dg-do run }
>> +
>> +#include <sstream>
>> +#include <testsuite_hooks.h>
>> +
>> +void
>> +test01()
>> +{
>> +  const int n = 20;
>> +  const char data[n] = "abcde";
>> +  char buf[n] = "0123456789";
>> +  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
>> +
>> +  std::ostringstream out;
>> +  out.rdbuf()->pubsetbuf(buf, n);
>> +  out << data;
>> +  VERIFY( out.str() == expected );
>> +  VERIFY( out.str() == expected );
>> +#if __cplusplus >= 201103L
>> +  VERIFY( std::move(out).str() == expected );
>> +#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
>> +  expected.clear();
>> +#endif
>> +  VERIFY( out.str() == expected );
>> +  VERIFY( std::move(out).str() == expected );
>> +#endif
>> +}
>> +
>> +void
>> +test02()
>> +{
>> +  const int n = 20;
>> +  const char data[n] = "abcde";
>> +  char buf[n] = "0123456789";
>> +  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
>> +
>> +  std::ostringstream out;
>> +  out << std::string(n * 2, 'a');
>> +  VERIFY( out.str() == std::string(n * 2, 'a') );
>> +  out.rdbuf()->pubsetbuf(buf, n);
>> +  out << data; // writes 6 chars
>> +  VERIFY( out.str() == expected );
>> +  VERIFY( out.str() == expected );
>> +#if __cplusplus >= 201103L
>> +  VERIFY( std::move(out).str() == expected );
>> +#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
>> +  expected.clear();
>> +#endif
>> +  VERIFY( out.str() == expected );
>> +  VERIFY( std::move(out).str() == expected );
>> +#endif
>> +}
>> +
>> +int main()
>> +{
>> +  test01();
>> +  test02();
>> +}
>> --
>> 2.52.0
>>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/sstream b/libstdc++-v3/include/std/sstream
index 4a5c090f97ba..f036165909df 100644
--- a/libstdc++-v3/include/std/sstream
+++ b/libstdc++-v3/include/std/sstream
@@ -298,7 +298,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 #if __cplusplus > 201703L
 #if _GLIBCXX_USE_CXX11_ABI
 #if __cpp_concepts
-	// P0407 Allocator-aware basic_streambuf
+      // P0407 Allocator-aware basic_streambuf
       template<__allocator_like _SAlloc>
 	_GLIBCXX_NODISCARD
 	basic_string<_CharT, _Traits, _SAlloc>
@@ -315,8 +315,11 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
       {
 	if (char_type* __hi = _M_high_mark())
 	  {
-	    // Set length to end of character sequence and add null terminator.
-	    _M_string._M_set_length(_M_high_mark() - this->pbase());
+	    if (_M_string.c_str() == this->pbase()) [[likely]]
+	      // Set length to end of sequence and add null terminator.
+	      _M_string._M_set_length(__hi - this->pbase());
+	    else
+	      _M_string.assign(this->pbase(), __hi);
 	  }
 	auto __str = std::move(_M_string);
 	_M_string.clear();
diff --git a/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
new file mode 100644
index 000000000000..174fe0ece1d6
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_stringbuf/str/char/123100.cc
@@ -0,0 +1,58 @@ 
+// { dg-do run }
+
+#include <sstream>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  const int n = 20;
+  const char data[n] = "abcde";
+  char buf[n] = "0123456789";
+  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
+
+  std::ostringstream out;
+  out.rdbuf()->pubsetbuf(buf, n);
+  out << data;
+  VERIFY( out.str() == expected );
+  VERIFY( out.str() == expected );
+#if __cplusplus >= 201103L
+  VERIFY( std::move(out).str() == expected );
+#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
+  expected.clear();
+#endif
+  VERIFY( out.str() == expected );
+  VERIFY( std::move(out).str() == expected );
+#endif
+}
+
+void
+test02()
+{
+  const int n = 20;
+  const char data[n] = "abcde";
+  char buf[n] = "0123456789";
+  std::string expected("abcde56789\0\0\0\0\0\0\0\0\0\0", n);
+
+  std::ostringstream out;
+  out << std::string(n * 2, 'a');
+  VERIFY( out.str() == std::string(n * 2, 'a') );
+  out.rdbuf()->pubsetbuf(buf, n);
+  out << data; // writes 6 chars
+  VERIFY( out.str() == expected );
+  VERIFY( out.str() == expected );
+#if __cplusplus >= 201103L
+  VERIFY( std::move(out).str() == expected );
+#if __cplusplus >= 202002L && _GLIBCXX_USE_CXX11_ABI
+  expected.clear();
+#endif
+  VERIFY( out.str() == expected );
+  VERIFY( std::move(out).str() == expected );
+#endif
+}
+
+int main()
+{
+  test01();
+  test02();
+}