PR debug/45682 - wrong struct DIE nesting with -fdebug-types-section

Hello,

In the example of the patch below, the DIE for the definition of the
class 'Executor', when put in the type unit section is not put under
the DIE for the 'thread' namespace.  Rather, it's put under the DIE
for the global namespace.

One thing to note is that the DIE for the declaration of 'Executor'
(the DIE that has a DW_AT_declaration attribute, and which is referred
to by the DW_AT_specification attribute of the DIE for the definition
of 'Executor') is correctly positioned as a children of the DIE for
the 'thread' namespace.

My understanding is that during the moving of a given type DIE into
its type unit section, we correctly move the declaration DIE of that
type along with its ancestor tree, but we fail to do the same for the
definition DIE.  For the later, we just omit its ancestor DIE in the
process.

Fixed thus, bootstrapped and tested on x86_64-unknown-linux-gnu
against trunk.

gcc/

	PR debug/45682
	* dwarf2out.c (copy_declaration_context): Return the copied DIE.
	(break_out_comdat_types): Move the definition DIE to under the
	same DIE as the declaration DIE.

gcc/testsuite/

	PR debug/45682
	* g++.dg/debug/dwarf2/nested-3.C: New test.
---
 gcc/dwarf2out.c                              |   13 ++++++----
 gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C |   34 ++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C

On Thu, Jan 05, 2012 at 11:47:49PM +0100, Dodji Seketeli wrote:
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C
> @@ -0,0 +1,34 @@
> +// Origin: PR debug/45682
> +// { dg-options "-g -gdwarf-4 -dA -fdebug-types-section" }
> +
> +namespace thread {
> +    class Executor {};
> +}
> +
> +thread::Executor *te;
> +
> +int
> +main ()
> +{
> +    return 0;
> +}
> +
> +// We want to express the fact that the DIE for the definition of
> +// 'Executor' is a child of the DIE for the namespace 'thread'. E.g,
> +// we must have this outout:
> +// 	.uleb128 0x2	# (DIE (0x25) DW_TAG_namespace)
> +//	.long	.LASF0	# DW_AT_name: "thread"
> +//			# DW_AT_declaration
> +//	.uleb128 0x3	# (DIE (0x2a) DW_TAG_class_type)
> +//	.long	.LASF1	# DW_AT_name: "Executor"
> +//			# DW_AT_declaration
> +//	.uleb128 0x4	# (DIE (0x2f) DW_TAG_class_type)
> +//	.byte	0x1	# DW_AT_byte_size
> +//	.byte	0x1	# DW_AT_decl_file (../../prtests/test-PR45682-2.cc)
> +//	.byte	0x2	# DW_AT_decl_line
> +//	.long	0x2a	# DW_AT_specification
> +//	.byte	0	# end of children of DIE 0x25
> +//
> +//	Hence the scary regexp:
> +//
> +//	{ dg-final { scan-assembler "\[^\n\r\]*\\(DIE \\(0x(\[0-9a-f\]+)\\) DW_TAG_namespace\\)\[\n\r\]+\[^\n\r\]*DW_AT_name: \"thread\"\[\n\r\]+\[^\n\r\]*DW_AT_declaration\[\n\r\]+\[^\n\r\]*\\(DIE \\(0x(\[0-9a-f\]+)\\)\[^\n\r\]*DW_TAG_class_type\\)\[\n\r\]+\[^\n\r\]*DW_AT_name: \"Executor\"\[\n\r\]+\[^\n\r\]*DW_AT_declaration\[\n\r\]+\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_class_type\\)\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*0x\\2\[^\n\r\]*\[\n\r\]+\[^\n\r\]*end of children of DIE 0x\\1" } }

Just a testcase comment, I bet this will surely fail on Darwin
or other targets that aren't capable of merging string sections.
So, you should add -fno-merge-debug-strings to dg-options
and adjust the example and regexp for that, that way
it will be the same on all targets.

	Jakub

> In the example of the patch below, the DIE for the definition of the
> class 'Executor', when put in the type unit section is not put under
> the DIE for the 'thread' namespace.  Rather, it's put under the DIE
> for the global namespace.

I don't think this is the problem that PR 45682 is complaining about.
Given the test case in your patch, the type unit looks like this:

  type_unit
  + namespace ("thread")
  + + L1: class_type ("Executor", declaration)
  + class_type (specification: L1)

The definition DIE is *supposed* to appear at the outer level, with
the specification DIE pointing to the declaration, which provides its
context. This is the way it would have appeared in the comp unit DIE
before splitting it out into a type unit. (And what it would look like
with -fno-debug-types-section.)

What PR 45682 is complaining about is that the skeleton DIE left
behind for class Executor is not inside the namespace. That problem,
however, is only visible with the full test case provided in the PR.
There, the DIE structure in .debug_types is good, but the DIE
structure in .debug_info is wrong:

  compile_unit
  + namespace ("thread")
  + class_type ("Executor", declaration)
  + + L1: subprogram ("CurrentExecutor", external)
  + ...
  + subprogram (specification: L1)
  + ...

Here, the DIE for class_type Executor should be under the namespace
DIE, but isn't. I don't think your patch fixes that problem.

I think the real problem is that the skeleton declaration DIE for
class Executor is being inserted under the wrong DIE -- probably in
remove_child_or_replace_with_skeleton().

Thanks for looking at this, though! It's been on my TODO list for a
while, but had dropped below the fold, and I'd forgotten about it.

-cary