[2,PR,gdb/16959] gdb hangs in infinite recursion

Message ID 1521740517-56879-1-git-send-email-weimin.pan@oracle.com
State New, archived
Headers

Commit Message

Weimin Pan March 22, 2018, 5:41 p.m. UTC
  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.

Added a new test case to the testsuite as Simon suggested.

Tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.
---
---
 gdb/ChangeLog                                 |    7 ++++
 gdb/cp-valprint.c                             |    2 +-
 gdb/testsuite/ChangeLog                       |    5 +++
 gdb/testsuite/gdb.cp/static-typedef-print.cc  |   35 +++++++++++++++++++++
 gdb/testsuite/gdb.cp/static-typedef-print.exp |   40 +++++++++++++++++++++++++
 5 files changed, 88 insertions(+), 1 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.cc
 create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.exp
  

Comments

Simon Marchi March 22, 2018, 8:27 p.m. UTC | #1
On 2018-03-22 01:41 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.
> 
> Added a new test case to the testsuite as Simon suggested.
> 
> Tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.
> ---
> ---
>  gdb/ChangeLog                                 |    7 ++++
>  gdb/cp-valprint.c                             |    2 +-
>  gdb/testsuite/ChangeLog                       |    5 +++
>  gdb/testsuite/gdb.cp/static-typedef-print.cc  |   35 +++++++++++++++++++++
>  gdb/testsuite/gdb.cp/static-typedef-print.exp |   40 +++++++++++++++++++++++++
>  5 files changed, 88 insertions(+), 1 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.cc
>  create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.exp
> 
> 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,

I pointed this out in my previous mail:

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.

Did you have a chance to take a look?

Simon
  
Weimin Pan March 22, 2018, 8:47 p.m. UTC | #2
On 3/22/2018 1:27 PM, Simon Marchi wrote:
> On 2018-03-22 01:41 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.
>>
>> Added a new test case to the testsuite as Simon suggested.
>>
>> Tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.
>> ---
>> ---
>>   gdb/ChangeLog                                 |    7 ++++
>>   gdb/cp-valprint.c                             |    2 +-
>>   gdb/testsuite/ChangeLog                       |    5 +++
>>   gdb/testsuite/gdb.cp/static-typedef-print.cc  |   35 +++++++++++++++++++++
>>   gdb/testsuite/gdb.cp/static-typedef-print.exp |   40 +++++++++++++++++++++++++
>>   5 files changed, 88 insertions(+), 1 deletions(-)
>>   create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.cc
>>   create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.exp
>>
>> 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,
> I pointed this out in my previous mail:
>
> 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.
>
> Did you have a chance to take a look?
>
> Simon

Sorry, I missed it. Will take a look.

Thanks,
Weimin
  

Patch

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,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0f02f4a..6849d5a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-03-20  Weimin Pan  <weimin.pan@oracle.com>
+
+	* gdb.cp/static-typedef-print.exp: New file.
+	* gdb.cp/static-typedef-print.cc: New file.
+
 2018-01-22  Pedro Alves  <palves@redhat.com>
 	    Sergio Durigan Junior  <sergiodj@redhat.com>
 
diff --git a/gdb/testsuite/gdb.cp/static-typedef-print.cc b/gdb/testsuite/gdb.cp/static-typedef-print.cc
new file mode 100644
index 0000000..d698d6f
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/static-typedef-print.cc
@@ -0,0 +1,35 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program 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 of the License, or
+   (at your option) any later version.
+
+   This program 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 program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+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;
+}
+
diff --git a/gdb/testsuite/gdb.cp/static-typedef-print.exp b/gdb/testsuite/gdb.cp/static-typedef-print.exp
new file mode 100644
index 0000000..e0da0c9
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/static-typedef-print.exp
@@ -0,0 +1,40 @@ 
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program 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 of the License, or
+# (at your option) any later version.
+#
+# This program 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 program.  If not, see <http://www.gnu.org/licenses/>.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if [get_compiler_info "c++"] {
+    return -1
+}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+clean_restart $testfile
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_test "print a" \
+         "static INSTANCE = <same as static member of an already seen type>}}.*" \
+         "print static member"
+
+gdb_exit
+return 0