Fix PR c++/23373: GDB hangs when printing a struct with a static member of itself

Message ID 20180707212617.12964-1-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior July 7, 2018, 9:26 p.m. UTC
  This patch fixes a failure that happens when a structure has a static
member whose type is the same as itself.  From the bug report:

  Example code:
  struct A
  {
      static A Empty;
      int a;
  };

  int main(void) {
      A a;
      return 0;
  }

  Output:
  (gdb) ptype/o A
  /* offset    |  size */  type = struct A {
			     static struct A {
				 static struct A {
				     static struct A {
					 static struct A {
					     static struct A {
						 static struct A {
						     ... # infinite loop

The problem here is that GDB is not taking into account the fact that
static members inside a class/struct are not stored in the
class/struct, and therefore they should not be accounted for during
the display of the offsets/sizes.  The fix is simple: we just check if
the field we're dealing with (on
c-typeprint.c:c_type_print_base_struct_union) is static, and if it is
then we don't iterate over it.

This patch also adds a new test for this case, and doesn't introduce
any regressions.  I believe it is important enough to be included in
the 8.2 branch.

OK?

gdb/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR c++/23373
	* c-typeprint.c (c_type_print_base_struct_union): Don't print
	offsets/sizes for static members of a class/struct.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR c++/23373
	* gdb.base/ptype-offsets.cc (struct static_member): New
	struct.
	(main) <stmember>: New variable.
	* gdb.base/ptype-offsets.exp: Add test for printing a struct
	with a static member in it.
---
 gdb/c-typeprint.c                        |  2 +-
 gdb/testsuite/gdb.base/ptype-offsets.cc  |  8 ++++++++
 gdb/testsuite/gdb.base/ptype-offsets.exp | 11 +++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
  

Comments

Simon Marchi July 11, 2018, 2:43 a.m. UTC | #1
On 2018-07-07 05:26 PM, Sergio Durigan Junior wrote:
> This patch fixes a failure that happens when a structure has a static
> member whose type is the same as itself.  From the bug report:
> 
>   Example code:
>   struct A
>   {
>       static A Empty;
>       int a;
>   };
> 
>   int main(void) {
>       A a;
>       return 0;
>   }
> 
>   Output:
>   (gdb) ptype/o A
>   /* offset    |  size */  type = struct A {
> 			     static struct A {
> 				 static struct A {
> 				     static struct A {
> 					 static struct A {
> 					     static struct A {
> 						 static struct A {
> 						     ... # infinite loop
> 
> The problem here is that GDB is not taking into account the fact that
> static members inside a class/struct are not stored in the
> class/struct, and therefore they should not be accounted for during
> the display of the offsets/sizes.  The fix is simple: we just check if
> the field we're dealing with (on
> c-typeprint.c:c_type_print_base_struct_union) is static, and if it is
> then we don't iterate over it.
> 
> This patch also adds a new test for this case, and doesn't introduce
> any regressions.  I believe it is important enough to be included in
> the 8.2 branch.
> 
> OK?

Thanks, LGTM.  It would make sense to have it in 8.2 I believe.

Simon
  
Sergio Durigan Junior July 11, 2018, 6:56 p.m. UTC | #2
On Tuesday, July 10 2018, Simon Marchi wrote:

> On 2018-07-07 05:26 PM, Sergio Durigan Junior wrote:
>> This patch fixes a failure that happens when a structure has a static
>> member whose type is the same as itself.  From the bug report:
>> 
>>   Example code:
>>   struct A
>>   {
>>       static A Empty;
>>       int a;
>>   };
>> 
>>   int main(void) {
>>       A a;
>>       return 0;
>>   }
>> 
>>   Output:
>>   (gdb) ptype/o A
>>   /* offset    |  size */  type = struct A {
>> 			     static struct A {
>> 				 static struct A {
>> 				     static struct A {
>> 					 static struct A {
>> 					     static struct A {
>> 						 static struct A {
>> 						     ... # infinite loop
>> 
>> The problem here is that GDB is not taking into account the fact that
>> static members inside a class/struct are not stored in the
>> class/struct, and therefore they should not be accounted for during
>> the display of the offsets/sizes.  The fix is simple: we just check if
>> the field we're dealing with (on
>> c-typeprint.c:c_type_print_base_struct_union) is static, and if it is
>> then we don't iterate over it.
>> 
>> This patch also adds a new test for this case, and doesn't introduce
>> any regressions.  I believe it is important enough to be included in
>> the 8.2 branch.
>> 
>> OK?
>
> Thanks, LGTM.  It would make sense to have it in 8.2 I believe.

Thanks, pushed to both master and 8.2:

16ff70ddd430b63ca4dd68bf1d52828f2ed4f030 (master)
ad3c631a762734c39b85086d20ba28940c2fc9b8 (8.2)
  

Patch

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index c167e212de..eccb972f24 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -1168,7 +1168,7 @@  c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
 
 	  int newshow = show - 1;
 
-	  if (flags->print_offsets
+	  if (!is_static && flags->print_offsets
 	      && (TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_STRUCT
 		  || TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_UNION))
 	    {
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.cc b/gdb/testsuite/gdb.base/ptype-offsets.cc
index 7b01dbc5fb..e1aefe5bb3 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.cc
+++ b/gdb/testsuite/gdb.base/ptype-offsets.cc
@@ -177,6 +177,13 @@  struct asd
   void *f16;
 };
 
+/* See PR c++/23373.  */
+
+struct static_member
+{
+  static static_member Empty;
+  int abc;
+};
 
 int
 main (int argc, char *argv[])
@@ -188,6 +195,7 @@  main (int argc, char *argv[])
   struct tyu e;
   struct asd f;
   uint8_t i;
+  static_member stmember;
 
   return 0;
 }
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
index d8718d581b..ca0c5de7ee 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.exp
+++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
@@ -328,3 +328,14 @@  gdb_test_multiple "$test" "$test" {
        pass $test
    }
 }
+
+# Test that printing a struct with a static member of itself doesn't
+# get us into an infinite loop.
+gdb_test "ptype/o static_member" \
+    [multi_line \
+{/\* offset    |  size \*/  type = struct static_member \{} \
+{                           static static_member Empty;} \
+{\/*    0      |     4 \*/    int abc;} \
+{} \
+{                           /\* total size \(bytes\):    4 \*/} \
+{                         \}}]