Fix some _GLIBCXX_DEBUG pretty printer errors

Message ID 62558510-8551-9275-d4dd-94acbd8914e0@gmail.com
State New
Headers show
Series
  • Fix some _GLIBCXX_DEBUG pretty printer errors
Related show

Commit Message

François Dumont Feb. 5, 2018, 6:49 a.m.
Hi

     Here is a patch to fix some pretty printer check errors when 
running those tests with _GLIBCXX_DEBUG.

     I introduced a special rendered for the std::forward_list iterator 
which is similar to the one used for the std::list and so used 
inheritance, I hope it is not a problem for Python 2/3 compatibility.

     I prefer to just rely on normal iterator rendered when we detect 
that iterator hasn't been initialized so that normal or debug modes give 
the same result.

     Ok to commit ?

François

Comments

Jonathan Wakely March 6, 2018, 9:21 p.m. | #1
On 5 February 2018 at 06:49, François Dumont wrote:
> Hi

>

>     Here is a patch to fix some pretty printer check errors when running

> those tests with _GLIBCXX_DEBUG.

>

>     I introduced a special rendered for the std::forward_list iterator which

> is similar to the one used for the std::list and so used inheritance, I hope

> it is not a problem for Python 2/3 compatibility.

>

>     I prefer to just rely on normal iterator rendered when we detect that

> iterator hasn't been initialized so that normal or debug modes give the same

> result.

>

>     Ok to commit ?

>

> François

>

>


>@@ -575,10 +586,12 @@ class StdDebugIteratorPrinter:

>     # and return the wrapped iterator value.

>     def to_string (self):

>         base_type = gdb.lookup_type('__gnu_debug::_Safe_iterator_base')

>+        itype = self.val.type.template_argument(0)

>         safe_seq = self.val.cast(base_type)['_M_sequence']

>-        if not safe_seq or self.val['_M_version'] != safe_seq['_M_version']:

>+        if not safe_seq:

>+            return str(self.val.cast(itype))


So what's the effect of this change when we get a value-initialized
debug iterator? It prints the wrapped (value-initialized) non-debug
iterator instead?
François Dumont March 7, 2018, 5:39 p.m. | #2
On 06/03/2018 22:21, Jonathan Wakely wrote:
>

>> @@ -575,10 +586,12 @@ class StdDebugIteratorPrinter:

>>      # and return the wrapped iterator value.

>>      def to_string (self):

>>          base_type = gdb.lookup_type('__gnu_debug::_Safe_iterator_base')

>> +        itype = self.val.type.template_argument(0)

>>          safe_seq = self.val.cast(base_type)['_M_sequence']

>> -        if not safe_seq or self.val['_M_version'] != safe_seq['_M_version']:

>> +        if not safe_seq:

>> +            return str(self.val.cast(itype))

> So what's the effect of this change when we get a value-initialized

> debug iterator? It prints the wrapped (value-initialized) non-debug

> iterator instead?

>

Yes, for instance this test in smiple11.cc:


   std::deque<int>::iterator deqiter0;
// { dg-final { note-test deqiter0 {non-dereferenceable iterator for 
std::deque} } }

     Was failing when running 'make check-debug 
RUNTESTFLAGS=prettyprinters.exp' because it was displaying 'invalid 
iterator' rather than non-dereferenceable. Now it is succeeded.

François
Jonathan Wakely March 7, 2018, 11:02 p.m. | #3
On 7 March 2018 at 17:39, François Dumont wrote:
> On 06/03/2018 22:21, Jonathan Wakely wrote:

>>

>>

>>> @@ -575,10 +586,12 @@ class StdDebugIteratorPrinter:

>>>      # and return the wrapped iterator value.

>>>      def to_string (self):

>>>          base_type = gdb.lookup_type('__gnu_debug::_Safe_iterator_base')

>>> +        itype = self.val.type.template_argument(0)

>>>          safe_seq = self.val.cast(base_type)['_M_sequence']

>>> -        if not safe_seq or self.val['_M_version'] !=

>>> safe_seq['_M_version']:

>>> +        if not safe_seq:

>>> +            return str(self.val.cast(itype))

>>

>> So what's the effect of this change when we get a value-initialized

>> debug iterator? It prints the wrapped (value-initialized) non-debug

>> iterator instead?

>>

> Yes, for instance this test in smiple11.cc:

>

>

>   std::deque<int>::iterator deqiter0;

> // { dg-final { note-test deqiter0 {non-dereferenceable iterator for

> std::deque} } }

>

>     Was failing when running 'make check-debug

> RUNTESTFLAGS=prettyprinters.exp' because it was displaying 'invalid

> iterator' rather than non-dereferenceable. Now it is succeeded.


OK, thanks for confirming.

This is OK for trunk, thanks. If anybody complains about the missing
printers for the __norm types we can add them back again.
Jonathan Wakely March 8, 2018, 6:11 p.m. | #4
This caused a new regression for:

  std::forward_list<std::string> flst;
  std::forward_list<std::string>::iterator flstiter0;
// { dg-final { note-test flstiter0 {non-dereferenceable iterator for
std::forward_list}} }


$1 = {_M_node = 0x0}
got: $1 = {_M_node = 0x0}
FAIL: libstdc++-prettyprinters/debug_cxx11.cc print flstiter0
François Dumont March 8, 2018, 9:32 p.m. | #5
On 08/03/2018 19:11, Jonathan Wakely wrote:
> This caused a new regression for:

>

>    std::forward_list<std::string> flst;

>    std::forward_list<std::string>::iterator flstiter0;

> // { dg-final { note-test flstiter0 {non-dereferenceable iterator for

> std::forward_list}} }

>

>

> $1 = {_M_node = 0x0}

> got: $1 = {_M_node = 0x0}

> FAIL: libstdc++-prettyprinters/debug_cxx11.cc print flstiter0

>

Yes, sorry, it looks like I was more concerned by the fixes than by the 
errors on the new test.

This is coming from an attempt to move the forward_list iterators 
outside the std namespace. I'll provide a fix tomorrow.

François
Jonathan Wakely March 8, 2018, 11:09 p.m. | #6
On 8 March 2018 at 21:32, François Dumont <frs.dumont@gmail.com> wrote:
> On 08/03/2018 19:11, Jonathan Wakely wrote:

>>

>> This caused a new regression for:

>>

>>    std::forward_list<std::string> flst;

>>    std::forward_list<std::string>::iterator flstiter0;

>> // { dg-final { note-test flstiter0 {non-dereferenceable iterator for

>> std::forward_list}} }

>>

>>

>> $1 = {_M_node = 0x0}

>> got: $1 = {_M_node = 0x0}

>> FAIL: libstdc++-prettyprinters/debug_cxx11.cc print flstiter0

>>

> Yes, sorry, it looks like I was more concerned by the fixes than by the

> errors on the new test.

>

> This is coming from an attempt to move the forward_list iterators outside

> the std namespace. I'll provide a fix tomorrow.


Thanks!


> François

>
François Dumont March 9, 2018, 5:57 a.m. | #7
On 08/03/2018 19:11, Jonathan Wakely wrote:
> This caused a new regression for:

>

>    std::forward_list<std::string> flst;

>    std::forward_list<std::string>::iterator flstiter0;

> // { dg-final { note-test flstiter0 {non-dereferenceable iterator for

> std::forward_list}} }

>

>

> $1 = {_M_node = 0x0}

> got: $1 = {_M_node = 0x0}

> FAIL: libstdc++-prettyprinters/debug_cxx11.cc print flstiter0

>

Fixed by the attached patch, committed as trivial.

François
diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index b5f76f2..45aaa12 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -1744,10 +1744,10 @@ def build_libstdcxx_dictionary ():
                                       StdVectorIteratorPrinter)
         libstdcxx_printer.add_version('__gnu_cxx::', '_Slist_iterator',
                                       StdSlistIteratorPrinter)
-        libstdcxx_printer.add_version('__gnu_cxx::', '_Fwd_list_iterator',
-                                      StdFwdListIteratorPrinter)
-        libstdcxx_printer.add_version('__gnu_cxx::', '_Fwd_list_const_iterator',
-                                      StdFwdListIteratorPrinter)
+        libstdcxx_printer.add_container('std::', '_Fwd_list_iterator',
+                                        StdFwdListIteratorPrinter)
+        libstdcxx_printer.add_container('std::', '_Fwd_list_const_iterator',
+                                        StdFwdListIteratorPrinter)
 
         # Debug (compiled with -D_GLIBCXX_DEBUG) printer
         # registrations.

Patch

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index d42d72c..f1cde11 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -249,21 +249,32 @@  class StdListPrinter:
             return 'empty %s' % (self.typename)
         return '%s' % (self.typename)
 
-class StdListIteratorPrinter:
-    "Print std::list::iterator"
-
-    def __init__(self, typename, val):
+class NodeIteratorPrinter:
+    def __init__(self, typename, val, contname):
         self.val = val
         self.typename = typename
+        self.contname = contname
 
     def to_string(self):
         if not self.val['_M_node']:
-            return 'non-dereferenceable iterator for std::list'
+            return 'non-dereferenceable iterator for std::%s' % (self.contname)
         nodetype = find_type(self.val.type, '_Node')
         nodetype = nodetype.strip_typedefs().pointer()
         node = self.val['_M_node'].cast(nodetype).dereference()
         return str(get_value_from_list_node(node))
 
+class StdListIteratorPrinter(NodeIteratorPrinter):
+    "Print std::list::iterator"
+
+    def __init__(self, typename, val):
+        NodeIteratorPrinter.__init__(self, typename, val, 'list')
+
+class StdFwdListIteratorPrinter(NodeIteratorPrinter):
+    "Print std::forward_list::iterator"
+
+    def __init__(self, typename, val):
+        NodeIteratorPrinter.__init__(self, typename, val, 'forward_list')
+
 class StdSlistPrinter:
     "Print a __gnu_cxx::slist"
 
@@ -575,10 +586,12 @@  class StdDebugIteratorPrinter:
     # and return the wrapped iterator value.
     def to_string (self):
         base_type = gdb.lookup_type('__gnu_debug::_Safe_iterator_base')
+        itype = self.val.type.template_argument(0)
         safe_seq = self.val.cast(base_type)['_M_sequence']
-        if not safe_seq or self.val['_M_version'] != safe_seq['_M_version']:
+        if not safe_seq:
+            return str(self.val.cast(itype))
+        if self.val['_M_version'] != safe_seq['_M_version']:
             return "invalid iterator"
-        itype = self.val.type.template_argument(0)
         return str(self.val.cast(itype))
 
 def num_elements(num):
@@ -1731,21 +1744,14 @@  def build_libstdcxx_dictionary ():
                                       StdVectorIteratorPrinter)
         libstdcxx_printer.add_version('__gnu_cxx::', '_Slist_iterator',
                                       StdSlistIteratorPrinter)
+        libstdcxx_printer.add_version('__gnu_cxx::', '_Fwd_list_iterator',
+                                      StdFwdListIteratorPrinter)
+        libstdcxx_printer.add_version('__gnu_cxx::', '_Fwd_list_const_iterator',
+                                      StdFwdListIteratorPrinter)
 
         # Debug (compiled with -D_GLIBCXX_DEBUG) printer
-        # registrations.  The Rb_tree debug iterator when unwrapped
-        # from the encapsulating __gnu_debug::_Safe_iterator does not
-        # have the __norm namespace. Just use the existing printer
-        # registration for that.
+        # registrations.
         libstdcxx_printer.add('__gnu_debug::_Safe_iterator',
                               StdDebugIteratorPrinter)
-        libstdcxx_printer.add('std::__norm::_List_iterator',
-                              StdListIteratorPrinter)
-        libstdcxx_printer.add('std::__norm::_List_const_iterator',
-                              StdListIteratorPrinter)
-        libstdcxx_printer.add('std::__norm::_Deque_const_iterator',
-                              StdDequeIteratorPrinter)
-        libstdcxx_printer.add('std::__norm::_Deque_iterator',
-                              StdDequeIteratorPrinter)
 
 build_libstdcxx_dictionary ()
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug.cc
index 72b2eeb..9f7f763 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug.cc
@@ -19,7 +19,9 @@ 
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-#define _GLIBCXX_DEBUG
+#ifndef _GLIBCXX_DEBUG
+# define _GLIBCXX_DEBUG
+#endif
 
 #include <string>
 #include <deque>
@@ -96,7 +98,7 @@  main()
   v.push_back(1);
   v.push_back(2);
   std::vector<int>::iterator viter0;
-// { dg-final { note-test viter0 {invalid iterator} } }
+// { dg-final { note-test viter0 {non-dereferenceable iterator for std::vector} } }
   std::vector<int>::iterator viter1 = v.begin();
   std::vector<int>::iterator viter2 = viter1 + 1;
   v.erase(viter1);
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug_cxx11.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug_cxx11.cc
index 5d98350..1990084 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug_cxx11.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/debug_cxx11.cc
@@ -19,7 +19,9 @@ 
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-#define _GLIBCXX_DEBUG
+#ifndef _GLIBCXX_DEBUG
+# define _GLIBCXX_DEBUG
+#endif
 
 #include <forward_list>
 #include <unordered_map>
@@ -31,7 +33,7 @@  main()
 {
   std::forward_list<std::string> flst;
   std::forward_list<std::string>::iterator flstiter0;
-// { dg-final { note-test flstiter0 {invalid iterator}} }
+// { dg-final { note-test flstiter0 {non-dereferenceable iterator for std::forward_list}} }
   flst.push_front("dum");
   std::forward_list<std::string>::iterator flstiter1 = flst.begin();
 // { dg-final { note-test *flstiter1 {"dum"}} }