[llvm-dev] [PATCH] D14358: DWARF's forward decl of a template should have template parameters.
David Blaikie via llvm-dev
llvm-dev at lists.llvm.org
Wed Dec 9 11:28:05 PST 2015
On Wed, Dec 9, 2015 at 11:11 AM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:
> 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
> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Wednesday, December 09, 2015 10:49 AM
> *To:* Robinson, Paul
> *Cc:* Marshall, Peter; llvm-dev; cfe-commits (cfe-commits at lists.llvm.org)
>
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Wed, Dec 9, 2015 at 10:40 AM, Robinson, Paul <
> Paul_Robinson at playstation.sony.com> wrote:
>
> 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
>
>
>
> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Friday, November 13, 2015 11:21 AM
> *To:* Marshall, Peter; llvm-dev
> *Cc:* Robinson, Paul
>
>
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Fri, Nov 13, 2015 at 6:16 AM, <Peter_Marshall at sn.scee.net> wrote:
>
> 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)
>
>
>
>
>
> As there are no template parameters for the forward declaration of either
> A<MONKEY>
> they are indistinguishable.
>
> The reason we currently have no need for the parameters in a template name
> is because we
> reconstruct template names from their parameter tags. This allow the
> pretty printing to match
> the templates from the DWARF to match our demangled symbols from the ELF
> symbol table.
>
> -Pete
>
>
>
>
> From: "Robinson, Paul" <Paul_Robinson at playstation.sony.com>
> To: David Blaikie <dblaikie at gmail.com>, "Marshall, Peter" <
> Peter_Marshall at sn.scee.net>
> Cc: "reviews+D14358+public+d3104135076f0a10 at reviews.llvm.org"
> <reviews+D14358+public+d3104135076f0a10 at reviews.llvm.org>,
> "cfe-commits (cfe-commits at lists.llvm.org)" <cfe-commits at lists.llvm.org>
> Date: 10/11/2015 01:08
> Subject: RE: [PATCH] D14358: DWARF's forward decl of a template
> should have template parameters.
> ------------------------------
>
>
>
>
> | when/where/why are types acquired from the mangled names of ELF
> symbols, rather than from corresponding DWARF?
>
> Pete, can you help me out here? David seems to want an ironclad case for
> not being able to do something any other way, before he will let me put the
> template type parameters on the declaration of a template instantiation.
> (He does not deny that doing so would be valid DWARF, only that it can't
> possibly be *useful* DWARF.)
> Thanks,
> --paulr
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com <dblaikie at gmail.com>]
> * Sent:* Monday, November 09, 2015 4:08 PM
> * To:* Robinson, Paul
> * Cc:* reviews+D14358+public+d3104135076f0a10 at reviews.llvm.org;
> cfe-commits (cfe-commits at lists.llvm.org)
> * Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
> On Mon, Nov 9, 2015 at 3:55 PM, Robinson, Paul <
> Paul_Robinson at playstation.sony.com> wrote:
> | Why is matching by name insufficient/not correct?
> I'm told we look at the mangled names in the ELF symbol table, demangle
> them, and look in the DWARF for the corresponding types.
>
> Not quite sure I follow that - when/where/why are types acquired from the
> mangled names of ELF symbols, rather than from corresponding DWARF? (eg:
> the DWARF describing the type of a function's parameter?)
>
> Now, the mangled name (for predefined types in particular) provides a
> type description, not the name-as-emitted-by-Clang, and in fact the same
> type can have more than one name ('const int' and 'int const' for a trivial
> example). The name Clang provides in the DWARF does not necessarily match
> the name produced by the demangler; this makes name-matching way more
> trouble than you'd think. We're not interested in teaching the debugger
> how to parse template instantiation names.
> Having the template type parameter means we have an unambiguous
> description of the type, and can match it easily.
>
> | including unreferenced entities fails source fidelity
> I'll assume you meant to say _excluding_ unreferenced entities fails
> source fidelity,
>
> Indeed
>
> which is quite true, but there is a valid engineering tradeoff in that
> what the DWARF actually contains (or not, in the case of, say, unused
> function declarations or unreferenced class contents) represents one
> possible valid source that could have produced the same object. (I'm
> curious why an unreferenced formal parameter of a function still gets
> described, if this is your argument for omitting template parameters.)
>
> Omitting parameters would make the function description unusable for
> callers, for example - so there's some value in describing them so that a
> debugger can evaluate expressions involving calls to the function, yes?
>
> Omitting template parameters however is not the same as omitting
> unreferenced entities, because the template parameters *are* referenced—by
> the template instantiation itself;
>
> Not quite sure I follow that logic. It's quite possible to have
> unreferenced template parameters:
>
> template<typename>
> void f() { }
>
> and, omitting them from the source does not produce a valid program.
>
> Omitting the names still produces a valid program - though I'm not quite
> sure which omission you're referring to. (& even if we omit the names, we
> still describe the parameters - as we do for unused/unnamed function
> parameters)
>
> Now, one of the 3 debuggers Clang explicitly supports (i.e. gdb) seems
> not to mind that they're missing, but the other two would benefit from
> having these things, and I would really like to have Clang produce these
> things.
>
> It sounds like the LLDB bug you cited is being treated as an LLDB bug, not
> a Clang one, for now. So I'm not sure it's relevant to justifying Clang
> changes just yet, unless they come back & suggest that they don't actually
> have enough information to implement the features they would like to
> implement.
>
> & equally I'd like to understand the features that you'd like to build
> with this info that can't be built without it (as a minimum: features that
> GDB doesn't support, since any features GDB does support seem to be
> implementable with the current info Clang and GCC emit)
>
> - David
>
>
> Thanks,
> --paulr
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> * Sent:* Monday, November 09, 2015 1:46 PM
>
> * To:* Robinson, Paul
> * Cc:* reviews+D14358+public+d3104135076f0a10 at reviews.llvm.org;
> cfe-commits (cfe-commits at lists.llvm.org)
> * Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
> On Thu, Nov 5, 2015 at 11:05 AM, Robinson, Paul <
> Paul_Robinson at playstation.sony.com> wrote:
> | What was your primary motivation?
> A similar concern to PR20455 from our own debugger. It much helps
> matching up the forward declaration and definition to have the parameters
> properly specified.
>
> Why is matching by name insufficient/not correct?
>
>
> | maybe it's possible to remangle the template using just the string name
> I have no idea what you're talking about here.
>
> Looking at PR20455 you linked, LLDB isn't finding the right function
> because of mangling:
> call to a function 'basic_string<char, char_traits<char>
> >::operator[](int) const' ('_ZNK12basic_stringIc17char_traits<char>EixEi')
> that is not present in the target
> It hasn't created the correct mangled name of operator[] - what I was
> saying is it might be possible to parse the template parameter from the
> pretty name, and use that to produce the mangled name. It /looks/ like GDB
> can manage this. Maybe only because we also include the mangled name of the
> member function? Not sure.
>
> | | Choosing to emit a forward/incomplete declaration in the first place
> fails source fidelity,
> | How so?
> When the source has a full definition but Clang chooses to emit only the
> declaration, per CGDebugInfo.cpp/shouldOmitDefinition().
>
> Sure, in the same way that including unreferenced entities fails source
> fidelity - all tradeoffs to reduce debug info size.
>
> Though the behavior is visible in a simpler example that doesn't have that
> failing (& if your change goes in, the test case should probably be
> simplified like this):
>
> template<typename T> struct foo;
> foo<int> *f;
>
> --paulr
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> * Sent:* Thursday, November 05, 2015 12:10 AM
> * To:* Robinson, Paul
> * Cc:* reviews+D14358+public+d3104135076f0a10 at reviews.llvm.org;
> cfe-commits (cfe-commits at lists.llvm.org)
>
> * Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
> On Wed, Nov 4, 2015 at 11:32 PM, Robinson, Paul <
> Paul_Robinson at playstation.sony.com> wrote:
> Would citing PR20455 help? It wasn't actually my primary motivation but
> it's not too far off. Having the template parameters there lets you know
> what's going on in the DWARF, without having to fetch and parse the name
> string of every struct you come across. Actually I'm not sure parsing the
> name string is unambiguous either; each parameter is either a typename, or
> an expression, but without the parameter DIEs you don't know which,
> a-priori. (What does <foo> mean? Depends on whether you think it should be
> a type name or a value; you can't tell, syntactically, you have to do some
> lookups. Ah, but if you had the parameter DIEs, you would Just Know.)
>
> For LLDB's needs, I'm not sure it's sufficient either - but I wouldn't
> mind an answer before we use it as the basis for this change (it sounds
> like maybe it's possible to remangle the template using just the string
> name, rather than needing an explicit representation of the parameters)
>
> What was your primary motivation?
>
> Choosing to emit a forward/incomplete declaration in the first place
> fails source fidelity,
>
> How so? You might have only a template declaration (template<typename T>
> struct foo; foo<int> *f;) or you may've only instantiated the declaration
> (the C++ language requires you to instantiate or avoid instantiating
> certain things in certain places, so in some contexts you /only/ have an
> instantiated declaration, not a definition)
>
> but it is a practical engineering tradeoff of compile/link performance
> against utility; and, after all, the source *could* have been written that
> way, with no semantic difference. But, if we're going to emit a white-lie
> incomplete declaration, we should do so correctly.
>
> Again, "correct" in DWARF is a fairly nebulous concept.
>
> --paulr
>
> P.S. We should talk about this forward-declaration tactic wrt LTO
> sometime. I have a case where a nested class got forward-declared; it's
> entirely conceivable that the outer class with the inner forward-declared
> class would end up being picked by LTO, leaving the user with no debug info
> for the inner class contents.
>
> I believe this Just Works(tm). The things that can vary per-insntance of a
> type (implicit special members, member template implicit specializations,
> and nested types*) are not added to the type's child list, but they
> reference the child as their parent. So they continue to apply no matter
> which instance of the type is picked for uniquing (because of the
> name-based referencing, so the nested type definition just says "my parent
> is _Zfoo" and whatever _Zfoo we end up picking in the LTO linking/metadata
> deduplication will serve that role just fine)
>
> * we could just do a better job of modelling nested types (& other
> non-globally scoped types) in a way that more closely models the source by
> emitting a declaration where they were declared, and a definition where
> they are defined (with the usual DW_AT_specification to wire them up)
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> * Sent:* Wednesday, November 04, 2015 8:30 PM
> * To:* reviews+D14358+public+d3104135076f0a10 at reviews.llvm.org; Robinson,
> Paul
> * Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
> On Wed, Nov 4, 2015 at 5:53 PM, Paul Robinson via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> probinson added a comment.
>
> GCC 4.8.4 on Linux doesn't produce these, but DWARF 4 section 5.5.8 says a
> class template instantiation is just like the equivalent non-template class
> entry, with the exception of the template parameter entries. I read that
> as meaning an incomplete description (i.e. with DW_AT_declaration) lets you
> omit all the other children, but not the template parameters.
>
> As usual, I think it's pretty hard to argue that DWARF /requires/ anything
> (permissive & all that). And I'm not sure that having these is particularly
> valuable/useful - what use do you have in mind for them?
>
> Wouldn't hurt to have some size info about the cost here, though I don't
> imagine it's massive, it does open us up to emitting a whole slew of new
> types (the types the template is instantiated with, and anything that
> depends on - breaking/avoiding type edges can, in my experience, be quite
> beneficial (I described an example of this in my lightning talk last
> week)).
>
>
> I don't think omitting the template DIEs was an intentional optimization,
> in the sense of being a decision separate from deciding to emit the
> incomplete/forward declaration in the first place. They were just omitted
> because we were omitting everything, but everything turns out to be
> non-compliant.
>
>
> http://reviews.llvm.org/D14358
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
>
>
>
> **********************************************************************
> This email and any files transmitted with it are confidential and intended
> solely for the use of the individual or entity to whom they are addressed.
> If you have received this email in error please notify postmaster at scee.net
> This footnote also confirms that this email message has been checked for
> all known viruses.
> Sony Computer Entertainment Europe Limited
> Registered Office: 10 Great Marlborough Street, London W1F 7LP, United
> Kingdom
> Registered in England: 3277793
> **********************************************************************
>
> P* Please consider the environment before printing this e-mail*
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151209/3df00c76/attachment-0001.html>
More information about the llvm-dev
mailing list