>> Would you consider it OK with a comment?
>
> Yes.
>
>> How about if I call copy_declaration_context directly from
>> remove_child_or_replace_with_skeleton, instead of calling them
>> sequentially in break_out_comdat_types?
>
> OK.
Updated patch attached. I fixed the regex so the test is independent
of whether the strings are emitted with DW_FORM_string or
DW_FORM_strp.
Bootstrapped on x86_64, no regressions.
OK for trunk, or hold for next stage 1?
-cary
2012-01-19 Cary Coutant <ccout...@google.com>
Dodji Seketeli <do...@redhat.com>
gcc/ChangeLog:
PR debug/45682
* dwarf2out.c (copy_declaration_context): Return ref to parent
of declaration DIE, if necessary.
(remove_child_or_replace_with_skeleton): Add new parameter; update
caller. Place skeleton DIE under parent DIE of original declaration.
Move call to copy_declaration_context to here ...
(break_out_comdat_types): ... from here.
gcc/testsuite/ChangeLog:
PR debug/45682
* g++.dg/debug/dwarf2/nested-3.C: New test.
2012-01-19 Cary Coutant <ccout...@google.com>
Dodji Seketeli <do...@redhat.com>
gcc/ChangeLog:
PR debug/45682
* dwarf2out.c (copy_declaration_context): Return ref to parent
of declaration DIE, if necessary.
(remove_child_or_replace_with_skeleton): Add new parameter; update
caller. Place skeleton DIE under parent DIE of original declaration.
Move call to copy_declaration_context to here ...
(break_out_comdat_types): ... from here.
gcc/testsuite/ChangeLog:
PR debug/45682
* g++.dg/debug/dwarf2/nested-3.C: New test.
commit 959d3b18ff6812a0316960ac49b283c11fd20460
Author: Cary Coutant <ccout...@google.com>
Date: Thu Jan 19 14:00:59 2012 -0800
PR debug/45682: Place skeleton DIE under parent of original decl.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index f9f4295..6101087 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3302,11 +3302,12 @@ static int should_move_die_to_comdat (dw_die_ref);
static dw_die_ref clone_as_declaration (dw_die_ref);
static dw_die_ref clone_die (dw_die_ref);
static dw_die_ref clone_tree (dw_die_ref);
-static void copy_declaration_context (dw_die_ref, dw_die_ref);
+static dw_die_ref copy_declaration_context (dw_die_ref, dw_die_ref);
static void generate_skeleton_ancestor_tree (skeleton_chain_node *);
static void generate_skeleton_bottom_up (skeleton_chain_node *);
static dw_die_ref generate_skeleton (dw_die_ref);
static dw_die_ref remove_child_or_replace_with_skeleton (dw_die_ref,
+ dw_die_ref,
dw_die_ref);
static void break_out_comdat_types (dw_die_ref);
static dw_die_ref copy_ancestor_tree (dw_die_ref, dw_die_ref, htab_t);
@@ -7070,16 +7071,17 @@ clone_as_declaration (dw_die_ref die)
return clone;
}
-/* Copy the declaration context to the new compile unit DIE. This includes
+/* Copy the declaration context to the new type unit DIE. This includes
any surrounding namespace or type declarations. If the DIE has an
AT_specification attribute, it also includes attributes and children
attached to the specification. */
-static void
+static dw_die_ref
copy_declaration_context (dw_die_ref unit, dw_die_ref die)
{
dw_die_ref decl;
dw_die_ref new_decl;
+ dw_die_ref orig_parent = NULL;
decl = get_AT_ref (die, DW_AT_specification);
if (decl == NULL)
@@ -7090,6 +7092,10 @@ copy_declaration_context (dw_die_ref unit, dw_die_ref
die)
dw_die_ref c;
dw_attr_ref a;
+ /* The original DIE will be changed to a declaration, and must
+ be moved to be a child of the original declaration DIE. */
+ orig_parent = decl->die_parent;
+
/* Copy the type node pointer from the new DIE to the original
declaration DIE so we can forward references later. */
decl->die_id.die_type_node = die->die_id.die_type_node;
@@ -7118,6 +7124,8 @@ copy_declaration_context (dw_die_ref unit, dw_die_ref die)
add_AT_specification (die, new_decl);
}
}
+
+ return orig_parent;
}
/* Generate the skeleton ancestor tree for the given NODE, then clone
@@ -7201,17 +7209,23 @@ generate_skeleton (dw_die_ref die)
return node.new_die;
}
-/* Remove the DIE from its parent, possibly replacing it with a cloned
- declaration. The original DIE will be moved to a new compile unit
- so that existing references to it follow it to the new location. If
- any of the original DIE's descendants is a declaration, we need to
- replace the original DIE with a skeleton tree and move the
- declarations back into the skeleton tree. */
+/* Remove the CHILD DIE from its parent, possibly replacing it with a cloned
+ declaration. The original DIE is moved to a new compile unit so that
+ existing references to it follow it to the new location. If any of the
+ original DIE's descendants is a declaration, we need to replace the
+ original DIE with a skeleton tree and move the declarations back into the
+ skeleton tree. */
static dw_die_ref
-remove_child_or_replace_with_skeleton (dw_die_ref child, dw_die_ref prev)
+remove_child_or_replace_with_skeleton (dw_die_ref unit, dw_die_ref child,
+ dw_die_ref prev)
{
- dw_die_ref skeleton;
+ dw_die_ref skeleton, orig_parent;
+
+ /* Copy the declaration context to the type unit DIE. If the returned
+ ORIG_PARENT is not NULL, the skeleton needs to be added as a child of
+ that DIE. */
+ orig_parent = copy_declaration_context (unit, c);
skeleton = generate_skeleton (child);
if (skeleton == NULL)
@@ -7219,7 +7233,19 @@ remove_child_or_replace_with_skeleton (dw_die_ref child,
dw_die_ref prev)
else
{
skeleton->die_id.die_type_node = child->die_id.die_type_node;
- replace_child (child, skeleton, prev);
+
+ /* If the original DIE was a specification, we need to put
+ the skeleton under the parent DIE of the declaration.
+ This leaves the original declaration in the tree, but
+ it will be pruned later since there are no longer any
+ references to it. */
+ if (new_parent != NULL)
+ {
+ remove_child_with_prev (child, prev);
+ add_child_die (new_parent, skeleton);
+ }
+ else
+ replace_child (child, skeleton, prev);
}
return skeleton;
@@ -7247,7 +7273,7 @@ break_out_comdat_types (dw_die_ref die)
next = (c == first ? NULL : c->die_sib);
if (should_move_die_to_comdat (c))
{
- dw_die_ref replacement;
+ dw_die_ref replacement, new_parent;
comdat_type_node_ref type_node;
/* Create a new type unit DIE as the root for the new tree, and
@@ -7264,11 +7290,9 @@ break_out_comdat_types (dw_die_ref die)
generate_type_signature (c, type_node);
/* Copy the declaration context, attributes, and children of the
- declaration into the new compile unit DIE. */
- copy_declaration_context (unit, c);
-
- /* Remove this DIE from the main CU. */
- replacement = remove_child_or_replace_with_skeleton (c, prev);
+ declaration into the new type unit DIE, then remove this DIE
+ from the main CU (or replace it with a skeleton if necessary). */
+ replacement = remove_child_or_replace_with_skeleton (unit, c, prev);
/* Break out nested types into their own type units. */
break_out_comdat_types (c);
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C
b/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C
new file mode 100644
index 0000000..707f02d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C
@@ -0,0 +1,54 @@
+// Origin: PR debug/45682
+// { dg-options "-g -fno-merge-debug-strings -gdwarf-4 -dA
-fdebug-types-section" }
+
+namespace thread {
+
+class Executor {
+ public:
+ static Executor* CurrentExecutor();
+};
+
+}
+
+namespace thread {
+
+Executor* Executor::CurrentExecutor() {
+ return 0;
+}
+
+}
+
+thread::Executor *te;
+
+int
+main ()
+{
+ return 0;
+}
+
+// We want to express the fact that the DIE for the definition of
+// 'Executor::CurrentExecutor' is a grand-child of the DIE for the
+// namespace 'thread'. We must have something like this output:
+// .uleb128 0x8 # (DIE (0x29) DW_TAG_namespace)
+// .ascii "thread\0" # DW_AT_name
+// .byte 0x1 # DW_AT_decl_file
(.../testsuite/g++.dg/debug/dwarf2/nested-3.C)
+// .byte 0x4 # DW_AT_decl_line
+// .long 0x4b # DW_AT_sibling
+// .uleb128 0x9 # (DIE (0x34) DW_TAG_class_type)
+// .long .LASF0 # DW_AT_name: "Executor"
+// # DW_AT_declaration
+// .uleb128 0x5 # (DIE (0x39) DW_TAG_subprogram)
+// # DW_AT_external
+// .long .LASF1 # DW_AT_name: "CurrentExecutor"
+// .byte 0x1 # DW_AT_decl_file
(.../testsuite/g++.dg/debug/dwarf2/nested-3.C)
+// .byte 0x8 # DW_AT_decl_line
+// .long .LASF2 # DW_AT_linkage_name:
"_ZN6thread8Executor15CurrentExecutorEv"
+// .long 0x4b # DW_AT_type
+// .byte 0x1 # DW_AT_accessibility
+// # DW_AT_declaration
+// .byte 0 # end of children of DIE 0x34
+// .byte 0 # end of children of DIE 0x29
+//
+// Hence the scary regexp:
+//
+// { dg-final { scan-assembler "\[^\n\r\]*\\(DIE \\(0x(\[0-9a-f\]+)\\)
DW_TAG_namespace\\)\[\n\r\]+\[^\n\r\]*\"thread\[\^\n\r]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\\(DIE
\\(0x(\[0-9a-f\]+)\\)
DW_TAG_class_type\\)\[\n\r\]+\[^\n\r\]*\"Executor\[^\n\r\]+\[\n\r\]+\[^\n\r\]*DW_AT_declaration\[\n\r\]+\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_subprogram\\)\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\"CurrentExecutor\[^\n\r\]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*end
of children of DIE 0x\\3\[\n\r]+\[^\n\r\]*end of children of DIE
0x\\1\[\n\r]+" } }
