[patch] [python libstdc++ printers] Fix gdb/15195

[patch] [python libstdc++ printers] Fix gdb/15195

[Phil Muldoon]

This patch fixes a bug in the std::tuple printer where, if the value
was passed by reference, the printer was not correctly dereferencing the
value before printing.

Cheers,

Phil


2013-03-21  Phil Muldoon  <pmuldoon@redhat.com>

	PR gdb/15195

	* python/libstdcxx/v6/printers.py (StdTuplePrinter): Convert
	referenced value to actual if type is TYPE_CODE_REF.

--

Index: printers.py
===================================================================
--- printers.py	(revision 196862)
+++ printers.py	(working copy)
@@ -318,7 +318,12 @@
         return self._iterator (self.val)
 
     def to_string (self):
-        if len (self.val.type.fields ()) == 0:
+        type = self.val.type
+        if type.code == gdb.TYPE_CODE_REF:
+            self.val = self.val.referenced_value()
+            type = self.val.type
+
+        if len(type.fields ()) == 0:
             return 'empty %s' % (self.typename)
         return '%s containing' % (self.typename)
 
	

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> 2013-03-21  Phil Muldoon  <pmuldoon@redhat.com>
Phil> 	PR gdb/15195

I think this should use a full URL.
Otherwise it is going to attach the commit to some random GCC bug.

Phil> 	* python/libstdcxx/v6/printers.py (StdTuplePrinter): Convert
Phil> 	referenced value to actual if type is TYPE_CODE_REF.

This should reference the function, like "(StdTuplePrinter.to_string):"

Phil>      def to_string (self):

Why doesn't the 'children' method also need a fix?

If they both need the fix you might as well put the dereferencing into
the constructor.


Also, Phil and I discussed this on irc, and he is going to write a
regression test.

thanks,
Tom

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Phil Muldoon]

On 21/03/13 14:20, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> 2013-03-21  Phil Muldoon  <pmuldoon@redhat.com>
> Phil> 	PR gdb/15195
> 
> I think this should use a full URL.

> Phil>      def to_string (self):
> 
> Why doesn't the 'children' method also need a fix?
> 
> Also, Phil and I discussed this on irc, and he is going to write a
> regression test.

Hi,

Attached is an updated patch correcting the issues that you pointed
out.

Cheers,

Phil


2013-06-11  Phil Muldoon  <pmuldoon@redhat.com>

	http://sourceware.org/bugzilla/show_bug.cgi?id=15195

	* python/libstdcxx/v6/printers.py (StdTuplePrinter.__init__):
	Acquire referenced value if tuple is a reference.
	* testsuite/libstdc++-prettyprinters/cxx11.cc (main): Add -O0
	flag. Add tuple and referenced tuple tests.

--

Index: python/libstdcxx/v6/printers.py
===================================================================
--- python/libstdcxx/v6/printers.py	(revision 199642)
+++ python/libstdcxx/v6/printers.py	(working copy)
@@ -313,6 +313,9 @@
     def __init__ (self, typename, val):
         self.typename = typename
         self.val = val;
+        type = self.val.type
+        if type.code == gdb.TYPE_CODE_REF:
+           self.val = self.val.referenced_value()
 
     def children (self):
         return self._iterator (self.val)
Index: testsuite/libstdc++-prettyprinters/cxx11.cc
===================================================================
--- testsuite/libstdc++-prettyprinters/cxx11.cc	(revision 199706)
+++ testsuite/libstdc++-prettyprinters/cxx11.cc	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do run }
-// { dg-options "-std=gnu++11 -g" }
+// { dg-options "-std=gnu++11 -g -O0" }
 
 // Copyright (C) 2011-2013 Free Software Foundation, Inc.
 //
@@ -25,6 +25,8 @@
 #include <memory>
 #include <iostream>
 
+typedef std::tuple<int, int> ExTuple;
+
 template<class T>
 void
 placeholder(const T &s)
@@ -100,6 +102,10 @@
   uptr->i = 23;
 // { dg-final { regexp-test uptr {std::unique_ptr.datum. containing 0x.*} } }
 
+  ExTuple tpl(6,7);
+// { dg-final { note-test tpl {std::tuple containing = {[1] = 6, [2] = 7}} } }  
+  ExTuple &rtpl = tpl;
+// { dg-final { note-test rtpl {std::tuple containing = {[1] = 6, [2] = 7}} } }   
   placeholder(""); // Mark SPOT
   use(efl);
   use(fl);
	

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> Attached is an updated patch correcting the issues that you pointed
Phil> out.

The patch itself looks fine to me, but I don't think I can approve it.

Tom

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Phil Muldoon]

On 13/06/13 14:49, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> Attached is an updated patch correcting the issues that you pointed
> Phil> out.
> 
> The patch itself looks fine to me, but I don't think I can approve it.
> 
> Tom

This new patch replaces and obsoletes the previous.  On further
inspection of some other pretty printer related bugs, it seems that
all of the printers need to fetch the referenced value where the value
type is a reference.  This patch applies that change, and adds a
number of reference based tests.

Cheers,

Phil

2013-07-03  Phil Muldoon  <pmuldoon@redhat.com>

	PR gcc/53477
	http://sourceware.org/bugzilla/show_bug.cgi?id=15195

	* python/libstdcxx/v6/printers.py (Printer.__call__): If a value
	is a reference, fetch referenced value.
	(RxPrinter.invoke): Ditto.
	* testsuite/libstdc++-prettyprinters/cxx11.cc (main): Add -O0
	flag. Add referenced value tests.

--

Index: python/libstdcxx/v6/printers.py
===================================================================
--- python/libstdcxx/v6/printers.py	(revision 199642)
+++ python/libstdcxx/v6/printers.py	(working copy)
@@ -786,6 +786,10 @@
     def invoke(self, value):
         if not self.enabled:
             return None
+
+        if value.type.code == gdb.TYPE_CODE_REF:
+            value = value.referenced_value()
+
         return self.function(self.name, value)
 
 # A pretty-printer that conforms to the "PrettyPrinter" protocol from
@@ -841,6 +845,10 @@
             return None
 
         basename = match.group(1)
+
+        if val.type.code == gdb.TYPE_CODE_REF:
+            val = val.referenced_value()
+
         if basename in self.lookup:
             return self.lookup[basename].invoke(val)
 
Index: testsuite/libstdc++-prettyprinters/cxx11.cc
===================================================================
--- testsuite/libstdc++-prettyprinters/cxx11.cc	(revision 199706)
+++ testsuite/libstdc++-prettyprinters/cxx11.cc	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do run }
-// { dg-options "-std=gnu++11 -g" }
+// { dg-options "-std=gnu++11 -g -O0" }
 
 // Copyright (C) 2011-2013 Free Software Foundation, Inc.
 //
@@ -25,6 +25,8 @@
 #include <memory>
 #include <iostream>
 
+typedef std::tuple<int, int> ExTuple;
+
 template<class T>
 void
 placeholder(const T &s)
@@ -63,43 +65,75 @@
   std::forward_list<int> efl;
 // { dg-final { note-test efl "empty std::forward_list" } }
 
+  std::forward_list<int> &refl = efl;
+// { dg-final { note-test refl "empty std::forward_list" } }
+
   std::forward_list<int> fl;
   fl.push_front(2);
   fl.push_front(1);
 // { dg-final { note-test fl {std::forward_list = {[0] = 1, [1] = 2}} } }
 
+  std::forward_list<int> &rfl = fl;
+// { dg-final { note-test rfl {std::forward_list = {[0] = 1, [1] = 2}} } }
+
   std::unordered_map<int, std::string> eum;
 // { dg-final { note-test eum "std::unordered_map with 0 elements" } }
+  std::unordered_map<int, std::string> &reum = eum;
+// { dg-final { note-test reum "std::unordered_map with 0 elements" } }
+
   std::unordered_multimap<int, std::string> eumm;
 // { dg-final { note-test eumm "std::unordered_multimap with 0 elements" } }
+  std::unordered_multimap<int, std::string> &reumm = eumm;
+// { dg-final { note-test reumm "std::unordered_multimap with 0 elements" } }
+
   std::unordered_set<int> eus;
 // { dg-final { note-test eus "std::unordered_set with 0 elements" } }
+  std::unordered_set<int> &reus = eus;
+// { dg-final { note-test reus "std::unordered_set with 0 elements" } }
+
   std::unordered_multiset<int> eums;
 // { dg-final { note-test eums "std::unordered_multiset with 0 elements" } }
+  std::unordered_multiset<int> &reums = eums;
+// { dg-final { note-test reums "std::unordered_multiset with 0 elements" } }
 
   std::unordered_map<int, std::string> uom;
   uom[5] = "three";
   uom[3] = "seven";
 // { dg-final { note-test uom {std::unordered_map with 2 elements = {[3] = "seven", [5] = "three"}} } }
 
+  std::unordered_map<int, std::string> &ruom = uom;
+// { dg-final { note-test ruom {std::unordered_map with 2 elements = {[3] = "seven", [5] = "three"}} } }
+
   std::unordered_multimap<int, std::string> uomm;
   uomm.insert(std::pair<int, std::string> (5, "three"));
   uomm.insert(std::pair<int, std::string> (5, "seven"));
 // { dg-final { note-test uomm {std::unordered_multimap with 2 elements = {[5] = "seven", [5] = "three"}} } }
+  std::unordered_multimap<int, std::string> &ruomm = uomm;
+// { dg-final { note-test ruomm {std::unordered_multimap with 2 elements = {[5] = "seven", [5] = "three"}} } }
 
   std::unordered_set<int> uos;
   uos.insert(5);
 // { dg-final { note-test uos {std::unordered_set with 1 elements = {[0] = 5}} } }
+  std::unordered_set<int> &ruos = uos;
+// { dg-final { note-test ruos {std::unordered_set with 1 elements = {[0] = 5}} } }
 
   std::unordered_multiset<int> uoms;
   uoms.insert(5);
 // { dg-final { note-test uoms {std::unordered_multiset with 1 elements = {[0] = 5}} } }
+  std::unordered_multiset<int> &ruoms = uoms;
+// { dg-final { note-test ruoms {std::unordered_multiset with 1 elements = {[0] = 5}} } }
 
   std::unique_ptr<datum> uptr (new datum);
   uptr->s = "hi bob";
   uptr->i = 23;
 // { dg-final { regexp-test uptr {std::unique_ptr.datum. containing 0x.*} } }
+  std::unique_ptr<datum> &ruptr = uptr;
+// { dg-final { regexp-test ruptr {std::unique_ptr.datum. containing 0x.*} } }
 
+  ExTuple tpl(6,7);
+// { dg-final { note-test tpl {std::tuple containing = {[1] = 6, [2] = 7}} } }
+  ExTuple &rtpl = tpl;
+// { dg-final { note-test rtpl {std::tuple containing = {[1] = 6, [2] = 7}} } }
   placeholder(""); // Mark SPOT
   use(efl);
   use(fl);

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> 2013-07-03  Phil Muldoon  <pmuldoon@redhat.com>
Phil> 	PR gcc/53477
Phil> 	http://sourceware.org/bugzilla/show_bug.cgi?id=15195
Phil> 	* python/libstdcxx/v6/printers.py (Printer.__call__): If a value
Phil> 	is a reference, fetch referenced value.
Phil> 	(RxPrinter.invoke): Ditto.
Phil> 	* testsuite/libstdc++-prettyprinters/cxx11.cc (main): Add -O0
Phil> 	flag. Add referenced value tests.

Thanks Phil.

Remember to CC <libstdc++@gcc.gnu.org> on these notes.

Phil> +        if value.type.code == gdb.TYPE_CODE_REF:
Phil> +            value = value.referenced_value()
Phil> +

I think this code should test for the existence of referenced_value
using hasattr.  Maybe somebody is still on gdb 7.4.

Tom

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Phil Muldoon]

On 03/07/13 08:33, Phil Muldoon wrote:

> This new patch replaces and obsoletes the previous.  On further
> inspection of some other pretty printer related bugs, it seems that
> all of the printers need to fetch the referenced value where the value
> type is a reference.  This patch applies that change, and adds a
> number of reference based tests.

ping

Cheers,

Phil

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> On 03/07/13 08:33, Phil Muldoon wrote:
>> This new patch replaces and obsoletes the previous.  On further
>> inspection of some other pretty printer related bugs, it seems that
>> all of the printers need to fetch the referenced value where the value
>> type is a reference.  This patch applies that change, and adds a
>> number of reference based tests.

Phil> ping

I sent a reply last Thursday:

http://gcc.gnu.org/ml/gcc-patches/2013-07/msg00713.html

Tom

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Phil Muldoon]

On 23/07/13 15:23, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> On 03/07/13 08:33, Phil Muldoon wrote:
>>> This new patch replaces and obsoletes the previous.  On further
>>> inspection of some other pretty printer related bugs, it seems that
>>> all of the printers need to fetch the referenced value where the value
>>> type is a reference.  This patch applies that change, and adds a
>>> number of reference based tests.
> 
> Phil> ping
> 
> I sent a reply last Thursday:
> 
> http://gcc.gnu.org/ml/gcc-patches/2013-07/msg00713.html

Weirdly I did not get this email in my inbox (but I see it right there
in the archives).

Anyway, I have regenerated the patch with the fixes requested.

Cheers,

Phil

2013-08-16  Phil Muldoon  <pmuldoon@redhat.com>

	PR gcc/53477
	http://sourceware.org/bugzilla/show_bug.cgi?id=15195

	* python/libstdcxx/v6/printers.py (Printer.__call__): If a value
	is a reference, fetch referenced value.
	(RxPrinter.invoke): Ditto.
	* testsuite/libstdc++-prettyprinters/cxx11.cc (main): Add -O0
	flag. Add referenced value tests.

--

Index: libstdc++-v3/python/libstdcxx/v6/printers.py
===================================================================
--- libstdc++-v3/python/libstdcxx/v6/printers.py	(revision 199642)
+++ libstdc++-v3/python/libstdcxx/v6/printers.py	(working copy)
@@ -786,6 +786,11 @@
     def invoke(self, value):
         if not self.enabled:
             return None
+
+        if value.type.code == gdb.TYPE_CODE_REF:
+            if hasattr(gdb.Value,"referenced_value"):
+                value = value.referenced_value()
+
         return self.function(self.name, value)
 
 # A pretty-printer that conforms to the "PrettyPrinter" protocol from
@@ -841,6 +846,11 @@
             return None
 
         basename = match.group(1)
+
+        if val.type.code == gdb.TYPE_CODE_REF:
+            if hasattr(gdb.Value,"referenced_value"):
+                val = val.referenced_value()
+
         if basename in self.lookup:
             return self.lookup[basename].invoke(val)
 
Index: libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
===================================================================
--- libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc	(revision 199706)
+++ libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do run }
-// { dg-options "-std=gnu++11 -g" }
+// { dg-options "-std=gnu++11 -g -O0" }
 
 // Copyright (C) 2011-2013 Free Software Foundation, Inc.
 //
@@ -25,6 +25,8 @@
 #include <memory>
 #include <iostream>
 
+typedef std::tuple<int, int> ExTuple;
+
 template<class T>
 void
 placeholder(const T &s)
@@ -63,43 +65,75 @@
   std::forward_list<int> efl;
 // { dg-final { note-test efl "empty std::forward_list" } }
 
+  std::forward_list<int> &refl = efl;
+// { dg-final { note-test refl "empty std::forward_list" } }
+
   std::forward_list<int> fl;
   fl.push_front(2);
   fl.push_front(1);
 // { dg-final { note-test fl {std::forward_list = {[0] = 1, [1] = 2}} } }
 
+  std::forward_list<int> &rfl = fl;
+// { dg-final { note-test rfl {std::forward_list = {[0] = 1, [1] = 2}} } }
+
   std::unordered_map<int, std::string> eum;
 // { dg-final { note-test eum "std::unordered_map with 0 elements" } }
+  std::unordered_map<int, std::string> &reum = eum;
+// { dg-final { note-test reum "std::unordered_map with 0 elements" } }
+
   std::unordered_multimap<int, std::string> eumm;
 // { dg-final { note-test eumm "std::unordered_multimap with 0 elements" } }
+  std::unordered_multimap<int, std::string> &reumm = eumm;
+// { dg-final { note-test reumm "std::unordered_multimap with 0 elements" } }
+
   std::unordered_set<int> eus;
 // { dg-final { note-test eus "std::unordered_set with 0 elements" } }
+  std::unordered_set<int> &reus = eus;
+// { dg-final { note-test reus "std::unordered_set with 0 elements" } }
+
   std::unordered_multiset<int> eums;
 // { dg-final { note-test eums "std::unordered_multiset with 0 elements" } }
+  std::unordered_multiset<int> &reums = eums;
+// { dg-final { note-test reums "std::unordered_multiset with 0 elements" } }
 
   std::unordered_map<int, std::string> uom;
   uom[5] = "three";
   uom[3] = "seven";
 // { dg-final { note-test uom {std::unordered_map with 2 elements = {[3] = "seven", [5] = "three"}} } }
 
+  std::unordered_map<int, std::string> &ruom = uom;
+// { dg-final { note-test ruom {std::unordered_map with 2 elements = {[3] = "seven", [5] = "three"}} } }
+
   std::unordered_multimap<int, std::string> uomm;
   uomm.insert(std::pair<int, std::string> (5, "three"));
   uomm.insert(std::pair<int, std::string> (5, "seven"));
 // { dg-final { note-test uomm {std::unordered_multimap with 2 elements = {[5] = "seven", [5] = "three"}} } }
+  std::unordered_multimap<int, std::string> &ruomm = uomm;
+// { dg-final { note-test ruomm {std::unordered_multimap with 2 elements = {[5] = "seven", [5] = "three"}} } }
 
   std::unordered_set<int> uos;
   uos.insert(5);
 // { dg-final { note-test uos {std::unordered_set with 1 elements = {[0] = 5}} } }
+  std::unordered_set<int> &ruos = uos;
+// { dg-final { note-test ruos {std::unordered_set with 1 elements = {[0] = 5}} } }
 
   std::unordered_multiset<int> uoms;
   uoms.insert(5);
 // { dg-final { note-test uoms {std::unordered_multiset with 1 elements = {[0] = 5}} } }
+  std::unordered_multiset<int> &ruoms = uoms;
+// { dg-final { note-test ruoms {std::unordered_multiset with 1 elements = {[0] = 5}} } }
 
   std::unique_ptr<datum> uptr (new datum);
   uptr->s = "hi bob";
   uptr->i = 23;
 // { dg-final { regexp-test uptr {std::unique_ptr.datum. containing 0x.*} } }
+  std::unique_ptr<datum> &ruptr = uptr;
+// { dg-final { regexp-test ruptr {std::unique_ptr.datum. containing 0x.*} } }
 
+  ExTuple tpl(6,7);
+// { dg-final { note-test tpl {std::tuple containing = {[1] = 6, [2] = 7}} } }  
+  ExTuple &rtpl = tpl;
+// { dg-final { note-test rtpl {std::tuple containing = {[1] = 6, [2] = 7}} } }   
   placeholder(""); // Mark SPOT
   use(efl);
   use(fl);

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> Anyway, I have regenerated the patch with the fixes requested.

Thanks.

Phil> 2013-08-16  Phil Muldoon  <pmuldoon@redhat.com>

Phil> 	PR gcc/53477

I think this should say  PR libstdc++/53477

Other than this nit, it looks good to me.
I can't remember if I can approve these, but I think not.

Tom

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Jonathan Wakely]

On 16 August 2013 16:28, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> Anyway, I have regenerated the patch with the fixes requested.
>
> Thanks.
>
> Phil> 2013-08-16  Phil Muldoon  <pmuldoon@redhat.com>
>
> Phil>   PR gcc/53477
>
> I think this should say  PR libstdc++/53477
>
> Other than this nit, it looks good to me.
> I can't remember if I can approve these, but I think not.

I can though, so it's approved with the ChangeLog nit fixed.

Thanks.

Re: [patch] [python libstdc++ printers] Fix gdb/15195

[Phil Muldoon]

On 19/08/13 09:00, Jonathan Wakely wrote:
> On 16 August 2013 16:28, Tom Tromey wrote:
>>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>>
>> Phil> Anyway, I have regenerated the patch with the fixes requested.
>>
>> Thanks.
>>
>> Phil> 2013-08-16  Phil Muldoon  <pmuldoon@redhat.com>
>>
>> Phil>   PR gcc/53477
>>
>> I think this should say  PR libstdc++/53477
>>
>> Other than this nit, it looks good to me.
>> I can't remember if I can approve these, but I think not.
> 
> I can though, so it's approved with the ChangeLog nit fixed.

Committed with ChangeLog fix:


2013-08-20  Phil Muldoon  <pmuldoon@redhat.com>

	PR libstdc++/53477
	http://sourceware.org/bugzilla/show_bug.cgi?id=15195

	* python/libstdcxx/v6/printers.py (Printer.__call__): If a value
	is a reference, fetch referenced value.
	(RxPrinter.invoke): Ditto.
	* testsuite/libstdc++-prettyprinters/cxx11.cc (main): Add -O0
	flag. Add referenced value tests.

Patch was as posted.


Cheers,

Phil