[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

Hi Paul,

Sorry for the delay, I've been out of the office.

I think this example shows that name matching does not always work:

template<typename T> class A {
public:
        A(T val);
private:
        T x;
};

struct B {
        typedef float MONKEY;

        A<MONKEY> *p;
};

B b;

struct C {
        typedef int MONKEY;

        A<MONKEY> *p;
};

C c;

This gives this DWARF:

+-0000003f DW_TAG_structure_type "B"
   -DW_AT_name DW_FORM_strp "B"
  +-00000047 DW_TAG_member "p"
     -DW_AT_name DW_FORM_strp "p"
    +-DW_AT_type DW_FORM_ref4 0x00000054
      +-00000054 DW_TAG_pointer_type
        +-DW_AT_type DW_FORM_ref4 0x00000059
          +-00000059 DW_TAG_class_type "A<MONKEY>"
             -DW_AT_name DW_FORM_strp "A<MONKEY>"
             -DW_AT_declaration DW_FORM_flag_present

+-00000073 DW_TAG_structure_type "C"
   -DW_AT_name DW_FORM_strp "C"
  +-0000007b DW_TAG_member "p"
     -DW_AT_name DW_FORM_strp "p"
    +-DW_AT_type DW_FORM_ref4 0x00000088
      +-00000088 DW_TAG_pointer_type
        +-DW_AT_type DW_FORM_ref4 0x0000008d
          +-0000008d DW_TAG_class_type "A<MONKEY>"
             -DW_AT_name DW_FORM_strp "A<MONKEY>"
             -DW_AT_declaration DW_FORM_flag_present

That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be
surprising if we used the typedef (or otherwise non-canonical) name in the
class name):

(I've trimmed a few irrelevant attributes)

0x0000001e: DW_TAG_variable [2]
                DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000004c] =
"b")
                DW_AT_type [DW_FORM_ref4] (cu + 0x0033 =>
{0x00000033})

0x00000033: DW_TAG_structure_type [3] *
                DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000059] =
"B")

0x0000003b: DW_TAG_member [4]
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000004e] =
"p")
                  DW_AT_type [DW_FORM_ref4] (cu + 0x0048 =>
{0x00000048})

0x00000047: NULL

0x00000048: DW_TAG_pointer_type [5]
                DW_AT_type [DW_FORM_ref4] (cu + 0x004d =>
{0x0000004d})

0x0000004d: DW_TAG_class_type [6]
                DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000050] =
"A<float>")
                DW_AT_declaration [DW_FORM_flag_present] (true)

0x00000052: DW_TAG_variable [2]
                DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000005b] =
"c")
                DW_AT_type [DW_FORM_ref4] (cu + 0x0067 =>
{0x00000067})

0x00000067: DW_TAG_structure_type [3] *
                DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000064] =
"C")

0x0000006f: DW_TAG_member [4]
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000004e] =
"p")
                  DW_AT_type [DW_FORM_ref4] (cu + 0x007c =>
{0x0000007c})

0x0000007b: NULL

0x0000007c: DW_TAG_pointer_type [5]
                DW_AT_type [DW_FORM_ref4] (cu + 0x0081 =>
{0x00000081})

0x00000081: DW_TAG_class_type [6]
                DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000005d] =
"A<int>")
                DW_AT_declaration [DW_FORM_flag_present] (true)

That doesn’t seem to be the DWARF I’m seeing from Clang (& it’d be surprising if we used the typedef (or otherwise non-canonical) name in the class name):

Finally getting back to this…… Ha. We don’t unwrap the typedefs (“name as it is in the source”), while the upstream compiler does.

Providing the template-parameter DIEs is still the correct thing to do per the DWARF spec.

–paulr

That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be
surprising if we used the typedef (or otherwise non-canonical) name in the
class name):

Finally getting back to this….. Ha. We don't unwrap the typedefs ("name
as it is in the source"), while the upstream compiler does.

Yeah, I imagine you'd want to fix that as I expect it would cause you other
problems, no? (or is there some reason you have this change to the
compiler? I imagine it'd be hard to have that divergence by accident?)

Providing the template-parameter DIEs is still the correct thing to do per
the DWARF

spec.

I still don't agree that the DWARF we produce here is incorrect (the DWARF
spec is pretty loose on "correctness" of DWARF). If there's some practical
problem/use case it'd be useful to understand it so we make sure we're
fixing it the right way.

- Dave

Actually no, we prefer to have the original typedef names in the instantiation name, for source fidelity. “Name as it is in the source” or something reasonably close. Unwrapping typedefs is going too far.

Re. “looseness” of the DWARF spec, it is not so loose as you like to think. Attributes tend to be fairly optional or can be used “in novel ways” but the DIEs and their relationships are not like that. “Where this specification provides a means for describing the source language, implementors are expected to adhere to that specification.”

–paulr

Actually no, we prefer to have the original typedef names in the
instantiation name, for source fidelity.

Then perhaps you should keep this change in your tree too - since that's
where the need is?

  "Name as it is in the source" or something reasonably close. Unwrapping
typedefs is going too far.

Yet this isn't the choice upstream in Clang or GCC. I don't know about
other DWARF generators, but it seems your interpretation isn't the way some
other people/implementers are reading the DWARF spec.

[This seems like it would present a multitude of challenges to any DWARF
debugger dealing with this kind of debug info - it'd have to know far more
about the rules of the C++ language (which you've previously argued in
favor of avoiding) to perform a variety of operations if the types don't
match up fairly trivially.]

In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies
to definitions of a type, not declarations. ("Each formal parameterized
type declaration appearing in the template definition is represented by a
debugging information entry with the tag DW_TAG_template_type_parameter")
which, I agree, seems like a bug in the spec to not /allow/ them on
declarations, but I'd equally argue requiring them would seem too narrow to
me.

Re. "looseness" of the DWARF spec, it is not so loose as you like to
think. Attributes tend to be fairly optional or can be used "in novel
ways" but the DIEs and their relationships are not like that. "Where this
specification provides a means for describing the source language,
implementors are expected to adhere to that specification."

Why would that clause apply to attributes any less than it applies to DIEs?
It seems like a fairly broad statement.

I forget whether we already discussed it - but do you have any size data
(preferably/possibly from a fission build or otherwise measurement of "just
the debug info" not the whole binary) on, for example, a clang selfhost?

- Dave

Maybe we are being too pedantic about the names. I’ll have to go back and look in detail at why we decided to do that.

In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies to definitions of a type, not declarations. (“Each formal parameterized type declaration appearing in the template definition is represented by a debugging information entry with the tag DW_TAG_template_type_parameter”)

Not so fast… It’s a template definition of a type declaration. DWARF 5 is less ambiguous about this IMO, although you are actually very good at finding the ambiguities! The relevant text in DWARF 5 current draft is: “A debugging information entry that represents a template instantiation will contain child entries describing the actual template parameters.” Are you willing to argue that this type declaration is not an instantiation? (If not, what is it?)

Why would that clause apply to attributes any less than it applies to DIEs?

It does apply equally. However, nearly all attribute descriptions are specified as “may have” and therefore can be omitted freely without being non-conforming.

–paulr

Maybe we are being too pedantic about the names. I'll have to go back and
look in detail at why we decided to do that.

In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies
to definitions of a type, not declarations. ("Each formal parameterized
type declaration appearing in the template definition is represented by a
debugging information entry with the tag DW_TAG_template_type_parameter")

Not so fast… It's a template definition of a type declaration. DWARF 5 is
less ambiguous about this IMO, although you are actually very good at
finding the ambiguities! The relevant text in DWARF 5 current draft is: "A
debugging information entry that represents a template instantiation will
contain child entries describing the actual template parameters." Are you
willing to argue that this type declaration is not an instantiation? (If
not, what is it?)

Nope, it is an instantiation, for sure.

Beyond that, I'd still argue that existing implementation experience points
to this being pretty reasonable - and without the motivating use case
(which only exists in your fork) it seems hard to justify adding the extra
data.

Why would that clause apply to attributes any less than it applies to
DIEs?

It does apply equally. However, nearly all attribute descriptions are
specified as "may have" and therefore can be omitted freely without being
non-conforming.

Fair point (& the same goes for the example I was going to give of omitting
DW_TAG_friends - they're "may" too)

Types are a bit more vague (as to whether omitting unreferenced types is
supported by the standard) DWARF 4 just says "Structure, union, and class
types are represented by debugging information entries ...".

Any size numbers for this change?

In any case, it seems like it might make sense for you to upstream your
template naming change and put it under the PS4 debugger tuning option, and
put this change there too, once the motivation for it is in-tree. At that
point, while I'd be curious about the size tradeoff, it'd be essentially
academic.

- David

Types are a bit more vague (as to whether omitting unreferenced types is supported by the standard) DWARF 4 just says “Structure, union, and class types are represented by debugging information entries …”.

There’s some expansion of the “permissive” discussion in the works for DWARF 5. In essence, DWARF doesn’t tell you what to describe, but if you describe something, you do it how the spec says. So, omitting unused types and function declarations, or lexical blocks with no containing declarations, or even things like inlined subroutines, etc. etc. is all kosher, while things like the template parameter DIEs and the artificial import of anonymous namespaces are actually required.

Any size numbers for this change?

I got in the neighborhood of 1% (just under, IIRC) of the sum of .debug_* sections for a self-build of Clang.

In any case, it seems like it might make sense for you to upstream your template naming change and put it under the PS4 debugger tuning option, and put this change there too, once the motivation for it is in-tree. At that point, while I’d be curious about the size tradeoff, it’d be essentially academic

Exposing tuning up through Clang is actually very nearly at the top of my list now.

–paulr

> Types are a bit more vague (as to whether omitting unreferenced types is
supported by the standard) DWARF 4 just says "Structure, union, and class
types are represented by debugging information entries ...".

There's some expansion of the "permissive" discussion in the works for
DWARF 5. In essence, DWARF doesn't tell you _what_ to describe, but if you
describe something, you do it _how_ the spec says. So, omitting unused
types and function declarations, or lexical blocks with no containing
declarations, or even things like inlined subroutines, etc. etc. is all
kosher, while things like the template parameter DIEs and the artificial
import of anonymous namespaces are actually required.

Honestly I'm more with you on the template parameter DIEs than I am on the
anonymous namespace... especially from a source fidelity perspective.

> Any size numbers for this change?

I got in the neighborhood of 1% (just under, IIRC) of the sum of .debug_*
sections for a self-build of Clang.

Ah, cool - good to know. Thanks!

> In any case, it seems like it might make sense for you to upstream your
template naming change and put it under the PS4 debugger tuning option, and
put this change there too, once the motivation for it is in-tree. At that
point, while I'd be curious about the size tradeoff, it'd be essentially
academic

Exposing tuning up through Clang is actually very nearly at the top of my
list now.

Great :slight_smile: