Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-23 Thread Paul_Koning

On Oct 5, 2012, at 6:05 PM, Cary Coutant wrote:

 There certainly is a fair amount of code in dwarf2read.c in gdb to handle 
 DW_AT_declaration and do things differently for declarations.
 
 Should I rework this patch to use that mechanism instead?  If so, how?  If 
 the class is marked only by prune_unused_types_mark visiting it as a parent, 
 but hasn't been marked by ??? that visits all its children, then emit it 
 with a DW_AT_declaration marking?
 
 One question I'd consider is what do you want to see in the debugger
 if this truly is the only debug info you have for the class? (For
 example, in the test case you added, a DW_AT_declaration attribute
 won't help if there's no full definition of the class anywhere else.)
 Is it reasonable to just show a truncated class definition in that
 case, or do you want the full definition available. My tentative
 answer would be that we do the pruning here because we expect there to
 be a full definition somewhere else, and that the lack of a
 DW_AT_declaration attribute is the bug.

The answer appears to be:
1. Until the full symbols have been read (via gdb -r, or by reference to 
another symbol in that compilation unit) such declarations are not visible.  
Once the full symbols have been read, a class marked as DW_AT_declaration is 
shown as imcomplete type which makes sense.

I think this is reasonable behavior.  I confirmed that if you do have a 
definition elsewhere, gdb does the correct thing.  That's what you would 
expect, given that the DW_AT_declaration flag was already being generated (for 
imcomplete types).
 
 As you've discovered, however, it's not straightforward. You'll want
 to add the declaration attribute if you mark the DIE from below, but
 not from any reference where dokids is true. Alternatively, add the
 declaration attribute if any of its children are pruned. Perhaps that
 could be done in prune_unused_types_prune().
 
 If you're willing to rework the patch this way (assuming GDB does the
 right thing with it), I think that would be better. Thanks.
 
 -cary

Attached is the revised patch.  It marks classes as declaration if they 
weren't marked by one of the mark functions that visits children, and something 
was actually pruned.  That second check is needed, otherwise the class 
Executor in testcase nested-3.C gets marked as a declaration.

The testcase has been reworked to check both aspects.  Struct s gets defined 
(because a variable of that type is defined), while class c and union u are 
not, so they are marked as declaration while struct s is not marked.  The 
testcase verifies that.

Tested by build and check RUNTESTFLAGS=dwarf2.exp on Linux and Darwin.  Ok to 
commit?

paul

ChangeLog:

2012-10-23  Paul Koning  n...@arrl.net

* dwarf2out.c (prune_unused_types_prune): If pruning a class and
not all its children were marked, add DW_AT_declaration flag.

testcases/ChangeLog:

2012-10-23  Paul Koning  n...@arrl.net

* g++.dg/debug/dwarf2/pr54508.C: New.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 192405)
+++ gcc/dwarf2out.c (working copy)
@@ -21218,6 +21218,7 @@
 prune_unused_types_prune (dw_die_ref die)
 {
   dw_die_ref c;
+  int pruned = 0;
 
   gcc_assert (die-die_mark);
   prune_unused_types_update_strings (die);
@@ -21240,13 +21241,24 @@
  prev-die_sib = c-die_sib;
  die-die_child = prev;
}
- return;
+ pruned = 1;
+ goto finished;
}
 
 if (c != prev-die_sib)
-  prev-die_sib = c;
+  {
+   prev-die_sib = c;
+   pruned = 1;
+  }
 prune_unused_types_prune (c);
   } while (c != die-die_child);
+
+ finished:
+  /* If we pruned children, and this is a class, mark it as a 
+ declaration to inform debuggers that this is not a complete
+ class definition.  */
+  if (pruned  die-die_mark == 1  class_scope_p (die))
+add_AT_flag (die, DW_AT_declaration, 1);
 }
 
 /* Remove dies representing declarations that we never use.  */

Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,72 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options -g2 -dA -fno-merge-debug-strings }
+
+// { dg-final { scan-assembler-not \cbase0\\[ \t\]+\[#;/!|@\]+ 
DW_AT_name } }
+// { dg-final { scan-assembler \c0\\[ \t\]+\[#;/!|@\]+ 
DW_AT_name\[\r\n\]+\[\^\r\n\]+\[\r\n\]+\[\^\r\n\]+\[\r\n\]+\[\^#;/!|@\]+\[#;/!|@\]+
 DW_AT_decl_line\[\r\n\]+\[\^#;/!|@\]+\[#;/!|@\]+ DW_AT_declaration } }
+// { dg-final { scan-assembler-not \OPCODE0\\[ \t\]+\[#;/!|@\]+ 
DW_AT_name } }
+// { dg-final { scan-assembler-not \bi0\\[ \t\]+\[#;/!|@\]+ DW_AT_name 
} }
+// { dg-final { scan-assembler-not \si0\\[ \t\]+\[#;/!|@\]+ DW_AT_name 
} }
+// { 

Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-23 Thread Jason Merrill

OK.

Jason


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-23 Thread Paul_Koning

On Oct 23, 2012, at 2:02 PM, Jason Merrill wrote:

 OK.
 
 Jason

Thanks.  Committed.

paul



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Jakub Jelinek
On Thu, Oct 04, 2012 at 05:26:11PM -0700, Cary Coutant wrote:
  Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
  ===
  --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
  +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
  @@ -59,11 +59,11 @@
   // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
   // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
   // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
  -// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
  -// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
  +// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
  +// { dg-final { scan-assembler staticfn4\[^\n\r\]*DW_AT_name } }
   // { dg-final { scan-assembler-not staticfn5\[^\n\r\]*DW_AT_name } }
   // { dg-final { scan-assembler-not staticfn6\[^\n\r\]*DW_AT_name } }
  -// { dg-final { scan-assembler-not method1\[^\n\r\]*DW_AT_name } }
  +// { dg-final { scan-assembler method1\[^\n\r\]*DW_AT_name } }
   // { dg-final { scan-assembler arg1\[^\n\r\]*DW_AT_name } }
   // { dg-final { scan-assembler arg2\[^\n\r\]*DW_AT_name } }
   // { dg-final { scan-assembler arg3\[^\n\r\]*DW_AT_name } }
 
 The fact that these two tests were specifically checking for the
 absence of staticfn3 and staticfn4 leads me to believe that the
 current behavior is deliberate. Jakub, that change was yours (it dates
 back to November 2008). Are you OK with Paul's change?

Yes, thought it would be interesting to get some .debug_info size growth
numbers for a few projects (say libstdc++.so and some larger C++ codebase
(some KDE core library, or OO.o) without/with the patch, to see how much
does it bring with it (I'm not that much worried about the DW_TAG_subprogram
added itself, but about about types it will additionally bring in).
We have dwz and likely would get most of the growth back due to redundancy
removal though.

Jakub


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Paul_Koning

On Oct 5, 2012, at 4:16 AM, Jakub Jelinek wrote:

 On Thu, Oct 04, 2012 at 05:26:11PM -0700, Cary Coutant wrote:
 Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
 ===
 --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
 +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
 @@ -59,11 +59,11 @@
 // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn4\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn5\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn6\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not method1\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler method1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg2\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg3\[^\n\r\]*DW_AT_name } }
 
 The fact that these two tests were specifically checking for the
 absence of staticfn3 and staticfn4 leads me to believe that the
 current behavior is deliberate. Jakub, that change was yours (it dates
 back to November 2008). Are you OK with Paul's change?
 
 Yes, thought it would be interesting to get some .debug_info size growth
 numbers for a few projects (say libstdc++.so and some larger C++ codebase
 (some KDE core library, or OO.o) without/with the patch, to see how much
 does it bring with it (I'm not that much worried about the DW_TAG_subprogram
 added itself, but about about types it will additionally bring in).
 We have dwz and likely would get most of the growth back due to redundancy
 removal though.
 
   Jakub

I did a quick test on a large application of ours where this issue was causing 
trouble.  Without the patch, it generates 115 MB of debug data; with the patch, 
that grows to 120 MB.

paul



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Paul_Koning

On Oct 5, 2012, at 11:34 AM, paul_kon...@dell.com
 paul_kon...@dell.com wrote:

 
 On Oct 5, 2012, at 4:16 AM, Jakub Jelinek wrote:
 
 On Thu, Oct 04, 2012 at 05:26:11PM -0700, Cary Coutant wrote:
 Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
 ===
 --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
 +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
 @@ -59,11 +59,11 @@
 // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn4\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn5\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn6\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not method1\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler method1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg2\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg3\[^\n\r\]*DW_AT_name } }
 
 The fact that these two tests were specifically checking for the
 absence of staticfn3 and staticfn4 leads me to believe that the
 current behavior is deliberate. Jakub, that change was yours (it dates
 back to November 2008). Are you OK with Paul's change?
 
 Yes, thought it would be interesting to get some .debug_info size growth
 numbers for a few projects (say libstdc++.so and some larger C++ codebase
 (some KDE core library, or OO.o) without/with the patch, to see how much
 does it bring with it (I'm not that much worried about the DW_TAG_subprogram
 added itself, but about about types it will additionally bring in).
 We have dwz and likely would get most of the growth back due to redundancy
 removal though.
 
  Jakub
 
 I did a quick test on a large application of ours where this issue was 
 causing trouble.  Without the patch, it generates 115 MB of debug data; with 
 the patch, that grows to 120 MB.

So given the comments, is this patch now ok to commit?

Thanks,
paul



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Cary Coutant
 So given the comments, is this patch now ok to commit?

Yes, this is OK. Thanks for doing the extra testing! (I also ran a
quick test with -fdebug-types-section, just to make sure.)

-cary


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Jason Merrill

On 10/04/2012 08:26 PM, Cary Coutant wrote:

It seems to me that there are cases where we just want to emit the
class for the context info (like a namespace, which doesn't have to be
complete everywhere). Is there a way to tell the debugger that this
class declaration is incomplete and that it should look elsewhere for
a full definition?


I think DW_AT_declaration would be appropriate.

Jason



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Cary Coutant
 It seems to me that there are cases where we just want to emit the
 class for the context info (like a namespace, which doesn't have to be
 complete everywhere). Is there a way to tell the debugger that this
 class declaration is incomplete and that it should look elsewhere for
 a full definition?

 I think DW_AT_declaration would be appropriate.

Yeah, that's what I was thinking. I was going to check with dje to see
if GDB would do the right thing in that case.

-cary


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Paul_Koning

On Oct 5, 2012, at 2:43 PM, Cary Coutant wrote:

 It seems to me that there are cases where we just want to emit the
 class for the context info (like a namespace, which doesn't have to be
 complete everywhere). Is there a way to tell the debugger that this
 class declaration is incomplete and that it should look elsewhere for
 a full definition?
 
 I think DW_AT_declaration would be appropriate.
 
 Yeah, that's what I was thinking. I was going to check with dje to see
 if GDB would do the right thing in that case.
 
 -cary

Does that mean I should hold off committing this change?

paul



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Paul_Koning

On Oct 5, 2012, at 2:43 PM, Cary Coutant wrote:

 It seems to me that there are cases where we just want to emit the
 class for the context info (like a namespace, which doesn't have to be
 complete everywhere). Is there a way to tell the debugger that this
 class declaration is incomplete and that it should look elsewhere for
 a full definition?
 
 I think DW_AT_declaration would be appropriate.
 
 Yeah, that's what I was thinking. I was going to check with dje to see
 if GDB would do the right thing in that case.
 
 -cary

There certainly is a fair amount of code in dwarf2read.c in gdb to handle 
DW_AT_declaration and do things differently for declarations.

Should I rework this patch to use that mechanism instead?  If so, how?  If the 
class is marked only by prune_unused_types_mark visiting it as a parent, but 
hasn't been marked by ??? that visits all its children, then emit it with a 
DW_AT_declaration marking?

paul



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Cary Coutant
 There certainly is a fair amount of code in dwarf2read.c in gdb to handle 
 DW_AT_declaration and do things differently for declarations.

 Should I rework this patch to use that mechanism instead?  If so, how?  If 
 the class is marked only by prune_unused_types_mark visiting it as a parent, 
 but hasn't been marked by ??? that visits all its children, then emit it with 
 a DW_AT_declaration marking?

One question I'd consider is what do you want to see in the debugger
if this truly is the only debug info you have for the class? (For
example, in the test case you added, a DW_AT_declaration attribute
won't help if there's no full definition of the class anywhere else.)
Is it reasonable to just show a truncated class definition in that
case, or do you want the full definition available. My tentative
answer would be that we do the pruning here because we expect there to
be a full definition somewhere else, and that the lack of a
DW_AT_declaration attribute is the bug.

As you've discovered, however, it's not straightforward. You'll want
to add the declaration attribute if you mark the DIE from below, but
not from any reference where dokids is true. Alternatively, add the
declaration attribute if any of its children are pruned. Perhaps that
could be done in prune_unused_types_prune().

If you're willing to rework the patch this way (assuming GDB does the
right thing with it), I think that would be better. Thanks.

-cary


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-05 Thread Paul_Koning

On Oct 5, 2012, at 6:05 PM, Cary Coutant wrote:

 There certainly is a fair amount of code in dwarf2read.c in gdb to handle 
 DW_AT_declaration and do things differently for declarations.
 
 Should I rework this patch to use that mechanism instead?  If so, how?  If 
 the class is marked only by prune_unused_types_mark visiting it as a parent, 
 but hasn't been marked by ??? that visits all its children, then emit it 
 with a DW_AT_declaration marking?
 
 One question I'd consider is what do you want to see in the debugger
 if this truly is the only debug info you have for the class? (For
 example, in the test case you added, a DW_AT_declaration attribute
 won't help if there's no full definition of the class anywhere else.)
 Is it reasonable to just show a truncated class definition in that
 case, or do you want the full definition available. My tentative
 answer would be that we do the pruning here because we expect there to
 be a full definition somewhere else, and that the lack of a
 DW_AT_declaration attribute is the bug.
 
 As you've discovered, however, it's not straightforward. You'll want
 to add the declaration attribute if you mark the DIE from below, but
 not from any reference where dokids is true. Alternatively, add the
 declaration attribute if any of its children are pruned. Perhaps that
 could be done in prune_unused_types_prune().
 
 If you're willing to rework the patch this way (assuming GDB does the
 right thing with it), I think that would be better. Thanks.
 
 -cary

I'll give it a try.  I'll keep the existing patch in reserve in case we run out 
of time before Phase 1 end -- it would be good to have some sort of fix for 
this bug even if it's not the ideal one.

paul



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-04 Thread Cary Coutant
   /* We also have to mark its parents as used.
 -(But we don't want to mark our parents' kids due to this.)  */
 +(But we don't want to mark our parent's kids due to this,
 +unless it is a class.)  */
   if (die-die_parent)
 -   prune_unused_types_mark (die-die_parent, 0);
 +   prune_unused_types_mark (die-die_parent,
 +(die-die_parent-die_tag == 
 DW_TAG_class_type ||
 + die-die_parent-die_tag == 
 DW_TAG_structure_type ||
 + die-die_parent-die_tag == 
 DW_TAG_union_type));

I'd suggest replacing these conditions with a call to class_scope_p().
That will also cover DW_TAG_interface_type, which might be irrelevant
for this particular case, but is probably good to cover in the general
case.

-cary


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-04 Thread Paul_Koning

On Oct 4, 2012, at 1:38 PM, Cary Coutant wrote:

  /* We also have to mark its parents as used.
 -(But we don't want to mark our parents' kids due to this.)  */
 +(But we don't want to mark our parent's kids due to this,
 +unless it is a class.)  */
  if (die-die_parent)
 -   prune_unused_types_mark (die-die_parent, 0);
 +   prune_unused_types_mark (die-die_parent,
 +(die-die_parent-die_tag == 
 DW_TAG_class_type ||
 + die-die_parent-die_tag == 
 DW_TAG_structure_type ||
 + die-die_parent-die_tag == 
 DW_TAG_union_type));
 
 I'd suggest replacing these conditions with a call to class_scope_p().
 That will also cover DW_TAG_interface_type, which might be irrelevant
 for this particular case, but is probably good to cover in the general
 case.
 
 -cary

Thanks, that makes it very simple.  Here is the updated patch.

Ok to commit with this change?

paul

ChangeLog:

2012-10-04  Paul Koning  n...@arrl.net

* dwarf2out.c (prune_unused_types_mark): Mark all of parent's
children if parent is a class.

testsuite/ChangeLog:

2012-10-04  Paul Koning  n...@arrl.net

* g++.dg/debug/dwarf2/pr54508.C: New.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 192048)
+++ gcc/dwarf2out.c (working copy)
@@ -21035,9 +21035,11 @@
   prune_unused_types_mark_generic_parms_dies (die);
 
   /* We also have to mark its parents as used.
-(But we don't want to mark our parents' kids due to this.)  */
+(But we don't want to mark our parent's kids due to this,
+unless it is a class.)  */
   if (die-die_parent)
-   prune_unused_types_mark (die-die_parent, 0);
+   prune_unused_types_mark (die-die_parent, 
+class_scope_p (die-die_parent));
 
   /* Mark any referenced nodes.  */
   prune_unused_types_walk_attribs (die);
Index: testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,67 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options -g2 -dA }
+
+// { dg-final { scan-assembler \cbase0\\[ \t\]+\[#;/!|@\]+ 
DW_AT_name\|DW_AT_name: \cbase\ } }
+// { dg-final { scan-assembler \OPCODE0\\[ \t\]+\[#;/!|@\]+ 
DW_AT_name\|DW_AT_name: \OPCODE\ } }
+// { dg-final { scan-assembler \bi0\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler \si0\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler \f10\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler \f20\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler-not \nc0\\[ \t\]+\# 
DW_AT_name\|DW_AT_name: \nc\ } }
+
+class cbase
+
+{
+public:
+ static int si;
+int bi;
+};
+
+class c : public cbase
+
+{
+public:
+ enum
+ {
+  OPCODE = 251
+ };
+ int i ;
+ static const char *testc (void) { return foo; }
+};
+
+struct s
+{
+int f1;
+static const char *tests (void) { return test; }
+};
+
+union u
+{
+int f2;
+double d;
+static const char *testu (void) { return test union; }
+};
+
+namespace n 
+{
+const char *ntest (void) { return test n; }
+
+class nc
+{
+public:
+int i;
+static int sj;
+};
+}
+
+extern void send (int, int, const void *, int);
+
+void test (int src)
+{
+  int cookie = 1;
+  send(src, c::OPCODE, c::testc (), cookie);
+  send(src, c::OPCODE, s::tests (), cookie);
+  send(src, c::OPCODE, u::testu (), cookie);
+  send(src, c::OPCODE, n::ntest (), cookie);
+}


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-04 Thread Paul_Koning
Updated patch: there were two existing testcases that needed to be adjusted 
because of this fix.

Ran check RUNTESTFLAGS=dwarf2.exp, no regressions.

paul

ChangeLog:

2012-10-04  Paul Koning  n...@arrl.net

* dwarf2out.c (prune_unused_types_mark): Mark all of parent's
children if parent is a class.

testsuite/ChangeLog:

2012-10-04  Paul Koning  n...@arrl.net

* g++.dg/debug/dwarf2/pr54508.C: New.
* g++.dg/debug/dwarf2/localclass1.C: Expect staticfn1, staticfn2,
method1 in debug output.
* g++.dg/debug/dwarf2/localclass2.C: Likewise.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 192048)
+++ gcc/dwarf2out.c (working copy)
@@ -21035,9 +21035,11 @@
 prune_unused_types_mark_generic_parms_dies (die);

 /* We also have to mark its parents as used.
-(But we don't want to mark our parents' kids due to this.)  */
+(But we don't want to mark our parent's kids due to this,
+unless it is a class.)  */
 if (die-die_parent)
-   prune_unused_types_mark (die-die_parent, 0);
+   prune_unused_types_mark (die-die_parent, 
+class_scope_p (die-die_parent));

 /* Mark any referenced nodes.  */
 prune_unused_types_walk_attribs (die);
Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,67 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options -g2 -dA -fno-merge-debug-strings }
+
+// { dg-final { scan-assembler \cbase0\\[ \t\]+\[#;/!|@\]+ DW_AT_name 
} }
+// { dg-final { scan-assembler \OPCODE0\\[ \t\]+\[#;/!|@\]+ DW_AT_name 
} }
+// { dg-final { scan-assembler \bi0\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler \si0\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler \f10\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler \f20\\[ \t\]+\[#;/!|@\]+ DW_AT_name } }
+// { dg-final { scan-assembler-not \nc0\\[ \t\]+\# DW_AT_name } }
+
+class cbase
+
+{
+public:
+ static int si;
+int bi;
+};
+
+class c : public cbase
+
+{
+public:
+ enum
+ {
+  OPCODE = 251
+ };
+ int i ;
+ static const char *testc (void) { return foo; }
+};
+
+struct s
+{
+int f1;
+static const char *tests (void) { return test; }
+};
+
+union u
+{
+int f2;
+double d;
+static const char *testu (void) { return test union; }
+};
+
+namespace n 
+{
+const char *ntest (void) { return test n; }
+
+class nc
+{
+public:
+int i;
+static int sj;
+};
+}
+
+extern void send (int, int, const void *, int);
+
+void test (int src)
+{
+  int cookie = 1;
+  send(src, c::OPCODE, c::testc (), cookie);
+  send(src, c::OPCODE, s::tests (), cookie);
+  send(src, c::OPCODE, u::testu (), cookie);
+  send(src, c::OPCODE, n::ntest (), cookie);
+}
Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
+++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
@@ -59,11 +59,11 @@
 // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
-// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
-// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
+// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
+// { dg-final { scan-assembler staticfn4\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn5\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn6\[^\n\r\]*DW_AT_name } }
-// { dg-final { scan-assembler-not method1\[^\n\r\]*DW_AT_name } }
+// { dg-final { scan-assembler method1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg2\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg3\[^\n\r\]*DW_AT_name } }
Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass2.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/localclass2.C (revision 192048)
+++ gcc/testsuite/g++.dg/debug/dwarf2/localclass2.C (working copy)
@@ -59,11 +59,11 @@
 // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
-// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
-// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
+// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
+// { dg-final { scan-assembler 

Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-04 Thread Cary Coutant
 Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
 ===
 --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
 +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
 @@ -59,11 +59,11 @@
  // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
  // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
  // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn4\[^\n\r\]*DW_AT_name } }
  // { dg-final { scan-assembler-not staticfn5\[^\n\r\]*DW_AT_name } }
  // { dg-final { scan-assembler-not staticfn6\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not method1\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler method1\[^\n\r\]*DW_AT_name } }
  // { dg-final { scan-assembler arg1\[^\n\r\]*DW_AT_name } }
  // { dg-final { scan-assembler arg2\[^\n\r\]*DW_AT_name } }
  // { dg-final { scan-assembler arg3\[^\n\r\]*DW_AT_name } }

The fact that these two tests were specifically checking for the
absence of staticfn3 and staticfn4 leads me to believe that the
current behavior is deliberate. Jakub, that change was yours (it dates
back to November 2008). Are you OK with Paul's change?

It seems to me that there are cases where we just want to emit the
class for the context info (like a namespace, which doesn't have to be
complete everywhere). Is there a way to tell the debugger that this
class declaration is incomplete and that it should look elsewhere for
a full definition?

-cary


Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-04 Thread Paul_Koning

On Oct 4, 2012, at 8:26 PM, Cary Coutant wrote:

 Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
 ===
 --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
 +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
 @@ -59,11 +59,11 @@
 // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn4\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn5\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn6\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not method1\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler method1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg2\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg3\[^\n\r\]*DW_AT_name } }
 
 The fact that these two tests were specifically checking for the
 absence of staticfn3 and staticfn4 leads me to believe that the
 current behavior is deliberate. Jakub, that change was yours (it dates
 back to November 2008). Are you OK with Paul's change?
 
 It seems to me that there are cases where we just want to emit the
 class for the context info (like a namespace, which doesn't have to be
 complete everywhere). Is there a way to tell the debugger that this
 class declaration is incomplete and that it should look elsewhere for
 a full definition?
 
 -cary

Certainly GDB does not currently do that.  As it stands, it uses whatever 
definition it finds first, so depending on which compilation unit's symbols are 
read first, what you see varies unpredictably. If there is a way to express in 
Dwarf-2 the fact that this is an incomplete definition, GDB could presumably be 
changed to take advantage of that.  I have no idea how hard that is.

paul



Re: PING^2: [patch] pr/54508: fix incomplete debug information for class

2012-10-04 Thread Paul_Koning

On Oct 4, 2012, at 8:26 PM, Cary Coutant wrote:

 Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
 ===
 --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
 +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
 @@ -59,11 +59,11 @@
 // { dg-final { scan-assembler foo\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler staticfn2\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn3\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not staticfn4\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn3\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler staticfn4\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn5\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler-not staticfn6\[^\n\r\]*DW_AT_name } }
 -// { dg-final { scan-assembler-not method1\[^\n\r\]*DW_AT_name } }
 +// { dg-final { scan-assembler method1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg1\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg2\[^\n\r\]*DW_AT_name } }
 // { dg-final { scan-assembler arg3\[^\n\r\]*DW_AT_name } }
 
 The fact that these two tests were specifically checking for the
 absence of staticfn3 and staticfn4 leads me to believe that the
 current behavior is deliberate. Jakub, that change was yours (it dates
 back to November 2008). Are you OK with Paul's change?
 
 It seems to me that there are cases where we just want to emit the
 class for the context info (like a namespace, which doesn't have to be
 complete everywhere). Is there a way to tell the debugger that this
 class declaration is incomplete and that it should look elsewhere for
 a full definition?
 
 -cary

The code itself (where I changed dwarf2out.c) is from 2003-02-28, by rth, what 
appears to be the original implementation of the -feliminate-unused-debug-types 
flag.

Looking at Jakub's change from 2008 and PR/27017 it fixes, it looks more like a 
case of much more debug information that was missing and Jakub's change 
corrected that.  It looks like those two testcase files describe the resulting 
behavior, but I don't read the discussion in PR/27017 as saying that having 
staticfn3 omitted was specifically desired. 

paul