public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH PR gdb/16959] gdb hangs in infinite recursion
@ 2018-03-16  1:07 Weimin Pan
  2018-03-17 17:13 ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Weimin Pan @ 2018-03-16  1:07 UTC (permalink / raw)
  To: gdb-patches

The original problem was fixed (see related PR 22242). But using a typedef
as the declared type for a static member variable, as commented in this PR,
is still causing gdb to get into infinite loop when printing the static
member's value. This problem can be reproduced as follows:

% cat t.cc
class A {
    typedef A type;
public:
    bool operator==(const type& other) { return true; }

    static const type INSTANCE;
};

const A A::INSTANCE;

int main() {
    A a;
    if (a == A::INSTANCE) {
        return -1;
    }
    return 0;
}
% g++ -g t.cc
% gdb -ex "start" -ex "p a" a.out

The fix is rather trivial - in cp_print_static_field(), should call
check_typedef() to get the static member's real type and use it to
check whether it's a struct or an array.

Tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.
---
 gdb/ChangeLog     |    7 +++++++
 gdb/cp-valprint.c |    2 +-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d0a8dfd..6fd43de 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-02-07  Weimin Pan  <weimin.pan@oracle.com>
+
+	PR gdb/16959
+	* cp-valprint.c: (cp_print_static_field) Use check_typedef() to get 
+	static member's real type for TYPE_CODE_STRUCT and TYPE_CODE_ARRAY 
+	comparisons. 
+
 2018-01-24  Pedro Alves  <palves@redhat.com>
 
 	GCC PR libstdc++/83906
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 486653f..0370b56 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -633,6 +633,7 @@ cp_print_static_field (struct type *type,
       return;
     }
 
+  type = check_typedef (type);
   if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
     {
       CORE_ADDR *first_dont_print;
@@ -658,7 +659,6 @@ cp_print_static_field (struct type *type,
       addr = value_address (val);
       obstack_grow (&dont_print_statmem_obstack, (char *) &addr,
 		    sizeof (CORE_ADDR));
-      type = check_typedef (type);
       cp_print_value_fields (type, value_enclosing_type (val),
 			     value_embedded_offset (val), addr,
 			     stream, recurse, val,
-- 
1.7.1

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

* Re: [PATCH PR gdb/16959] gdb hangs in infinite recursion
  2018-03-16  1:07 [PATCH PR gdb/16959] gdb hangs in infinite recursion Weimin Pan
@ 2018-03-17 17:13 ` Simon Marchi
  2018-03-19 19:46   ` Weimin Pan
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2018-03-17 17:13 UTC (permalink / raw)
  To: Weimin Pan, gdb-patches

On 2018-03-15 08:42 PM, Weimin Pan wrote:
> The original problem was fixed (see related PR 22242). But using a typedef
> as the declared type for a static member variable, as commented in this PR,
> is still causing gdb to get into infinite loop when printing the static
> member's value. This problem can be reproduced as follows:
> 
> % cat t.cc
> class A {
>     typedef A type;
> public:
>     bool operator==(const type& other) { return true; }
> 
>     static const type INSTANCE;
> };
> 
> const A A::INSTANCE;
> 
> int main() {
>     A a;
>     if (a == A::INSTANCE) {
>         return -1;
>     }
>     return 0;
> }
> % g++ -g t.cc
> % gdb -ex "start" -ex "p a" a.out
> 
> The fix is rather trivial - in cp_print_static_field(), should call
> check_typedef() to get the static member's real type and use it to
> check whether it's a struct or an array.

Hi Weimin,

Would it be possible to add a test case for this?  I suppose you can quite
easily enhance the test case added by commit

  a43f3893f6cb ("Fix broken recursion detection when printing static members")

> Tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.
> ---
>  gdb/ChangeLog     |    7 +++++++
>  gdb/cp-valprint.c |    2 +-
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index d0a8dfd..6fd43de 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2018-02-07  Weimin Pan  <weimin.pan@oracle.com>
> +
> +	PR gdb/16959
> +	* cp-valprint.c: (cp_print_static_field) Use check_typedef() to get 
> +	static member's real type for TYPE_CODE_STRUCT and TYPE_CODE_ARRAY 
> +	comparisons. 
> +
>  2018-01-24  Pedro Alves  <palves@redhat.com>
>  
>  	GCC PR libstdc++/83906
> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
> index 486653f..0370b56 100644
> --- a/gdb/cp-valprint.c
> +++ b/gdb/cp-valprint.c
> @@ -633,6 +633,7 @@ cp_print_static_field (struct type *type,
>        return;
>      }
>  
> +  type = check_typedef (type);
>    if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
>      {
>        CORE_ADDR *first_dont_print;
> @@ -658,7 +659,6 @@ cp_print_static_field (struct type *type,
>        addr = value_address (val);
>        obstack_grow (&dont_print_statmem_obstack, (char *) &addr,
>  		    sizeof (CORE_ADDR));
> -      type = check_typedef (type);
>        cp_print_value_fields (type, value_enclosing_type (val),
>  			     value_embedded_offset (val), addr,
>  			     stream, recurse, val,
> 

type is passed below to val_print.  I think it would be better to continue
passing the original type to that function instead of the resolved type.  It
could affect how things are printed (if the type name is printed somewhere,
or if pretty printers are involved).  Many functions use a variable "real_type"
to hold the result from check_typedef, you could follow that pattern.

Thanks,

Simon

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

* Re: [PATCH PR gdb/16959] gdb hangs in infinite recursion
  2018-03-17 17:13 ` Simon Marchi
@ 2018-03-19 19:46   ` Weimin Pan
  0 siblings, 0 replies; 3+ messages in thread
From: Weimin Pan @ 2018-03-19 19:46 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches


On 3/17/2018 10:13 AM, Simon Marchi wrote:
> On 2018-03-15 08:42 PM, Weimin Pan wrote:
>> The original problem was fixed (see related PR 22242). But using a typedef
>> as the declared type for a static member variable, as commented in this PR,
>> is still causing gdb to get into infinite loop when printing the static
>> member's value. This problem can be reproduced as follows:
>>
>> % cat t.cc
>> class A {
>>      typedef A type;
>> public:
>>      bool operator==(const type& other) { return true; }
>>
>>      static const type INSTANCE;
>> };
>>
>> const A A::INSTANCE;
>>
>> int main() {
>>      A a;
>>      if (a == A::INSTANCE) {
>>          return -1;
>>      }
>>      return 0;
>> }
>> % g++ -g t.cc
>> % gdb -ex "start" -ex "p a" a.out
>>
>> The fix is rather trivial - in cp_print_static_field(), should call
>> check_typedef() to get the static member's real type and use it to
>> check whether it's a struct or an array.
> Hi Weimin,
>
> Would it be possible to add a test case for this?  I suppose you can quite
> easily enhance the test case added by commit
>
>    a43f3893f6cb ("Fix broken recursion detection when printing static members")

Hi Simon,

Will see if I can add this testing to an existing test case or I need to 
create a new
test case for it. Maybe the latter is preferred?

Also I need to study/learn your "commit" command since I'm a new "git" user.

Thank you very much for your comment.

Weimin

>
>> Tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.
>> ---
>>   gdb/ChangeLog     |    7 +++++++
>>   gdb/cp-valprint.c |    2 +-
>>   2 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index d0a8dfd..6fd43de 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,10 @@
>> +2018-02-07  Weimin Pan  <weimin.pan@oracle.com>
>> +
>> +	PR gdb/16959
>> +	* cp-valprint.c: (cp_print_static_field) Use check_typedef() to get
>> +	static member's real type for TYPE_CODE_STRUCT and TYPE_CODE_ARRAY
>> +	comparisons.
>> +
>>   2018-01-24  Pedro Alves  <palves@redhat.com>
>>   
>>   	GCC PR libstdc++/83906
>> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
>> index 486653f..0370b56 100644
>> --- a/gdb/cp-valprint.c
>> +++ b/gdb/cp-valprint.c
>> @@ -633,6 +633,7 @@ cp_print_static_field (struct type *type,
>>         return;
>>       }
>>   
>> +  type = check_typedef (type);
>>     if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
>>       {
>>         CORE_ADDR *first_dont_print;
>> @@ -658,7 +659,6 @@ cp_print_static_field (struct type *type,
>>         addr = value_address (val);
>>         obstack_grow (&dont_print_statmem_obstack, (char *) &addr,
>>   		    sizeof (CORE_ADDR));
>> -      type = check_typedef (type);
>>         cp_print_value_fields (type, value_enclosing_type (val),
>>   			     value_embedded_offset (val), addr,
>>   			     stream, recurse, val,
>>
> type is passed below to val_print.  I think it would be better to continue
> passing the original type to that function instead of the resolved type.  It
> could affect how things are printed (if the type name is printed somewhere,
> or if pretty printers are involved).  Many functions use a variable "real_type"
> to hold the result from check_typedef, you could follow that pattern.
>
> Thanks,
>
> Simon
>

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

end of thread, other threads:[~2018-03-19 19:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-16  1:07 [PATCH PR gdb/16959] gdb hangs in infinite recursion Weimin Pan
2018-03-17 17:13 ` Simon Marchi
2018-03-19 19:46   ` Weimin Pan

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).