[PATCH][_GLIBCXX_DEBUG] Refine singular iterator state

[PATCH][_GLIBCXX_DEBUG] Refine singular iterator state

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

This an old patch I had prepared a long time ago, I don't think I ever 
submitted it.

     libstdc++: [_GLIBCXX_DEBUG] Do not consider detached iterators as 
value-initialized

     An attach iterator has its _M_version set to something != 0. This 
value shall be preserved
     when detaching it so that the iterator does not look like a value 
initialized one.

     libstdc++-v3/ChangeLog:

             * include/debug/formatter.h (__singular_value_init): New 
_Iterator_state enum entry.
             (_Parameter<>(const _Safe_iterator<>&, const char*, 
_Is_iterator)): Check if iterator
             parameter is value-initialized.
             (_Parameter<>(const _Safe_local_iterator<>&, const char*, 
_Is_iterator)): Likewise.
             * include/debug/safe_iterator.h 
(_Safe_iterator<>::_M_value_initialized()): New. Adapt
             checks.
             * include/debug/safe_local_iterator.h 
(_Safe_local_iterator<>::_M_value_initialized()): New.
             Adapt checks.
             * src/c++11/debug.cc (_Safe_iterator_base::_M_reset): Do 
not reset _M_version.
             (print_field(PrintContext&, const _Parameter&, const 
char*)): Adapt state_names.
             * testsuite/23_containers/deque/debug/iterator1_neg.cc: New 
test.
             * testsuite/23_containers/deque/debug/iterator2_neg.cc: New 
test.
             * 
testsuite/23_containers/forward_list/debug/iterator1_neg.cc: New test.
             * 
testsuite/23_containers/forward_list/debug/iterator2_neg.cc: New test.

Tested under Linux x86_64 _GLIBCXX_DEBUG mode.

Ok to commit ?

François

[-- Attachment #2: singular_value_init.patch --]
[-- Type: text/x-patch, Size: 13821 bytes --]

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index 80e8ba46d1e..748d4fbfea4 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -185,6 +185,7 @@ namespace __gnu_debug
       __rbegin,		// dereferenceable, and at the reverse-beginning
       __rmiddle,	// reverse-dereferenceable, not at the reverse-beginning
       __rend,		// reverse-past-the-end
+      __singular_value_init,	// singular, value initialized
       __last_state
     };
 
@@ -280,7 +281,12 @@ namespace __gnu_debug
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
 	  if (__it._M_singular())
-	    _M_variant._M_iterator._M_state = __singular;
+	    {
+	      if (__it._M_value_initialized())
+		_M_variant._M_iterator._M_state = __singular_value_init;
+	      else
+		_M_variant._M_iterator._M_state = __singular;
+	    }
 	  else
 	    {
 	      if (__it._M_is_before_begin())
@@ -308,7 +314,12 @@ namespace __gnu_debug
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
 	  if (__it._M_singular())
-	    _M_variant._M_iterator._M_state = __singular;
+	    {
+	      if (__it._M_value_initialized())
+		_M_variant._M_iterator._M_state = __singular_value_init;
+	      else
+		_M_variant._M_iterator._M_state = __singular;
+	    }
 	  else
 	    {
 	      if (__it._M_is_end())
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index d613933e236..33f7a86478a 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -41,8 +41,8 @@
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs, _BadMsgId, _DiffMsgId) \
   _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
-			|| (_Lhs.base() == _Iterator()			\
-			    && _Rhs.base() == _Iterator()),		\
+			|| (_Lhs._M_value_initialized()			\
+			    && _Rhs._M_value_initialized()),		\
 			_M_message(_BadMsgId)				\
 			._M_iterator(_Lhs, #_Lhs)			\
 			._M_iterator(_Rhs, #_Rhs));			\
@@ -177,7 +177,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -193,7 +193,7 @@ namespace __gnu_debug
       : _Iter_base()
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -220,7 +220,7 @@ namespace __gnu_debug
 	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
 	  // DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	  _GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-				|| __x.base() == _MutableIterator(),
+				|| __x._M_value_initialized(),
 				_M_message(__msg_init_const_singular)
 				._M_iterator(*this, "this")
 				._M_iterator(__x, "other"));
@@ -236,7 +236,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -266,7 +266,7 @@ namespace __gnu_debug
       operator=(_Safe_iterator&& __x) noexcept
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -405,6 +405,11 @@ namespace __gnu_debug
       _M_incrementable() const
       { return !this->_M_singular() && !_M_is_end(); }
 
+      /// Is the iterator value-initialized?
+      bool
+      _M_value_initialized() const
+      { return _M_version == 0 && base() == _Iter_base(); }
+
       // Can we advance the iterator @p __n steps (@p __n may be negative)
       bool
       _M_can_advance(difference_type __n, bool __strict = false) const;
diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h
index b5318f50b6a..6e3c4eb1505 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.h
+++ b/libstdc++-v3/include/debug/safe_local_iterator.h
@@ -33,8 +33,8 @@
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs) \
   _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
-			|| (_Lhs.base() == _Iterator{}			\
-			    && _Rhs.base() == _Iterator{}),		\
+			|| (_Lhs._M_value_initialized()			\
+			    && _Rhs._M_value_initialized()),		\
 			_M_message(__msg_iter_compare_bad)		\
 			._M_iterator(_Lhs, "lhs")			\
 			._M_iterator(_Rhs, "rhs"));			\
@@ -127,7 +127,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -142,7 +142,7 @@ namespace __gnu_debug
       : _Iter_base()
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -167,7 +167,7 @@ namespace __gnu_debug
 	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
 	  // DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	  _GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-				|| __x.base() == _MutableIterator(),
+				|| __x._M_value_initialized(),
 				_M_message(__msg_init_const_singular)
 				._M_iterator(*this, "this")
 				._M_iterator(__x, "other"));
@@ -183,7 +183,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -212,7 +212,7 @@ namespace __gnu_debug
       operator=(_Safe_local_iterator&& __x) noexcept
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -343,6 +343,11 @@ namespace __gnu_debug
       _M_incrementable() const
       { return !this->_M_singular() && !_M_is_end(); }
 
+      /// Is the iterator value-initialized?
+      bool
+      _M_value_initialized() const
+      { return _M_version == 0 && base() == _Iter_base{}; }
+
       // Is the iterator range [*this, __rhs) valid?
       bool
       _M_valid_range(const _Safe_local_iterator& __rhs,
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 4706defedf1..cf8e6f48081 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -426,7 +426,8 @@ namespace __gnu_debug
   _M_reset() throw ()
   {
     __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELEASE);
-    _M_version = 0;
+    // Detach iterator shall not look like a value-initialized one.
+    // _M_version = 0;
     _M_prior = 0;
     _M_next = 0;
   }
@@ -767,7 +768,8 @@ namespace
 	 "before-begin",
 	 "dereferenceable (start-of-reverse-sequence)",
 	 "dereferenceable (reverse)",
-	 "past-the-reverse-end"
+	 "past-the-reverse-end",
+	 "singular (value-initialized)"
 	};
       print_word(ctx, state_names[iterator._M_state]);
     }
diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc
new file mode 100644
index 00000000000..097a035c461
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <deque>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::deque<int>::iterator It;
+  std::deque<int> dq;
+  dq.push_back(1);
+
+  It it = It();
+  VERIFY( dq.begin() != it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc
new file mode 100644
index 00000000000..097b5ba15cb
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc
@@ -0,0 +1,42 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <deque>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::deque<int>::iterator It;
+  It it;
+  {
+    std::deque<int> dq;
+    it = dq.begin();
+  }
+
+  It value_init_it = It();
+  VERIFY( it != value_init_it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc
new file mode 100644
index 00000000000..5472b0ed231
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  std::forward_list<int> fl;
+  fl.push_front(1);
+
+  It it = It();
+  VERIFY( fl.begin() != it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc
new file mode 100644
index 00000000000..74278eb8cc8
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc
@@ -0,0 +1,42 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { target c++11 xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  It it;
+  {
+    std::forward_list<int> fl;
+    it = fl.begin();
+  }
+
+  It value_init_it{};
+  VERIFY( it != value_init_it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

Re: [PATCH][_GLIBCXX_DEBUG] Refine singular iterator state

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]

Another version of this patch with just a new test case showing what 
wrong code was unnoticed previously by the _GLIBCXX_DEBUG mode.

On 04/08/22 22:56, François Dumont wrote:
> This an old patch I had prepared a long time ago, I don't think I ever 
> submitted it.
>
>     libstdc++: [_GLIBCXX_DEBUG] Do not consider detached iterators as 
> value-initialized
>
>     An attach iterator has its _M_version set to something != 0. This 
> value shall be preserved
>     when detaching it so that the iterator does not look like a value 
> initialized one.
>
>     libstdc++-v3/ChangeLog:
>
>             * include/debug/formatter.h (__singular_value_init): New 
> _Iterator_state enum entry.
>             (_Parameter<>(const _Safe_iterator<>&, const char*, 
> _Is_iterator)): Check if iterator
>             parameter is value-initialized.
>             (_Parameter<>(const _Safe_local_iterator<>&, const char*, 
> _Is_iterator)): Likewise.
>             * include/debug/safe_iterator.h 
> (_Safe_iterator<>::_M_value_initialized()): New. Adapt
>             checks.
>             * include/debug/safe_local_iterator.h 
> (_Safe_local_iterator<>::_M_value_initialized()): New.
>             Adapt checks.
>             * src/c++11/debug.cc (_Safe_iterator_base::_M_reset): Do 
> not reset _M_version.
>             (print_field(PrintContext&, const _Parameter&, const 
> char*)): Adapt state_names.
>             * testsuite/23_containers/deque/debug/iterator1_neg.cc: 
> New test.
>             * testsuite/23_containers/deque/debug/iterator2_neg.cc: 
> New test.
>             * 
> testsuite/23_containers/forward_list/debug/iterator1_neg.cc: New test.
>             * 
> testsuite/23_containers/forward_list/debug/iterator2_neg.cc: New test.
>
> Tested under Linux x86_64 _GLIBCXX_DEBUG mode.
>
> Ok to commit ?
>
> François


[-- Attachment #2: singular_value_init.patch --]
[-- Type: text/x-patch, Size: 15352 bytes --]

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index 80e8ba46d1e..748d4fbfea4 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -185,6 +185,7 @@ namespace __gnu_debug
       __rbegin,		// dereferenceable, and at the reverse-beginning
       __rmiddle,	// reverse-dereferenceable, not at the reverse-beginning
       __rend,		// reverse-past-the-end
+      __singular_value_init,	// singular, value initialized
       __last_state
     };
 
@@ -280,7 +281,12 @@ namespace __gnu_debug
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
 	  if (__it._M_singular())
-	    _M_variant._M_iterator._M_state = __singular;
+	    {
+	      if (__it._M_value_initialized())
+		_M_variant._M_iterator._M_state = __singular_value_init;
+	      else
+		_M_variant._M_iterator._M_state = __singular;
+	    }
 	  else
 	    {
 	      if (__it._M_is_before_begin())
@@ -308,7 +314,12 @@ namespace __gnu_debug
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
 	  if (__it._M_singular())
-	    _M_variant._M_iterator._M_state = __singular;
+	    {
+	      if (__it._M_value_initialized())
+		_M_variant._M_iterator._M_state = __singular_value_init;
+	      else
+		_M_variant._M_iterator._M_state = __singular;
+	    }
 	  else
 	    {
 	      if (__it._M_is_end())
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index d613933e236..33f7a86478a 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -41,8 +41,8 @@
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs, _BadMsgId, _DiffMsgId) \
   _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
-			|| (_Lhs.base() == _Iterator()			\
-			    && _Rhs.base() == _Iterator()),		\
+			|| (_Lhs._M_value_initialized()			\
+			    && _Rhs._M_value_initialized()),		\
 			_M_message(_BadMsgId)				\
 			._M_iterator(_Lhs, #_Lhs)			\
 			._M_iterator(_Rhs, #_Rhs));			\
@@ -177,7 +177,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -193,7 +193,7 @@ namespace __gnu_debug
       : _Iter_base()
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -220,7 +220,7 @@ namespace __gnu_debug
 	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
 	  // DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	  _GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-				|| __x.base() == _MutableIterator(),
+				|| __x._M_value_initialized(),
 				_M_message(__msg_init_const_singular)
 				._M_iterator(*this, "this")
 				._M_iterator(__x, "other"));
@@ -236,7 +236,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -266,7 +266,7 @@ namespace __gnu_debug
       operator=(_Safe_iterator&& __x) noexcept
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -405,6 +405,11 @@ namespace __gnu_debug
       _M_incrementable() const
       { return !this->_M_singular() && !_M_is_end(); }
 
+      /// Is the iterator value-initialized?
+      bool
+      _M_value_initialized() const
+      { return _M_version == 0 && base() == _Iter_base(); }
+
       // Can we advance the iterator @p __n steps (@p __n may be negative)
       bool
       _M_can_advance(difference_type __n, bool __strict = false) const;
diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h
index b5318f50b6a..6e3c4eb1505 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.h
+++ b/libstdc++-v3/include/debug/safe_local_iterator.h
@@ -33,8 +33,8 @@
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs) \
   _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
-			|| (_Lhs.base() == _Iterator{}			\
-			    && _Rhs.base() == _Iterator{}),		\
+			|| (_Lhs._M_value_initialized()			\
+			    && _Rhs._M_value_initialized()),		\
 			_M_message(__msg_iter_compare_bad)		\
 			._M_iterator(_Lhs, "lhs")			\
 			._M_iterator(_Rhs, "rhs"));			\
@@ -127,7 +127,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -142,7 +142,7 @@ namespace __gnu_debug
       : _Iter_base()
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -167,7 +167,7 @@ namespace __gnu_debug
 	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
 	  // DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	  _GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-				|| __x.base() == _MutableIterator(),
+				|| __x._M_value_initialized(),
 				_M_message(__msg_init_const_singular)
 				._M_iterator(*this, "this")
 				._M_iterator(__x, "other"));
@@ -183,7 +183,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -212,7 +212,7 @@ namespace __gnu_debug
       operator=(_Safe_local_iterator&& __x) noexcept
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -343,6 +343,11 @@ namespace __gnu_debug
       _M_incrementable() const
       { return !this->_M_singular() && !_M_is_end(); }
 
+      /// Is the iterator value-initialized?
+      bool
+      _M_value_initialized() const
+      { return _M_version == 0 && base() == _Iter_base{}; }
+
       // Is the iterator range [*this, __rhs) valid?
       bool
       _M_valid_range(const _Safe_local_iterator& __rhs,
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 4706defedf1..cf8e6f48081 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -426,7 +426,8 @@ namespace __gnu_debug
   _M_reset() throw ()
   {
     __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELEASE);
-    _M_version = 0;
+    // Detach iterator shall not look like a value-initialized one.
+    // _M_version = 0;
     _M_prior = 0;
     _M_next = 0;
   }
@@ -767,7 +768,8 @@ namespace
 	 "before-begin",
 	 "dereferenceable (start-of-reverse-sequence)",
 	 "dereferenceable (reverse)",
-	 "past-the-reverse-end"
+	 "past-the-reverse-end",
+	 "singular (value-initialized)"
 	};
       print_word(ctx, state_names[iterator._M_state]);
     }
diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc
new file mode 100644
index 00000000000..097a035c461
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <deque>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::deque<int>::iterator It;
+  std::deque<int> dq;
+  dq.push_back(1);
+
+  It it = It();
+  VERIFY( dq.begin() != it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc
new file mode 100644
index 00000000000..097b5ba15cb
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc
@@ -0,0 +1,42 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <deque>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::deque<int>::iterator It;
+  It it;
+  {
+    std::deque<int> dq;
+    it = dq.begin();
+  }
+
+  It value_init_it = It();
+  VERIFY( it != value_init_it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc
new file mode 100644
index 00000000000..5472b0ed231
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  std::forward_list<int> fl;
+  fl.push_front(1);
+
+  It it = It();
+  VERIFY( fl.begin() != it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc
new file mode 100644
index 00000000000..74278eb8cc8
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc
@@ -0,0 +1,42 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { target c++11 xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  It it;
+  {
+    std::forward_list<int> fl;
+    it = fl.begin();
+  }
+
+  It value_init_it{};
+  VERIFY( it != value_init_it );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator3_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator3_neg.cc
new file mode 100644
index 00000000000..205c57825b6
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator3_neg.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  It end1, end2;
+
+  {
+    std::forward_list<int> fl;
+    fl.push_front(1);
+
+    end1 = end2 = fl.end();
+    VERIFY( end1 == end2 );
+  }
+
+  VERIFY( end1 == end2 );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

Re: [PATCH][_GLIBCXX_DEBUG] Refine singular iterator state

[Jonathan Wakely]

On Mon, 8 Aug 2022 at 06:07, François Dumont via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Another version of this patch with just a new test case showing what
> wrong code was unnoticed previously by the _GLIBCXX_DEBUG mode.
>
> On 04/08/22 22:56, François Dumont wrote:
> > This an old patch I had prepared a long time ago, I don't think I ever
> > submitted it.
> >
> >     libstdc++: [_GLIBCXX_DEBUG] Do not consider detached iterators as
> > value-initialized
> >
> >     An attach iterator has its _M_version set to something != 0. This
> > value shall be preserved
> >     when detaching it so that the iterator does not look like a value
> > initialized one.
> >
> >     libstdc++-v3/ChangeLog:
> >
> >             * include/debug/formatter.h (__singular_value_init): New
> > _Iterator_state enum entry.
> >             (_Parameter<>(const _Safe_iterator<>&, const char*,
> > _Is_iterator)): Check if iterator
> >             parameter is value-initialized.
> >             (_Parameter<>(const _Safe_local_iterator<>&, const char*,
> > _Is_iterator)): Likewise.
> >             * include/debug/safe_iterator.h
> > (_Safe_iterator<>::_M_value_initialized()): New. Adapt
> >             checks.
> >             * include/debug/safe_local_iterator.h
> > (_Safe_local_iterator<>::_M_value_initialized()): New.
> >             Adapt checks.
> >             * src/c++11/debug.cc (_Safe_iterator_base::_M_reset): Do
> > not reset _M_version.
> >             (print_field(PrintContext&, const _Parameter&, const
> > char*)): Adapt state_names.
> >             * testsuite/23_containers/deque/debug/iterator1_neg.cc:
> > New test.
> >             * testsuite/23_containers/deque/debug/iterator2_neg.cc:
> > New test.
> >             *
> > testsuite/23_containers/forward_list/debug/iterator1_neg.cc: New test.
> >             *
> > testsuite/23_containers/forward_list/debug/iterator2_neg.cc: New test.
> >
> > Tested under Linux x86_64 _GLIBCXX_DEBUG mode.
> >
> > Ok to commit ?
> >
> > François


>diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
>index 4706defedf1..cf8e6f48081 100644
>--- a/libstdc++-v3/src/c++11/debug.cc
>+++ b/libstdc++-v3/src/c++11/debug.cc
>@@ -426,7 +426,8 @@ namespace __gnu_debug
>   _M_reset() throw ()
>   {
>     __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELEASE);
>-    _M_version = 0;
>+    // Detach iterator shall not look like a value-initialized one.
>+    // _M_version = 0;
>     _M_prior = 0;
>     _M_next = 0;
>   }

I think this would be clearer as "Do not reset version, so that a
detached iterator does not look like a value-initialized one."

>+// { dg-do run { xfail *-*-* } }
>+// { dg-require-debug-mode "" }
>+
>+#include <deque>
>+
>+#include <testsuite_hooks.h>
>+
>+void test01()
>+{
>+  typedef typename std::deque<int>::iterator It;
>+  std::deque<int> dq;
>+  dq.push_back(1);
>+
>+  It it = It();
>+  VERIFY( dq.begin() != it );

Is there any reason to use VERIFY here?

We're expecting the comparison to abort in the debug mode checks,
right? Which would happen if we just do:

(void) dq.begin() == it;

Using VERIFY just makes it look like we're expecting the test to be
XFAIL because the assertion will fail, but that's not what is being
tested.

OK for trunk with those changes, thanks.

Re: [PATCH][_GLIBCXX_DEBUG] Refine singular iterator state

[François Dumont]

[-- Attachment #1: Type: text/plain, Size: 3608 bytes --]

On 08/08/22 15:19, Jonathan Wakely wrote:
> On Mon, 8 Aug 2022 at 06:07, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> Another version of this patch with just a new test case showing what
>> wrong code was unnoticed previously by the _GLIBCXX_DEBUG mode.
>>
>> On 04/08/22 22:56, François Dumont wrote:
>>> This an old patch I had prepared a long time ago, I don't think I ever
>>> submitted it.
>>>
>>>      libstdc++: [_GLIBCXX_DEBUG] Do not consider detached iterators as
>>> value-initialized
>>>
>>>      An attach iterator has its _M_version set to something != 0. This
>>> value shall be preserved
>>>      when detaching it so that the iterator does not look like a value
>>> initialized one.
>>>
>>>      libstdc++-v3/ChangeLog:
>>>
>>>              * include/debug/formatter.h (__singular_value_init): New
>>> _Iterator_state enum entry.
>>>              (_Parameter<>(const _Safe_iterator<>&, const char*,
>>> _Is_iterator)): Check if iterator
>>>              parameter is value-initialized.
>>>              (_Parameter<>(const _Safe_local_iterator<>&, const char*,
>>> _Is_iterator)): Likewise.
>>>              * include/debug/safe_iterator.h
>>> (_Safe_iterator<>::_M_value_initialized()): New. Adapt
>>>              checks.
>>>              * include/debug/safe_local_iterator.h
>>> (_Safe_local_iterator<>::_M_value_initialized()): New.
>>>              Adapt checks.
>>>              * src/c++11/debug.cc (_Safe_iterator_base::_M_reset): Do
>>> not reset _M_version.
>>>              (print_field(PrintContext&, const _Parameter&, const
>>> char*)): Adapt state_names.
>>>              * testsuite/23_containers/deque/debug/iterator1_neg.cc:
>>> New test.
>>>              * testsuite/23_containers/deque/debug/iterator2_neg.cc:
>>> New test.
>>>              *
>>> testsuite/23_containers/forward_list/debug/iterator1_neg.cc: New test.
>>>              *
>>> testsuite/23_containers/forward_list/debug/iterator2_neg.cc: New test.
>>>
>>> Tested under Linux x86_64 _GLIBCXX_DEBUG mode.
>>>
>>> Ok to commit ?
>>>
>>> François
>
>> diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
>> index 4706defedf1..cf8e6f48081 100644
>> --- a/libstdc++-v3/src/c++11/debug.cc
>> +++ b/libstdc++-v3/src/c++11/debug.cc
>> @@ -426,7 +426,8 @@ namespace __gnu_debug
>>    _M_reset() throw ()
>>    {
>>      __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELEASE);
>> -    _M_version = 0;
>> +    // Detach iterator shall not look like a value-initialized one.
>> +    // _M_version = 0;
>>      _M_prior = 0;
>>      _M_next = 0;
>>    }
> I think this would be clearer as "Do not reset version, so that a
> detached iterator does not look like a value-initialized one."
>
>> +// { dg-do run { xfail *-*-* } }
>> +// { dg-require-debug-mode "" }
>> +
>> +#include <deque>
>> +
>> +#include <testsuite_hooks.h>
>> +
>> +void test01()
>> +{
>> +  typedef typename std::deque<int>::iterator It;
>> +  std::deque<int> dq;
>> +  dq.push_back(1);
>> +
>> +  It it = It();
>> +  VERIFY( dq.begin() != it );
> Is there any reason to use VERIFY here?
Only make sure the compiler do not optimize this check away.
>
> We're expecting the comparison to abort in the debug mode checks,
> right? Which would happen if we just do:
>
> (void) dq.begin() == it;

I guess this (void) cast is doing the same so adopted.

> Using VERIFY just makes it look like we're expecting the test to be
> XFAIL because the assertion will fail, but that's not what is being
> tested.
>
> OK for trunk with those changes, thanks.
>
Updated committed patch attached.


[-- Attachment #2: singular_value_init.patch --]
[-- Type: text/x-patch, Size: 15255 bytes --]

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index 80e8ba46d1e..748d4fbfea4 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -185,6 +185,7 @@ namespace __gnu_debug
       __rbegin,		// dereferenceable, and at the reverse-beginning
       __rmiddle,	// reverse-dereferenceable, not at the reverse-beginning
       __rend,		// reverse-past-the-end
+      __singular_value_init,	// singular, value initialized
       __last_state
     };
 
@@ -280,7 +281,12 @@ namespace __gnu_debug
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
 	  if (__it._M_singular())
-	    _M_variant._M_iterator._M_state = __singular;
+	    {
+	      if (__it._M_value_initialized())
+		_M_variant._M_iterator._M_state = __singular_value_init;
+	      else
+		_M_variant._M_iterator._M_state = __singular;
+	    }
 	  else
 	    {
 	      if (__it._M_is_before_begin())
@@ -308,7 +314,12 @@ namespace __gnu_debug
 	  _M_variant._M_iterator._M_seq_type = _GLIBCXX_TYPEID(_Sequence);
 
 	  if (__it._M_singular())
-	    _M_variant._M_iterator._M_state = __singular;
+	    {
+	      if (__it._M_value_initialized())
+		_M_variant._M_iterator._M_state = __singular_value_init;
+	      else
+		_M_variant._M_iterator._M_state = __singular;
+	    }
 	  else
 	    {
 	      if (__it._M_is_end())
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index d613933e236..33f7a86478a 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -41,8 +41,8 @@
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs, _BadMsgId, _DiffMsgId) \
   _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
-			|| (_Lhs.base() == _Iterator()			\
-			    && _Rhs.base() == _Iterator()),		\
+			|| (_Lhs._M_value_initialized()			\
+			    && _Rhs._M_value_initialized()),		\
 			_M_message(_BadMsgId)				\
 			._M_iterator(_Lhs, #_Lhs)			\
 			._M_iterator(_Rhs, #_Rhs));			\
@@ -177,7 +177,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -193,7 +193,7 @@ namespace __gnu_debug
       : _Iter_base()
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -220,7 +220,7 @@ namespace __gnu_debug
 	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
 	  // DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	  _GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-				|| __x.base() == _MutableIterator(),
+				|| __x._M_value_initialized(),
 				_M_message(__msg_init_const_singular)
 				._M_iterator(*this, "this")
 				._M_iterator(__x, "other"));
@@ -236,7 +236,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -266,7 +266,7 @@ namespace __gnu_debug
       operator=(_Safe_iterator&& __x) noexcept
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -405,6 +405,11 @@ namespace __gnu_debug
       _M_incrementable() const
       { return !this->_M_singular() && !_M_is_end(); }
 
+      /// Is the iterator value-initialized?
+      bool
+      _M_value_initialized() const
+      { return _M_version == 0 && base() == _Iter_base(); }
+
       // Can we advance the iterator @p __n steps (@p __n may be negative)
       bool
       _M_can_advance(difference_type __n, bool __strict = false) const;
diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h
index b5318f50b6a..6e3c4eb1505 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.h
+++ b/libstdc++-v3/include/debug/safe_local_iterator.h
@@ -33,8 +33,8 @@
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs) \
   _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
-			|| (_Lhs.base() == _Iterator{}			\
-			    && _Rhs.base() == _Iterator{}),		\
+			|| (_Lhs._M_value_initialized()			\
+			    && _Rhs._M_value_initialized()),		\
 			_M_message(__msg_iter_compare_bad)		\
 			._M_iterator(_Lhs, "lhs")			\
 			._M_iterator(_Rhs, "rhs"));			\
@@ -127,7 +127,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -142,7 +142,7 @@ namespace __gnu_debug
       : _Iter_base()
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_init_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -167,7 +167,7 @@ namespace __gnu_debug
 	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
 	  // DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	  _GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-				|| __x.base() == _MutableIterator(),
+				|| __x._M_value_initialized(),
 				_M_message(__msg_init_const_singular)
 				._M_iterator(*this, "this")
 				._M_iterator(__x, "other"));
@@ -183,7 +183,7 @@ namespace __gnu_debug
 	// _GLIBCXX_RESOLVE_LIB_DEFECTS
 	// DR 408. Is vector<reverse_iterator<char*> > forbidden?
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -212,7 +212,7 @@ namespace __gnu_debug
       operator=(_Safe_local_iterator&& __x) noexcept
       {
 	_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
-			      || __x.base() == _Iterator(),
+			      || __x._M_value_initialized(),
 			      _M_message(__msg_copy_singular)
 			      ._M_iterator(*this, "this")
 			      ._M_iterator(__x, "other"));
@@ -343,6 +343,11 @@ namespace __gnu_debug
       _M_incrementable() const
       { return !this->_M_singular() && !_M_is_end(); }
 
+      /// Is the iterator value-initialized?
+      bool
+      _M_value_initialized() const
+      { return _M_version == 0 && base() == _Iter_base{}; }
+
       // Is the iterator range [*this, __rhs) valid?
       bool
       _M_valid_range(const _Safe_local_iterator& __rhs,
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 4706defedf1..8ed61a69913 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -426,7 +426,9 @@ namespace __gnu_debug
   _M_reset() throw ()
   {
     __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0, __ATOMIC_RELEASE);
-    _M_version = 0;
+    // Do not reset version, so that a detached iterator does not look like a
+    // value-initialized one.
+    // _M_version = 0;
     _M_prior = 0;
     _M_next = 0;
   }
@@ -767,7 +769,8 @@ namespace
 	 "before-begin",
 	 "dereferenceable (start-of-reverse-sequence)",
 	 "dereferenceable (reverse)",
-	 "past-the-reverse-end"
+	 "past-the-reverse-end",
+	 "singular (value-initialized)"
 	};
       print_word(ctx, state_names[iterator._M_state]);
     }
diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc
new file mode 100644
index 00000000000..73f8a044d43
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator1_neg.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <deque>
+
+void test01()
+{
+  typedef typename std::deque<int>::iterator It;
+  std::deque<int> dq;
+  dq.push_back(1);
+
+  It it = It();
+  (void)(dq.begin() != it);
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc
new file mode 100644
index 00000000000..0abf5cbd4ec
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/debug/iterator2_neg.cc
@@ -0,0 +1,40 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <deque>
+
+void test01()
+{
+  typedef typename std::deque<int>::iterator It;
+  It it;
+  {
+    std::deque<int> dq;
+    it = dq.begin();
+  }
+
+  It value_init_it = It();
+  (void)(it != value_init_it);
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc
new file mode 100644
index 00000000000..8ca44e248ed
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator1_neg.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  std::forward_list<int> fl;
+  fl.push_front(1);
+
+  It it = It();
+  (void)(fl.begin() != it);
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc
new file mode 100644
index 00000000000..92ab059e6b8
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator2_neg.cc
@@ -0,0 +1,40 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { target c++11 xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  It it;
+  {
+    std::forward_list<int> fl;
+    it = fl.begin();
+  }
+
+  It value_init_it{};
+  (void)(it != value_init_it);
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator3_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator3_neg.cc
new file mode 100644
index 00000000000..32ae7a5b7a6
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/debug/iterator3_neg.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2022 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-do run { xfail *-*-* } }
+// { dg-require-debug-mode "" }
+
+#include <forward_list>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  typedef typename std::forward_list<int>::iterator It;
+  It end1, end2;
+
+  {
+    std::forward_list<int> fl;
+    fl.push_front(1);
+
+    end1 = end2 = fl.end();
+    VERIFY( end1 == end2 );
+  }
+
+  (void)(end1 == end2);
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

Re: [PATCH][_GLIBCXX_DEBUG] Refine singular iterator state

[Jonathan Wakely]

On Mon, 8 Aug 2022, 19:15 François Dumont via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

> On 08/08/22 15:19, Jonathan Wakely wrote:
> > On Mon, 8 Aug 2022 at 06:07, François Dumont via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> >> Another version of this patch with just a new test case showing what
> >> wrong code was unnoticed previously by the _GLIBCXX_DEBUG mode.
> >>
> >> On 04/08/22 22:56, François Dumont wrote:
> >>> This an old patch I had prepared a long time ago, I don't think I ever
> >>> submitted it.
> >>>
> >>>      libstdc++: [_GLIBCXX_DEBUG] Do not consider detached iterators as
> >>> value-initialized
> >>>
> >>>      An attach iterator has its _M_version set to something != 0. This
> >>> value shall be preserved
> >>>      when detaching it so that the iterator does not look like a value
> >>> initialized one.
> >>>
> >>>      libstdc++-v3/ChangeLog:
> >>>
> >>>              * include/debug/formatter.h (__singular_value_init): New
> >>> _Iterator_state enum entry.
> >>>              (_Parameter<>(const _Safe_iterator<>&, const char*,
> >>> _Is_iterator)): Check if iterator
> >>>              parameter is value-initialized.
> >>>              (_Parameter<>(const _Safe_local_iterator<>&, const char*,
> >>> _Is_iterator)): Likewise.
> >>>              * include/debug/safe_iterator.h
> >>> (_Safe_iterator<>::_M_value_initialized()): New. Adapt
> >>>              checks.
> >>>              * include/debug/safe_local_iterator.h
> >>> (_Safe_local_iterator<>::_M_value_initialized()): New.
> >>>              Adapt checks.
> >>>              * src/c++11/debug.cc (_Safe_iterator_base::_M_reset): Do
> >>> not reset _M_version.
> >>>              (print_field(PrintContext&, const _Parameter&, const
> >>> char*)): Adapt state_names.
> >>>              * testsuite/23_containers/deque/debug/iterator1_neg.cc:
> >>> New test.
> >>>              * testsuite/23_containers/deque/debug/iterator2_neg.cc:
> >>> New test.
> >>>              *
> >>> testsuite/23_containers/forward_list/debug/iterator1_neg.cc: New test.
> >>>              *
> >>> testsuite/23_containers/forward_list/debug/iterator2_neg.cc: New test.
> >>>
> >>> Tested under Linux x86_64 _GLIBCXX_DEBUG mode.
> >>>
> >>> Ok to commit ?
> >>>
> >>> François
> >
> >> diff --git a/libstdc++-v3/src/c++11/debug.cc
> b/libstdc++-v3/src/c++11/debug.cc
> >> index 4706defedf1..cf8e6f48081 100644
> >> --- a/libstdc++-v3/src/c++11/debug.cc
> >> +++ b/libstdc++-v3/src/c++11/debug.cc
> >> @@ -426,7 +426,8 @@ namespace __gnu_debug
> >>    _M_reset() throw ()
> >>    {
> >>      __atomic_store_n(&_M_sequence, (_Safe_sequence_base*)0,
> __ATOMIC_RELEASE);
> >> -    _M_version = 0;
> >> +    // Detach iterator shall not look like a value-initialized one.
> >> +    // _M_version = 0;
> >>      _M_prior = 0;
> >>      _M_next = 0;
> >>    }
> > I think this would be clearer as "Do not reset version, so that a
> > detached iterator does not look like a value-initialized one."
> >
> >> +// { dg-do run { xfail *-*-* } }
> >> +// { dg-require-debug-mode "" }
> >> +
> >> +#include <deque>
> >> +
> >> +#include <testsuite_hooks.h>
> >> +
> >> +void test01()
> >> +{
> >> +  typedef typename std::deque<int>::iterator It;
> >> +  std::deque<int> dq;
> >> +  dq.push_back(1);
> >> +
> >> +  It it = It();
> >> +  VERIFY( dq.begin() != it );
> > Is there any reason to use VERIFY here?
> Only make sure the compiler do not optimize this check away.
>

It can't do that. If the debug check results in output to the terminal and
aborting the program then that's an observable side effect that cannot be
optimised away.

If the compiler was somehow smart enough to know that the comparison is
undefined then we wouldn't need debug mode.


>
> > We're expecting the comparison to abort in the debug mode checks,
> > right? Which would happen if we just do:
> >
> > (void) dq.begin() == it;
>
> I guess this (void) cast is doing the same so adopted.
>

No, the cast silences a warning.



> > Using VERIFY just makes it look like we're expecting the test to be
> > XFAIL because the assertion will fail, but that's not what is being
> > tested.
> >
> > OK for trunk with those changes, thanks.
> >
> Updated committed patch attached.
>

Thanks.


>