public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix PR debug/45088
@ 2010-11-12 12:04 Dodji Seketeli
  2010-11-25  5:57 ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Dodji Seketeli @ 2010-11-12 12:04 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

Hello,

In the first example of the patch below the DW_AT_pointer_type
DIE describing the type of the C::ai member lacks the DW_AT_type
attribute, making C::ai look like if it had void* type in a debugger.

This is because the type of C::ai is the self-reference type of
C. When gen_type_die_with_usage sees that self-reference it recognizes
it as a typedef, tries to emit a DIE for that typedef but it's
lieutenant gen_decl_die refuses to do so because the typedef is
artificial. So it looks like the dwarf emitter is sometimes faced with
artificial typedefs it must drop on the floor, and sometimes with
artificial typedefs for which it has to emit the underlying type.

One possible way to do resolve that dilemma is [at the FE level] to
fixup the type of *ai to make it be A instead of the self-reference
type of A. I figure the fact that the self-reference type is different
from A itself should be kept as an implementation detail of the FE
anyway.

The second hunk of the patch below hopefully does the fixup during the
parsing of the simple-type-specifier production for cases where the
type is neither a template nor a template instantiation. This is
because the self-reference information is useful for those template
related classes to resolve some ambiguities later e.g, with
maybe_get_template_decl_from_type_decl.

I noticed we were trying to do a similar fixup in
check_elaborated_type_specifier but were failing in doing so, maybe
because the representation of self-reference types has changed since
then. The first hunk addresses that.

Bootstrapped and tested on x86-64-unknown-linux-gnu against trunk.

-- 
	Dodji

From e4e8f472bbabae7ed2a23906cc14b90fe5d699f8 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@redhat.com>
Date: Thu, 11 Nov 2010 23:42:51 +0100
Subject: [PATCH] Fix PR debug/45088

gcc/cp/
	* parser.c (cp_parser_simple_type_specifier): Ensure
	self-reference type doesn't get through all the way to the
	debug info emitter.
	* decl.c (check_elaborated_type_specifier): Likewise.

gcc/testsuite/
	* g++.dg/debug/dwarf2/self-ref-1.C: New test.
	* g++.dg/debug/dwarf2/self-ref-2.C: Likewise.
---
 gcc/cp/decl.c                                  |    2 +-
 gcc/cp/parser.c                                |   17 ++++++++++++++
 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C |   28 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C |   29 ++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index fb5ca7f..feba130 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -10902,7 +10902,7 @@ check_elaborated_type_specifier (enum tag_types tag_code,
      name lookup will find the TYPE_DECL for the implicit "S::S"
      typedef.  Adjust for that here.  */
   if (DECL_SELF_REFERENCE_P (decl))
-    decl = TYPE_NAME (TREE_TYPE (decl));
+    decl = TYPE_NAME (DECL_ORIGINAL_TYPE (decl));
 
   type = TREE_TYPE (decl);
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6a9e4d7..c3ed5b2 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12791,6 +12791,23 @@ cp_parser_simple_type_specifier (cp_parser* parser,
       /* Otherwise, look for a type-name.  */
       else
 	type = cp_parser_type_name (parser);
+
+      /* Self reference typedefs should not be kept because it
+	 confuses the debug info emitter that wouldn't know how to
+	 tell the difference between an artificial typedef to drop
+	 and an artificial self reference typedef to keep.
+	 So let's replace the self reference by the type by its
+	 underlying type.  */
+      if (type && TREE_CODE (type) == TYPE_DECL
+	  && DECL_SELF_REFERENCE_P (type)
+	  /* If TYPE is a decl of a template or a template
+	     instantiation we need to keep it because the information
+	     is used later by e.g,
+	     maybe_get_template_decl_from_type_decl.  */
+	  && !CLASSTYPE_IS_TEMPLATE (TREE_TYPE (type))
+	  && !CLASSTYPE_TEMPLATE_INFO (TREE_TYPE (type)))
+	type = TYPE_NAME (DECL_ORIGINAL_TYPE (type));
+
       /* Keep track of all name-lookups performed in class scopes.  */
       if (type
 	  && !global_p
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
new file mode 100644
index 0000000..ee8342b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
@@ -0,0 +1,28 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 3 } }
+
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
new file mode 100644
index 0000000..b1c5401
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
@@ -0,0 +1,29 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+template<class T>
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A<int>
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A<int>* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+
-- 
1.7.2.3

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

* Re: Fix PR debug/45088
  2010-11-12 12:04 Fix PR debug/45088 Dodji Seketeli
@ 2010-11-25  5:57 ` Jason Merrill
  2010-11-30 17:53   ` Dodji Seketeli
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2010-11-25  5:57 UTC (permalink / raw)
  To: Dodji Seketeli; +Cc: GCC Patches

On 11/12/2010 06:47 AM, Dodji Seketeli wrote:
> One possible way to do resolve that dilemma is [at the FE level] to
> fixup the type of *ai to make it be A instead of the self-reference
> type of A. I figure the fact that the self-reference type is different
> from A itself should be kept as an implementation detail of the FE
> anyway.

I think the typedef should be emitted so that name lookup in the 
debugger can find it.  TYPE_DECL_IS_STUB should not be true for the 
injected-class-name.

Jason

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

* Re: Fix PR debug/45088
  2010-11-25  5:57 ` Jason Merrill
@ 2010-11-30 17:53   ` Dodji Seketeli
  2010-12-02 18:55     ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Dodji Seketeli @ 2010-11-30 17:53 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

Jason Merrill <jason@redhat.com> writes:

> On 11/12/2010 06:47 AM, Dodji Seketeli wrote:
>> One possible way to do resolve that dilemma is [at the FE level] to
>> fixup the type of *ai to make it be A instead of the self-reference
>> type of A. I figure the fact that the self-reference type is different
>> from A itself should be kept as an implementation detail of the FE
>> anyway.
>
> I think the typedef should be emitted so that name lookup in the
> debugger can find it.  TYPE_DECL_IS_STUB should not be true for the
> injected-class-name.

Yes this makes a lot of sense. I initially got somewhat confused by this
code snippet in is_redundant_typedef:

  if (DECL_ARTIFICIAL (decl)
      && DECL_CONTEXT (decl)
      && is_tagged_type (DECL_CONTEXT (decl))
      && TREE_CODE (TYPE_NAME (DECL_CONTEXT (decl))) == TYPE_DECL
      && DECL_NAME (decl) == DECL_NAME (TYPE_NAME (DECL_CONTEXT (decl))))
    /* Also ignore the artificial member typedef for the class name.  */
    return 1;

From what I understand it basically is tailored to not emit debug info
precisely for injected-class-names.

But maybe at that time we didn't have the help of cgraph and the unused
type debug info pruning we have today. In other words I think emitting
debug info for the injected-class-name won't necessarily increase bloat
because if the debug info of the injected-class-name typedef will be
pruned if it is not used.

So would the patch below be more acceptable? 

I have tested it against trunk on x86_64-unknown-linux-gnu.

-- 
	Dodji

From: Dodji Seketeli <dodji@seketeli.org>
Date: Thu, 11 Nov 2010 23:42:51 +0100
Subject: [PATCH] Fix PR debug/45088

gcc/
	* dwarf2out.c (is_redundant_typedef): Do not consider
	injected-class-names as being redundant typedefs anymore.

gcc/testsuite/
	* g++.dg/debug/dwarf2/self-ref-1.C: New test.
	* g++.dg/debug/dwarf2/self-ref-2.C: Likewise.
---
 gcc/dwarf2out.c                                |   13 +++++-----
 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C |   28 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C |   29 ++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 04764ba..a8fe4b5 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -20566,13 +20566,12 @@ is_redundant_typedef (const_tree decl)
   if (TYPE_DECL_IS_STUB (decl))
     return 1;
 
-  if (DECL_ARTIFICIAL (decl)
-      && DECL_CONTEXT (decl)
-      && is_tagged_type (DECL_CONTEXT (decl))
-      && TREE_CODE (TYPE_NAME (DECL_CONTEXT (decl))) == TYPE_DECL
-      && DECL_NAME (decl) == DECL_NAME (TYPE_NAME (DECL_CONTEXT (decl))))
-    /* Also ignore the artificial member typedef for the class name.  */
-    return 1;
+  /* For C++ We consider injected-class-names (which are typedefs)
+     like a non-redundant typedef. They are needed so that lookups of
+     a class name from inside the scope of the class to succeed in the
+     debugger.  This should hopefully not result in debug size
+     explosion as we have infrastructure in place to prune debug info
+     of unused types anyway.  */
 
   return 0;
 }
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
new file mode 100644
index 0000000..81bcb27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
@@ -0,0 +1,28 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
new file mode 100644
index 0000000..b1c5401
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
@@ -0,0 +1,29 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+template<class T>
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A<int>
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A<int>* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+
-- 
1.7.2.3

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

* Re: Fix PR debug/45088
  2010-11-30 17:53   ` Dodji Seketeli
@ 2010-12-02 18:55     ` Jason Merrill
  2010-12-12 15:42       ` Dodji Seketeli
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2010-12-02 18:55 UTC (permalink / raw)
  To: Dodji Seketeli; +Cc: GCC Patches

On 11/30/2010 11:06 AM, Dodji Seketeli wrote:
> Jason Merrill<jason@redhat.com>  writes:
>
>> On 11/12/2010 06:47 AM, Dodji Seketeli wrote:
>>> One possible way to do resolve that dilemma is [at the FE level] to
>>> fixup the type of *ai to make it be A instead of the self-reference
>>> type of A. I figure the fact that the self-reference type is different
>>> from A itself should be kept as an implementation detail of the FE
>>> anyway.
>>
>> I think the typedef should be emitted so that name lookup in the
>> debugger can find it.  TYPE_DECL_IS_STUB should not be true for the
>> injected-class-name.
>
> Yes this makes a lot of sense. I initially got somewhat confused by this
> code snippet in is_redundant_typedef:
>
>    if (DECL_ARTIFICIAL (decl)
>        &&  DECL_CONTEXT (decl)
>        &&  is_tagged_type (DECL_CONTEXT (decl))
>        &&  TREE_CODE (TYPE_NAME (DECL_CONTEXT (decl))) == TYPE_DECL
>        &&  DECL_NAME (decl) == DECL_NAME (TYPE_NAME (DECL_CONTEXT (decl))))
>      /* Also ignore the artificial member typedef for the class name.  */
>      return 1;
>
>  From what I understand it basically is tailored to not emit debug info
> precisely for injected-class-names.

Yep.

> But maybe at that time we didn't have the help of cgraph and the unused
> type debug info pruning we have today. In other words I think emitting
> debug info for the injected-class-name won't necessarily increase bloat
> because if the debug info of the injected-class-name typedef will be
> pruned if it is not used.

No, I don't think we can prune those.  Perhaps bloat is a reason not to 
go this way.

What about just having gen_type_die_with_usage check is_redundant_typedef?

Jason

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

* Re: Fix PR debug/45088
  2010-12-02 18:55     ` Jason Merrill
@ 2010-12-12 15:42       ` Dodji Seketeli
  2010-12-13 14:27         ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Dodji Seketeli @ 2010-12-12 15:42 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

Jason Merrill <jason@redhat.com> writes:

[...]

> What about just having gen_type_die_with_usage check is_redundant_typedef?

Like the patch below?

Bootrapped and tested on x86_64-unknown-linux-gnu against trunk.

commit d9f6a81775a92638d2c50d8478b261dd71ee2442
Author: Dodji Seketeli <dodji@seketeli.org>
Date:   Thu Nov 11 23:42:51 2010 +0100

    Fix PR debug/45088
    
    gcc/
    	* dwarf2out.c (gen_type_die_with_usage): Don't try to generate a
    	typedef DIE for an injected-class-name. Consider it as a class
    	name instead.
    
    gcc/testsuite/
    
    	* g++.dg/debug/dwarf2/self-ref-1.C: New test.
    	* g++.dg/debug/dwarf2/self-ref-2.C: Likewise.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index c985527..38e96a8 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -20260,7 +20260,8 @@ gen_type_die_with_usage (tree type, dw_die_ref context_die,
 
   /* If TYPE is a typedef type variant, let's generate debug info
      for the parent typedef which TYPE is a type of.  */
-  if (typedef_variant_p (type))
+  if (typedef_variant_p (type)
+      && !is_redundant_typedef (TYPE_NAME (type)))
     {
       if (TREE_ASM_WRITTEN (type))
 	return;
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
new file mode 100644
index 0000000..81bcb27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
@@ -0,0 +1,28 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
new file mode 100644
index 0000000..b1c5401
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
@@ -0,0 +1,29 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+template<class T>
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A<int>
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A<int>* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+

-- 
	Dodji

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

* Re: Fix PR debug/45088
  2010-12-12 15:42       ` Dodji Seketeli
@ 2010-12-13 14:27         ` Jason Merrill
  2010-12-14 19:13           ` Dodji Seketeli
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2010-12-13 14:27 UTC (permalink / raw)
  To: Dodji Seketeli; +Cc: GCC Patches

On 12/12/2010 10:04 AM, Dodji Seketeli wrote:
>     /* If TYPE is a typedef type variant, let's generate debug info
>        for the parent typedef which TYPE is a type of.  */
> -  if (typedef_variant_p (type))
> +  if (typedef_variant_p (type)
> +      &&  !is_redundant_typedef (TYPE_NAME (type)))

I would expect this to cause problems when things use both the main type 
and the injected variant, since the two types have different 
TREE_ASM_WRITTEN.  Rather, if type is a redundant typedef, we should 
strip it to get the underlying type.

Jason

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

* Re: Fix PR debug/45088
  2010-12-13 14:27         ` Jason Merrill
@ 2010-12-14 19:13           ` Dodji Seketeli
  2010-12-15  3:52             ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Dodji Seketeli @ 2010-12-14 19:13 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

Jason Merrill <jason@redhat.com> writes:

> On 12/12/2010 10:04 AM, Dodji Seketeli wrote:
>>     /* If TYPE is a typedef type variant, let's generate debug info
>>        for the parent typedef which TYPE is a type of.  */
>> -  if (typedef_variant_p (type))
>> +  if (typedef_variant_p (type)
>> +      &&  !is_redundant_typedef (TYPE_NAME (type)))
>
> I would expect this to cause problems when things use both the main
> type and the injected variant, since the two types have different
> TREE_ASM_WRITTEN.

I see.

>  Rather, if type is a redundant typedef, we should
> strip it to get the underlying type.

Okay, the patch below hopefully does that.

Boostrapped and tested on x86_64-unknown-linux-gnu.

-- 
	Dodji

commit 2befd0376755e820b60bf6654689b59a7b9e22cd
Author: Dodji Seketeli <dodji@seketeli.org>
Date:   Thu Nov 11 23:42:51 2010 +0100

    Fix PR debug/45088
    
    gcc/
    	* dwarf2out.c (is_injected_class_name): New function.
    	(gen_type_die_with_usage): Use it. Don't try to generate a typedef
    	DIE for an injected-class-name. Consider its underlying class name
    	instead.
    
    gcc/testsuite/
    
    	* g++.dg/debug/dwarf2/self-ref-1.C: New test.
    	* g++.dg/debug/dwarf2/self-ref-2.C: Likewise.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index c985527..3c9eb95 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -6454,6 +6454,7 @@ static void gen_typedef_die (tree, dw_die_ref);
 static void gen_type_die (tree, dw_die_ref);
 static void gen_block_die (tree, dw_die_ref, int);
 static void decls_for_scope (tree, dw_die_ref, int);
+static bool is_injected_class_name (const_tree decl);
 static int is_redundant_typedef (const_tree);
 static bool is_naming_typedef_decl (const_tree);
 static inline dw_die_ref get_context_die (tree);
@@ -20251,13 +20252,21 @@ gen_tagged_type_die (tree type,
 
 static void
 gen_type_die_with_usage (tree type, dw_die_ref context_die,
-				enum debug_info_usage usage)
+			 enum debug_info_usage usage)
 {
   struct array_descr_info info;
 
   if (type == NULL_TREE || type == error_mark_node)
     return;
 
+  /* The C++ FE uses a special typedef variant to represent the
+     injected-class-name of a type. Letting this through confuses us
+     because we'd consider this typedef as bloat and won't generate
+     debug info for it. What we really want is the underlying type of
+     the injected-class-name.  */
+  if (is_injected_class_name (TYPE_NAME (type)))
+    type = DECL_ORIGINAL_TYPE (TYPE_NAME (type));
+
   /* If TYPE is a typedef type variant, let's generate debug info
      for the parent typedef which TYPE is a type of.  */
   if (typedef_variant_p (type))
@@ -20567,6 +20576,22 @@ decls_for_scope (tree stmt, dw_die_ref context_die, int depth)
     gen_block_die (subblocks, context_die, depth + 1);
 }
 
+/* Return TRUE if DECL is a C++ injected-class-name.  */
+
+static inline bool
+is_injected_class_name (const_tree decl)
+{
+  if (is_typedef_decl (CONST_CAST_TREE (decl))
+      && !TYPE_DECL_IS_STUB (decl)
+      && DECL_ARTIFICIAL (decl)
+      && DECL_CONTEXT (decl)
+      && is_tagged_type (DECL_CONTEXT (decl))
+      && TREE_CODE (TYPE_NAME (DECL_CONTEXT (decl))) == TYPE_DECL
+      && DECL_NAME (decl) == DECL_NAME (TYPE_NAME (DECL_CONTEXT (decl))))
+    return 1;
+  return 0;
+}
+
 /* Is this a typedef we can avoid emitting?  */
 
 static inline int
@@ -20575,12 +20600,7 @@ is_redundant_typedef (const_tree decl)
   if (TYPE_DECL_IS_STUB (decl))
     return 1;
 
-  if (DECL_ARTIFICIAL (decl)
-      && DECL_CONTEXT (decl)
-      && is_tagged_type (DECL_CONTEXT (decl))
-      && TREE_CODE (TYPE_NAME (DECL_CONTEXT (decl))) == TYPE_DECL
-      && DECL_NAME (decl) == DECL_NAME (TYPE_NAME (DECL_CONTEXT (decl))))
-    /* Also ignore the artificial member typedef for the class name.  */
+  if (is_injected_class_name (decl))
     return 1;
 
   return 0;
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
new file mode 100644
index 0000000..81bcb27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
@@ -0,0 +1,28 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
new file mode 100644
index 0000000..b1c5401
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
@@ -0,0 +1,29 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+template<class T>
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A<int>
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A<int>* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+

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

* Re: Fix PR debug/45088
  2010-12-14 19:13           ` Dodji Seketeli
@ 2010-12-15  3:52             ` Jason Merrill
  2010-12-16 17:32               ` Dodji Seketeli
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2010-12-15  3:52 UTC (permalink / raw)
  To: Dodji Seketeli; +Cc: GCC Patches

On 12/14/2010 01:26 PM, Dodji Seketeli wrote:
> +  if (is_injected_class_name (TYPE_NAME (type)))
> +    type = DECL_ORIGINAL_TYPE (TYPE_NAME (type));

Yes, that should do the trick, but I might still tweak it a bit.

The problematic case is when is_redundant_typedef (decl) && 
DECL_ORIGINAL_TYPE (decl) are both true; gen_decl_die assumes that these 
two conditions can't occur together, which is why things break.  This 
change will make that assumption valid again, but I'd feel better about 
using that condition directly (that is, if (is_redundant_typedef 
(TYPE_NAME (type)) && DECL_ORIGINAL_TYPE (TYPE_NAME (type))) rather than 
assuming that the injected-class-name is the only redundant typedef for 
which this can happen.

I think we should also add an explicit assert to gen_decl_die in the 
is_redundant_typedef case to make sure that DECL_ORIGINAL_TYPE is NULL_TREE.

Jason

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

* Re: Fix PR debug/45088
  2010-12-15  3:52             ` Jason Merrill
@ 2010-12-16 17:32               ` Dodji Seketeli
  2010-12-16 21:17                 ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Dodji Seketeli @ 2010-12-16 17:32 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

Jason Merrill <jason@redhat.com> writes:

> On 12/14/2010 01:26 PM, Dodji Seketeli wrote:
>> +  if (is_injected_class_name (TYPE_NAME (type)))
>> +    type = DECL_ORIGINAL_TYPE (TYPE_NAME (type));
>
> Yes, that should do the trick, but I might still tweak it a bit.
>
> The problematic case is when is_redundant_typedef (decl) &&
> DECL_ORIGINAL_TYPE (decl) are both true; gen_decl_die assumes that
> these two conditions can't occur together, which is why things break.
> This change will make that assumption valid again, but I'd feel better
> about using that condition directly (that is, if (is_redundant_typedef
> (TYPE_NAME (type)) && DECL_ORIGINAL_TYPE (TYPE_NAME (type))) rather
> than assuming that the injected-class-name is the only redundant
> typedef for which this can happen.

OK.

> I think we should also add an explicit assert to gen_decl_die in the
> is_redundant_typedef case to make sure that DECL_ORIGINAL_TYPE is
> NULL_TREE.

That assert would be violated because gen_decl_die can be called e.g
from gen_member_die with the injected-class-name typedef in argument as
it is a member of the class. In that case, gen_decl_die sees the
injected-class-name, considers it as redundant typedef, tries to
generate debug info for its typedef variant type and at that point
gen_type_die_with_usage handles it correctly.

I have made and tested the first change you suggested about though.

-- 
	Dodji

commit 10dbc390376bc6f37fa7d3b85bd160e717ad4a0b
Author: Dodji Seketeli <dodji@redhat.com>
Date:   Thu Dec 16 12:31:56 2010 +0100

    Fix for PR debug/45088
    
    gcc/
    
    	* dwarf2out.c (gen_type_die_with_usage): Do not try to emit debug
    	info for a redundant typedef that has DECL_ORIGINAL_TYPE set. Use
    	that underlying type instead.
    
    gcc/testsuite/
    
    	* g++.dg/debug/dwarf2/self-ref-1.C: New test.
    	* g++.dg/debug/dwarf2/self-ref-2.C: Likewise.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index c985527..1fa3300 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -20251,13 +20251,23 @@ gen_tagged_type_die (tree type,
 
 static void
 gen_type_die_with_usage (tree type, dw_die_ref context_die,
-				enum debug_info_usage usage)
+			 enum debug_info_usage usage)
 {
   struct array_descr_info info;
 
   if (type == NULL_TREE || type == error_mark_node)
     return;
 
+  if (TYPE_NAME (type) != NULL_TREE
+      && TREE_CODE (TYPE_NAME (type)) == TYPE_DECL
+      && is_redundant_typedef (TYPE_NAME (type))
+      && DECL_ORIGINAL_TYPE (TYPE_NAME (type)))
+    /* The DECL of this type is a typedef we don't want to emit debug
+       info for but we want debug info for its underlying typedef.
+       This can happen for e.g, the injected-class-name of a C++
+       type.  */
+    type = DECL_ORIGINAL_TYPE (TYPE_NAME (type));
+
   /* If TYPE is a typedef type variant, let's generate debug info
      for the parent typedef which TYPE is a type of.  */
   if (typedef_variant_p (type))
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
new file mode 100644
index 0000000..81bcb27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-1.C
@@ -0,0 +1,28 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
new file mode 100644
index 0000000..b1c5401
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/self-ref-2.C
@@ -0,0 +1,29 @@
+// Origin: PR debug/45088
+// { dg-do compile }
+// { dg-options "-g -dA" }
+// { dg-final { scan-assembler-times "\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_pointer_type\\)\[\n\r\]{1,2}\[^\n\r\]*DW_AT_byte_size\[\n\r\]{1,2}\[^\n\r\]*DW_AT_type" 4 } }
+
+template<class T>
+struct A
+{
+    virtual ~A();
+};
+
+struct B : public A<int>
+{
+    virtual ~B(){}
+};
+
+struct C : public B
+{
+    A<int>* a1;
+};
+
+int
+main()
+{
+    C c;
+    c.a1 = 0;
+    return 0;
+}
+

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

* Re: Fix PR debug/45088
  2010-12-16 17:32               ` Dodji Seketeli
@ 2010-12-16 21:17                 ` Jason Merrill
  0 siblings, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2010-12-16 21:17 UTC (permalink / raw)
  To: Dodji Seketeli; +Cc: GCC Patches

OK.

Jason

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

end of thread, other threads:[~2010-12-16 20:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-12 12:04 Fix PR debug/45088 Dodji Seketeli
2010-11-25  5:57 ` Jason Merrill
2010-11-30 17:53   ` Dodji Seketeli
2010-12-02 18:55     ` Jason Merrill
2010-12-12 15:42       ` Dodji Seketeli
2010-12-13 14:27         ` Jason Merrill
2010-12-14 19:13           ` Dodji Seketeli
2010-12-15  3:52             ` Jason Merrill
2010-12-16 17:32               ` Dodji Seketeli
2010-12-16 21:17                 ` Jason Merrill

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