public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr
@ 2020-07-28 19:05 Andres Rodriguez
  0 siblings, 0 replies; 5+ messages in thread
From: Andres Rodriguez @ 2020-07-28 19:05 UTC (permalink / raw)
  To: gcc-patches; +Cc: andresx7

On binaries compiled against gcc5 the impl_type parameter is None,
which results in an exception being raised by is_specialization_of()

These versions of std::unique_ptr have the tuple as a root element.
---

Hi,

I ran into this issue when debugging a binary built using gcc5.

I'm not very familiar with python or the gcc codebase, so this might be
the wrong way to address this problem. But a patch seemed like a good
way to start the conversation.

Thanks for taking a look.

-Andres


 libstdc++-v3/python/libstdcxx/v6/printers.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index e4da8dfe5b6..3154a2a6f9d 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -247,7 +247,9 @@ class UniquePointerPrinter:
         self.val = val
         impl_type = val.type.fields()[0].type.tag
         # Check for new implementations first:
-        if is_specialization_of(impl_type, '__uniq_ptr_data') \
+        if impl_type is None:
+            tuple_member = val['_M_t']
+        elif is_specialization_of(impl_type, '__uniq_ptr_data') \
             or is_specialization_of(impl_type, '__uniq_ptr_impl'):
             tuple_member = val['_M_t']['_M_t']
         elif is_specialization_of(impl_type, 'tuple'):
-- 
2.27.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr
  2020-08-10 17:49   ` Jonathan Wakely
@ 2020-08-10 18:20     ` Andres Rodriguez
  0 siblings, 0 replies; 5+ messages in thread
From: Andres Rodriguez @ 2020-08-10 18:20 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches, libstdc++

On Mon, Aug 10, 2020 at 1:49 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On 10/08/20 09:45 -0400, Andres Rodriguez wrote:
> >*ping*
>
> As it says at https://gcc.gnu.org/lists.html all patches for libstdc++
> need to be sent to the libstdc++ mailing as well as the gcc-patches
> list. Otherwise I won't see them, and everybody else will ignore them.
>

Thanks for the heads up on the libstdc++ mailing list and for the
unique_ptr fix.

-Andres


> >On Tue, Aug 4, 2020 at 10:51 AM Andres Rodriguez <andresx7@gmail.com> wrote:
> >>
> >> On binaries compiled against gcc5 the impl_type parameter is None,
> >> which results in an exception being raised by is_specialization_of()
> >>
> >> These versions of std::unique_ptr have the tuple as a root element.
>
> The unique_ptr implementation in GCC 5 should already be handled by
> the second branch which handles a tuple member:
>
>          if is_specialization_of(impl_type, '__uniq_ptr_data') \
>              or is_specialization_of(impl_type, '__uniq_ptr_impl'):
>              tuple_member = val['_M_t']['_M_t']
>          elif is_specialization_of(impl_type, 'tuple'):
>              tuple_member = val['_M_t']
>          else:
>              raise ValueError("Unsupported implementation for unique_ptr: %s" % impl_type)
>
> We have a test that's supposed to verify that printing the old
> unique_ptr implementation still works:
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/testsuite/libstdc%2B%2B-prettyprinters/compat.cc;h=f1c3b599634af2eb0a5a0889c528d9de2d21aacd;hb=HEAD
>
> So the right fix is to work out why that branch isn't taken, and fix
> it, not add a kluge.
>
> If impl_type is None that means the .tag field is missing, which seems
> to be because the _M_t member is declared using the __tuple_type
> typedef, rather than std::tuple itself.
>
> That's fixed by the attached patch.
>
> Tested x86_64-linux, pushed to master. I'll backport it to the
> branches too.
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr
  2020-08-10 13:45 ` Andres Rodriguez
@ 2020-08-10 17:49   ` Jonathan Wakely
  2020-08-10 18:20     ` Andres Rodriguez
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2020-08-10 17:49 UTC (permalink / raw)
  To: Andres Rodriguez; +Cc: gcc-patches, libstdc++

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

On 10/08/20 09:45 -0400, Andres Rodriguez wrote:
>*ping*

As it says at https://gcc.gnu.org/lists.html all patches for libstdc++
need to be sent to the libstdc++ mailing as well as the gcc-patches
list. Otherwise I won't see them, and everybody else will ignore them.

>On Tue, Aug 4, 2020 at 10:51 AM Andres Rodriguez <andresx7@gmail.com> wrote:
>>
>> On binaries compiled against gcc5 the impl_type parameter is None,
>> which results in an exception being raised by is_specialization_of()
>>
>> These versions of std::unique_ptr have the tuple as a root element.

The unique_ptr implementation in GCC 5 should already be handled by
the second branch which handles a tuple member:

         if is_specialization_of(impl_type, '__uniq_ptr_data') \
             or is_specialization_of(impl_type, '__uniq_ptr_impl'):
             tuple_member = val['_M_t']['_M_t']
         elif is_specialization_of(impl_type, 'tuple'):
             tuple_member = val['_M_t']
         else:
             raise ValueError("Unsupported implementation for unique_ptr: %s" % impl_type)

We have a test that's supposed to verify that printing the old
unique_ptr implementation still works:
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/testsuite/libstdc%2B%2B-prettyprinters/compat.cc;h=f1c3b599634af2eb0a5a0889c528d9de2d21aacd;hb=HEAD

So the right fix is to work out why that branch isn't taken, and fix
it, not add a kluge.

If impl_type is None that means the .tag field is missing, which seems
to be because the _M_t member is declared using the __tuple_type
typedef, rather than std::tuple itself.

That's fixed by the attached patch.

Tested x86_64-linux, pushed to master. I'll backport it to the
branches too.



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

commit ed11f7e84bcae89f486f5023e566726a7faa7dd4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Aug 10 18:44:06 2020

    libstdc++: Fix compatibility support in unique_ptr pretty printer
    
    The support for the old std::unique_ptr implementation was failing,
    because it tried to work on a typedef instead of the underlying type.
    The test supposed to verify the support worked wasn't using a typedef,
    so didn't notice the problem.
    
    libstdc++-v3/ChangeLog:
    
            * python/libstdcxx/v6/printers.py (UniquePointerPrinter.__init__):
            Use gdb.Type.strip_typedefs().
            * testsuite/libstdc++-prettyprinters/compat.cc: Use a typedef in
            the emulated old type.

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 0bf307b8e5f..c0f061f79c1 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -245,7 +245,7 @@ class UniquePointerPrinter:
 
     def __init__ (self, typename, val):
         self.val = val
-        impl_type = val.type.fields()[0].type.tag
+        impl_type = val.type.fields()[0].type.strip_typedefs()
         # Check for new implementations first:
         if is_specialization_of(impl_type, '__uniq_ptr_data') \
             or is_specialization_of(impl_type, '__uniq_ptr_impl'):
@@ -253,7 +253,7 @@ class UniquePointerPrinter:
         elif is_specialization_of(impl_type, 'tuple'):
             tuple_member = val['_M_t']
         else:
-            raise ValueError("Unsupported implementation for unique_ptr: %s" % impl_type)
+            raise ValueError("Unsupported implementation for unique_ptr: %s" % str(impl_type))
         tuple_impl_type = tuple_member.type.fields()[0].type # _Tuple_impl
         tuple_head_type = tuple_impl_type.fields()[1].type   # _Head_base
         head_field = tuple_head_type.fields()[0]
@@ -262,7 +262,7 @@ class UniquePointerPrinter:
         elif head_field.is_base_class:
             self.pointer = tuple_member.cast(head_field.type)
         else:
-            raise ValueError("Unsupported implementation for tuple in unique_ptr: %s" % impl_type)
+            raise ValueError("Unsupported implementation for tuple in unique_ptr: %s" % str(impl_type))
 
     def children (self):
         return SmartPtrIterator(self.pointer)
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
index f1c3b599634..c681becf8b9 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
@@ -53,7 +53,9 @@ namespace std
     {
       unique_ptr(T* p) { _M_t._M_head_impl = p; }
 
-      tuple<T*, D> _M_t;
+      using __tuple_type = tuple<T*, D>;
+
+      __tuple_type _M_t;
     };
 
   // Old representation of std::optional, before GCC 9

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr
  2020-08-04 14:51 Andres Rodriguez
@ 2020-08-10 13:45 ` Andres Rodriguez
  2020-08-10 17:49   ` Jonathan Wakely
  0 siblings, 1 reply; 5+ messages in thread
From: Andres Rodriguez @ 2020-08-10 13:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: jwakely

*ping*

On Tue, Aug 4, 2020 at 10:51 AM Andres Rodriguez <andresx7@gmail.com> wrote:
>
> On binaries compiled against gcc5 the impl_type parameter is None,
> which results in an exception being raised by is_specialization_of()
>
> These versions of std::unique_ptr have the tuple as a root element.
> ---
>
> Hi,
>
> I ran into this issue when debugging a binary built using gcc5.
>
> I'm not very familiar with python or the gcc codebase, so this might be
> the wrong way to address this problem. But a patch seemed like a good
> way to start the conversation.
>
> Thanks for taking a look.
>
> -Andres
>
> P.S.: This is a resend of the patch. I joined the mailing list after
> sending this patch so I'm guessing the original email got stuck in a
> moderation queue.
>
>
>  libstdc++-v3/python/libstdcxx/v6/printers.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
> index e4da8dfe5b6..3154a2a6f9d 100644
> --- a/libstdc++-v3/python/libstdcxx/v6/printers.py
> +++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
> @@ -247,7 +247,9 @@ class UniquePointerPrinter:
>          self.val = val
>          impl_type = val.type.fields()[0].type.tag
>          # Check for new implementations first:
> -        if is_specialization_of(impl_type, '__uniq_ptr_data') \
> +        if impl_type is None:
> +            tuple_member = val['_M_t']
> +        elif is_specialization_of(impl_type, '__uniq_ptr_data') \
>              or is_specialization_of(impl_type, '__uniq_ptr_impl'):
>              tuple_member = val['_M_t']['_M_t']
>          elif is_specialization_of(impl_type, 'tuple'):
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr
@ 2020-08-04 14:51 Andres Rodriguez
  2020-08-10 13:45 ` Andres Rodriguez
  0 siblings, 1 reply; 5+ messages in thread
From: Andres Rodriguez @ 2020-08-04 14:51 UTC (permalink / raw)
  To: gcc-patches

On binaries compiled against gcc5 the impl_type parameter is None,
which results in an exception being raised by is_specialization_of()

These versions of std::unique_ptr have the tuple as a root element.
---

Hi,

I ran into this issue when debugging a binary built using gcc5.

I'm not very familiar with python or the gcc codebase, so this might be
the wrong way to address this problem. But a patch seemed like a good
way to start the conversation.

Thanks for taking a look.

-Andres

P.S.: This is a resend of the patch. I joined the mailing list after
sending this patch so I'm guessing the original email got stuck in a
moderation queue.


 libstdc++-v3/python/libstdcxx/v6/printers.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index e4da8dfe5b6..3154a2a6f9d 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -247,7 +247,9 @@ class UniquePointerPrinter:
         self.val = val
         impl_type = val.type.fields()[0].type.tag
         # Check for new implementations first:
-        if is_specialization_of(impl_type, '__uniq_ptr_data') \
+        if impl_type is None:
+            tuple_member = val['_M_t']
+        elif is_specialization_of(impl_type, '__uniq_ptr_data') \
             or is_specialization_of(impl_type, '__uniq_ptr_impl'):
             tuple_member = val['_M_t']['_M_t']
         elif is_specialization_of(impl_type, 'tuple'):
-- 
2.27.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-08-10 18:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 19:05 [RFC] libstdc++: Fix pretty-printing old implementations of std::unique_ptr Andres Rodriguez
2020-08-04 14:51 Andres Rodriguez
2020-08-10 13:45 ` Andres Rodriguez
2020-08-10 17:49   ` Jonathan Wakely
2020-08-10 18:20     ` Andres Rodriguez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).