public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: [committed] libstdc++: Fix std::basic_string::resize_and_overwrite
Date: Wed, 16 Aug 2023 20:04:07 +0100	[thread overview]
Message-ID: <20230816190453.709520-1-jwakely@redhat.com> (raw)

Tested x86_64-linux, pushed to trunk. This should be backported to
gcc-12 and gcc-13 too (without the std::format test changes).

-- >8 --

The callable used for resize_and_overwrite was being passed the string's
expanded capacity, which might be greater than the new size being
requested. This is not conforming, as the standard requires the same n
to be passed to the callable that the user passed to
resize_and_overwrite.

The existing tests didn't catch this because they all used a value which
was more than twice the existing capacity, so the _M_create call
allocated exactly what was requested, and the value passed to the
callable was correct. But when the requested size is greater than the
current capacity but smaller than twice the current capacity, _M_create
will allocate twice the current capacity and then that value was being
passed to the callable.

I noticed this because std::format(L"{}", 0.25) was producing L"0.25XX"
where the XX characters were whatever happened to be on the stack before
the call. When std::format used resize_and_overwrite to widen a string
it was copying too many characters into the destination and setting the
result's length too long. I've added a test for this case, and a new
test that doesn't hardcode -std=gnu++20 so can be used to test
std::format in C++23 and C++26 modes.

libstdc++-v3/ChangeLog:

	* include/bits/basic_string.tcc (resize_and_overwrite): Invoke
	the callable with the same size as resize_and_overwrite was
	called with.
	* testsuite/21_strings/basic_string/capacity/char/resize_and_overwrite.cc:
	Check with small values for the new size.
	* testsuite/std/format/functions/format.cc: Check wide
	formatting of double values that produce small strings.
	* testsuite/std/format/functions/format_c++23.cc: New test.
---
 libstdc++-v3/include/bits/basic_string.tcc    |  7 ++++---
 .../capacity/char/resize_and_overwrite.cc     | 21 +++++++++++++++++++
 .../testsuite/std/format/functions/format.cc  |  5 +++++
 .../std/format/functions/format_c++23.cc      |  4 ++++
 4 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/std/format/functions/format_c++23.cc

diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index d8a279fc9ed..c759c2f9525 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -566,13 +566,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Operation>
     constexpr void
     basic_string<_CharT, _Traits, _Alloc>::
-    resize_and_overwrite(size_type __n, _Operation __op)
+    resize_and_overwrite(const size_type __n, _Operation __op)
     {
       const size_type __capacity = capacity();
       _CharT* __p;
       if (__n > __capacity)
 	{
-	  __p = _M_create(__n, __capacity);
+	  auto __new_capacity = __n; // Must not allow _M_create to modify __n.
+	  __p = _M_create(__new_capacity, __capacity);
 	  this->_S_copy(__p, _M_data(), length()); // exclude trailing null
 #if __cpp_lib_is_constant_evaluated
 	  if (std::is_constant_evaluated())
@@ -580,7 +581,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 	  _M_dispose();
 	  _M_data(__p);
-	  _M_capacity(__n);
+	  _M_capacity(__new_capacity);
 	}
       else
 	__p = _M_data();
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/resize_and_overwrite.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/resize_and_overwrite.cc
index f716030dad7..0ea5e2b10ef 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/resize_and_overwrite.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/resize_and_overwrite.cc
@@ -120,6 +120,26 @@ test05()
   return true;
 }
 
+void
+test06()
+{
+  std::string s = "0123456789";
+  s.resize_and_overwrite(16, [](char* p, int n) {
+    VERIFY( n == 16 );
+    std::char_traits<char>::copy(p + 10, "0123456798", 6);
+    return n;
+  });
+  VERIFY( s.size() == 16 );
+  VERIFY( s == "0123456789012345" );
+
+  s.resize_and_overwrite(4, [](char* p, int n) {
+    VERIFY( n == 4 );
+    std::char_traits<char>::copy(p, "abcd", 4);
+    return n;
+  });
+  VERIFY( s.size() == 4 );
+}
+
 int main()
 {
   test01();
@@ -127,4 +147,5 @@ int main()
   test03();
   test04();
   static_assert( test05() );
+  test06();
 }
diff --git a/libstdc++-v3/testsuite/std/format/functions/format.cc b/libstdc++-v3/testsuite/std/format/functions/format.cc
index 471cffb2b36..a8d5b652a5e 100644
--- a/libstdc++-v3/testsuite/std/format/functions/format.cc
+++ b/libstdc++-v3/testsuite/std/format/functions/format.cc
@@ -256,6 +256,11 @@ test_wchar()
   std::locale loc;
   s = std::format(loc, L"{:L} {:.3s}{:Lc}", true, L"data"sv, '.');
   VERIFY( s == L"true dat." );
+
+  s = std::format(L"{}", 0.0625);
+  VERIFY( s == L"0.0625" );
+  s = std::format(L"{}", 0.25);
+  VERIFY( s == L"0.25" );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/std/format/functions/format_c++23.cc b/libstdc++-v3/testsuite/std/format/functions/format_c++23.cc
new file mode 100644
index 00000000000..f20c46cd7e3
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/format/functions/format_c++23.cc
@@ -0,0 +1,4 @@
+// { dg-do run { target c++23 } }
+// This test does not have -std=gnu++20 in dg-options so that format.cc
+// can be tested for e.g. -std=c++26
+#include "format.cc"
-- 
2.41.0


                 reply	other threads:[~2023-08-16 19:04 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230816190453.709520-1-jwakely@redhat.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).