diff mbox series

[committed,v3] libstdc++: Fix std::string construction from volatile char* [PR119748]

Message ID 20250415082852.2239699-1-jwakely@redhat.com
State New
Headers
Series [committed,v3] libstdc++: Fix std::string construction from volatile char* [PR119748] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply

Commit Message

Jonathan Wakely April 15, 2025, 8:27 a.m. UTC 
  My recent r15-9381-g648d5c26e25497 change assumes that a contiguous
iterator with the correct value_type can be converted to a const charT*
but that's not true for volatile charT*. The optimization should only be
done if it can be converted to the right pointer type.

Additionally, some generic loops for non-contiguous iterators need an
explicit cast to deal with iterator reference types that do not bind to
the const charT& parameter of traits_type::assign.

libstdc++-v3/ChangeLog:

	PR libstdc++/119748
	* include/bits/basic_string.h (_S_copy_chars): Only optimize for
	contiguous iterators that are convertible to const charT*. Use
	explicit conversion to charT after dereferencing iterator.
	(_S_copy_range): Likewise for contiguous ranges.
	* include/bits/basic_string.tcc (_M_construct): Use explicit
	conversion to charT after dereferencing iterator.
	* include/bits/cow_string.h (_S_copy_chars): Likewise.
	(basic_string(from_range_t, R&&, const Allocator&)): Likewise.
	Only optimize for contiguous iterators that are convertible to
	const charT*.
	* testsuite/21_strings/basic_string/cons/char/119748.cc: New
	test.
	* testsuite/21_strings/basic_string/cons/wchar_t/119748.cc:
	New test.

Reviewed-by: Tomasz Kaminski <tkaminsk@redhat.com>
---

Changes in v3:
- Fixed commit message to not talk about iterator references that aren't
  implicitly convertible to value_type.
- Used testsuite_iterators.h for new tests (after enabling the
  test_container(T(&)[N]) constructor for C++98).

Tested x86_64-linux. Pushed to trunk.

The static_cast parts would be OK to backport too.

 libstdc++-v3/include/bits/basic_string.h      | 24 +++++++++----
 libstdc++-v3/include/bits/basic_string.tcc    |  3 +-
 libstdc++-v3/include/bits/cow_string.h        | 17 ++++++---
 .../basic_string/cons/char/119748.cc          | 35 +++++++++++++++++++
 .../basic_string/cons/wchar_t/119748.cc       |  7 ++++
 5 files changed, 73 insertions(+), 13 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc
 create mode 100644 libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 9c431c765ab..c90bd099b63 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -488,8 +488,11 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 			      is_same<_IterBase, const _CharT*>>::value)
 	    _S_copy(__p, std::__niter_base(__k1), __k2 - __k1);
 #if __cpp_lib_concepts
-	  else if constexpr (contiguous_iterator<_Iterator>
-			       && is_same_v<iter_value_t<_Iterator>, _CharT>)
+	  else if constexpr (requires {
+			       requires contiguous_iterator<_Iterator>;
+			       { std::to_address(__k1) }
+				 -> convertible_to<const _CharT*>;
+			     })
 	    {
 	      const auto __d = __k2 - __k1;
 	      (void) (__k1 + __d); // See P3349R1
@@ -499,7 +502,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	  else
 #endif
 	  for (; __k1 != __k2; ++__k1, (void)++__p)
-	    traits_type::assign(*__p, *__k1); // These types are off.
+	    traits_type::assign(*__p, static_cast<_CharT>(*__k1));
 	}
 #pragma GCC diagnostic pop
 
@@ -527,12 +530,19 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	static constexpr void
 	_S_copy_range(pointer __p, _Rg&& __rg, size_type __n)
 	{
-	  if constexpr (ranges::contiguous_range<_Rg>
-			  && is_same_v<ranges::range_value_t<_Rg>, _CharT>)
+	  if constexpr (requires {
+			  requires ranges::contiguous_range<_Rg>;
+			  { ranges::data(std::forward<_Rg>(__rg)) }
+			    -> convertible_to<const _CharT*>;
+			})
 	    _S_copy(__p, ranges::data(std::forward<_Rg>(__rg)), __n);
 	  else
-	    for (auto&& __e : __rg)
-	      traits_type::assign(*__p++, std::forward<decltype(__e)>(__e));
+	    {
+	      auto __first = ranges::begin(__rg);
+	      const auto __last = ranges::end(__rg);
+	      for (; __first != __last; ++__first)
+		traits_type::assign(*__p++, static_cast<_CharT>(*__first));
+	    }
 	}
 #endif
 
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 02230aca5d2..bca55bc5658 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -210,7 +210,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		_M_data(__another);
 		_M_capacity(__capacity);
 	      }
-	    traits_type::assign(_M_data()[__len++], *__beg);
+	    traits_type::assign(_M_data()[__len++],
+				static_cast<_CharT>(*__beg));
 	    ++__beg;
 	  }
 
diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h
index b250397151b..f9df2be20be 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -423,7 +423,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_S_copy_chars(_CharT* __p, _Iterator __k1, _Iterator __k2)
 	{
 	  for (; __k1 != __k2; ++__k1, (void)++__p)
-	    traits_type::assign(*__p, *__k1); // These types are off.
+	    traits_type::assign(*__p, static_cast<_CharT>(*__k1));
 	}
 
       static void
@@ -656,12 +656,19 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 	      reserve(__n);
 	      pointer __p = _M_data();
-	      if constexpr (ranges::contiguous_range<_Rg>
-			      && is_same_v<ranges::range_value_t<_Rg>, _CharT>)
+	      if constexpr (requires {
+			      requires ranges::contiguous_range<_Rg>;
+			      { ranges::data(std::forward<_Rg>(__rg)) }
+				-> convertible_to<const _CharT*>;
+			    })
 		_M_copy(__p, ranges::data(std::forward<_Rg>(__rg)), __n);
 	      else
-		for (auto&& __e : __rg)
-		  traits_type::assign(*__p++, std::forward<decltype(__e)>(__e));
+		{
+		  auto __first = ranges::begin(__rg);
+		  const auto __last = ranges::end(__rg);
+		  for (; __first != __last; ++__first)
+		    traits_type::assign(*__p++, static_cast<_CharT>(*__first));
+		}
 	      _M_rep()->_M_set_length_and_sharable(__n);
 	    }
 	  else
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc
new file mode 100644
index 00000000000..301ca5ddd8e
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/119748.cc
@@ -0,0 +1,35 @@ 
+// { dg-do compile }
+
+// Bug 119748
+// string(InputIterator, InputIterator) rejects volatile charT* as iterator
+
+#ifndef TEST_CHAR_TYPE
+#define TEST_CHAR_TYPE char
+#endif
+
+#include <string>
+#include <testsuite_iterators.h>
+
+typedef TEST_CHAR_TYPE C;
+
+volatile C vs[42] = {};
+std::basic_string<C> s(vs+0, vs+42);
+#ifdef __cpp_lib_containers_ranges
+std::basic_string<C> s2(std::from_range, vs);
+#endif
+
+using namespace __gnu_test;
+
+test_container<volatile C, input_iterator_wrapper> input_cont(vs);
+std::basic_string<C> s3(input_cont.begin(), input_cont.end());
+
+test_container<volatile C, forward_iterator_wrapper> fwd_cont(vs);
+std::basic_string<C> s4(fwd_cont.begin(), fwd_cont.end());
+
+#ifdef __cpp_lib_containers_ranges
+test_input_range<volatile C> input_range(vs);
+std::basic_string<C> s5(std::from_range, input_range);
+
+test_forward_range<volatile C> fwd_range(vs);
+std::basic_string<C> s6(std::from_range, fwd_range);
+#endif
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc
new file mode 100644
index 00000000000..7d3ba10d2d4
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/119748.cc
@@ -0,0 +1,7 @@ 
+// { dg-do compile }
+
+// Bug 119748
+// string(InputIterator, InputIterator) rejects volatile charT* as iterator
+
+#define TEST_CHAR_TYPE wchar_t
+#include "../char/119748.cc"