diff mbox series

libstdc++: Adjust how __gnu_debug::vector detects invalidation

Message ID 20250325112447.185346-1-jwakely@redhat.com
State New
Headers show
Series libstdc++: Adjust how __gnu_debug::vector detects invalidation | expand

Commit Message

Jonathan Wakely March 25, 2025, 11:23 a.m. UTC 
The new C++23 member functions assign_range, insert_range and
append_range were checking whether the begin() iterator changed after
calling the base class member. That works, but is technically undefined
when the original iterator has been invalidated by a change in capacity.

We can just check the capacity directly, because reallocation only
occurs if a change in capacity is required.

N.B. we can't use data() either because std::vector<bool> doesn't have
it.

libstdc++-v3/ChangeLog:

	* include/debug/vector (vector::assign_range): Use change in
	capacity to detect reallocation.
	(vector::insert_range, vector::append_range): Likewise. Remove
	unused variables.
---

Tested x86_64-linux.

 libstdc++-v3/include/debug/vector | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Tomasz Kamiński March 25, 2025, 11:56 a.m. UTC | #1 
On Tue, Mar 25, 2025 at 12:26 PM Jonathan Wakely <jwakely@redhat.com> wrote:

> The new C++23 member functions assign_range, insert_range and
> append_range were checking whether the begin() iterator changed after
> calling the base class member. That works, but is technically undefined
> when the original iterator has been invalidated by a change in capacity.
>
> We can just check the capacity directly, because reallocation only
> occurs if a change in capacity is required.
>
> N.B. we can't use data() either because std::vector<bool> doesn't have
> it.
>
> libstdc++-v3/ChangeLog:
>
>         * include/debug/vector (vector::assign_range): Use change in
>         capacity to detect reallocation.
>         (vector::insert_range, vector::append_range): Likewise. Remove
>         unused variables.
> ---
>
> Tested x86_64-linux.
>
LGTM.

>
>  libstdc++-v3/include/debug/vector | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/libstdc++-v3/include/debug/vector
> b/libstdc++-v3/include/debug/vector
> index 022ebe8c664..b49766c18a7 100644
> --- a/libstdc++-v3/include/debug/vector
> +++ b/libstdc++-v3/include/debug/vector
> @@ -876,12 +876,12 @@ namespace __debug
>         constexpr void
>         assign_range(_Rg&& __rg)
>         {
> -         auto __old_begin = _Base::begin();
> +         auto __old_capacity = _Base::capacity();
>           auto __old_size = _Base::size();
>           _Base::assign_range(__rg);
>           if (!std::__is_constant_evaluated())
>             {
> -             if (_Base::begin() != __old_begin)
> +             if (_Base::capacity() != __old_capacity)
>                 this->_M_invalidate_all();
>               else if (_Base::size() < __old_size)
>                 this->_M_invalidate_after_nth(_Base::size());
> @@ -893,12 +893,11 @@ namespace __debug
>         constexpr iterator
>         insert_range(const_iterator __pos, _Rg&& __rg)
>         {
> -         auto __old_begin = _Base::begin();
> -         auto __old_size = _Base::size();
> +         auto __old_capacity = _Base::capacity();
>           auto __res = _Base::insert_range(__pos.base(), __rg);
>           if (!std::__is_constant_evaluated())
>             {
> -             if (_Base::begin() != __old_begin)
> +             if (_Base::capacity() != __old_capacity)
>                 this->_M_invalidate_all();
>               this->_M_update_guaranteed_capacity();
>             }
> @@ -909,12 +908,11 @@ namespace __debug
>         constexpr void
>         append_range(_Rg&& __rg)
>         {
> -         auto __old_begin = _Base::begin();
> -         auto __old_size = _Base::size();
> +         auto __old_capacity = _Base::capacity();
>           _Base::append_range(__rg);
>           if (!std::__is_constant_evaluated())
>             {
> -             if (_Base::begin() != __old_begin)
> +             if (_Base::capacity() != __old_capacity)
>                 this->_M_invalidate_all();
>               this->_M_update_guaranteed_capacity();
>             }
> --
> 2.49.0
>
>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector
index 022ebe8c664..b49766c18a7 100644
--- a/libstdc++-v3/include/debug/vector
+++ b/libstdc++-v3/include/debug/vector
@@ -876,12 +876,12 @@  namespace __debug
 	constexpr void
 	assign_range(_Rg&& __rg)
 	{
-	  auto __old_begin = _Base::begin();
+	  auto __old_capacity = _Base::capacity();
 	  auto __old_size = _Base::size();
 	  _Base::assign_range(__rg);
 	  if (!std::__is_constant_evaluated())
 	    {
-	      if (_Base::begin() != __old_begin)
+	      if (_Base::capacity() != __old_capacity)
 		this->_M_invalidate_all();
 	      else if (_Base::size() < __old_size)
 		this->_M_invalidate_after_nth(_Base::size());
@@ -893,12 +893,11 @@  namespace __debug
 	constexpr iterator
 	insert_range(const_iterator __pos, _Rg&& __rg)
 	{
-	  auto __old_begin = _Base::begin();
-	  auto __old_size = _Base::size();
+	  auto __old_capacity = _Base::capacity();
 	  auto __res = _Base::insert_range(__pos.base(), __rg);
 	  if (!std::__is_constant_evaluated())
 	    {
-	      if (_Base::begin() != __old_begin)
+	      if (_Base::capacity() != __old_capacity)
 		this->_M_invalidate_all();
 	      this->_M_update_guaranteed_capacity();
 	    }
@@ -909,12 +908,11 @@  namespace __debug
 	constexpr void
 	append_range(_Rg&& __rg)
 	{
-	  auto __old_begin = _Base::begin();
-	  auto __old_size = _Base::size();
+	  auto __old_capacity = _Base::capacity();
 	  _Base::append_range(__rg);
 	  if (!std::__is_constant_evaluated())
 	    {
-	      if (_Base::begin() != __old_begin)
+	      if (_Base::capacity() != __old_capacity)
 		this->_M_invalidate_all();
 	      this->_M_update_guaranteed_capacity();
 	    }