diff mbox

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

Message ID C75A84166056C94F84D238A44AF9F6AD277757@AUSX10MPC103.AMER.DELL.COM
State New
Headers show

Commit Message

Paul Koning Oct. 23, 2012, 5:52 p.m. UTC 
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  <ni1d@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  <ni1d@arrl.net>

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

Comments

Jason Merrill Oct. 23, 2012, 6:02 p.m. UTC | #1 
OK.

Jason
Paul Koning Oct. 23, 2012, 6:44 p.m. UTC | #2 
On Oct 23, 2012, at 2:02 PM, Jason Merrill wrote:

> OK.
> 
> Jason

Thanks.  Committed.

	paul
diff mbox

Patch

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 "\"cbase\\\\0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"c\\\\0\"\[ \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 "\"OPCODE\\\\0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"bi\\\\0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"si\\\\0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"s\\\\0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"s\\\\0\"\[^#;/!|@\]+\[#;/!|@\]+ DW_AT_name\[\r\n\]+\[\^\r\n\]+\[\r\n\]+\[\^\r\n\]+\[\r\n\]+\[\^#;/!|@\]+\[#;/!|@\]+ DW_AT_decl_line\[\r\n\]+\[ \t\]+\[#;/!|@\]+ DW_AT_declaration" } }
+// { dg-final { scan-assembler "\"f1\\\\0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"u\\\\0\"\[ \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 "\"f2\\\\0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"nc\\\\0\"\[ \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;
+  static struct s ss;
+  
+  send(src, c::OPCODE, c::testc (), cookie);
+  send(src, c::OPCODE, u::testu (), cookie);
+  send(src, c::OPCODE, n::ntest (), cookie);
+}