Patchwork Fix PR debug/45088

login
register
mail settings
Submitter Dodji Seketeli
Date Nov. 30, 2010, 4:06 p.m.
Message ID <m3ipzeyewd.fsf@redhat.com>
Download mbox | patch
Permalink /patch/73620/
State New
Headers show

Comments

Dodji Seketeli - Nov. 30, 2010, 4:06 p.m.
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.
Jason Merrill - Dec. 2, 2010, 6:55 p.m.
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

Patch

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;
+}
+