On Wed, Dec 9, 2015 at 11:11 AM, Robinson, Paul < paul_robin...@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:dblai...@gmail.com] > *Sent:* Wednesday, December 09, 2015 10:49 AM > *To:* Robinson, Paul > *Cc:* Marshall, Peter; llvm-dev; cfe-commits (cfe-commits@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_robin...@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:dblai...@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_marsh...@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_robin...@playstation.sony.com> > To: David Blaikie <dblai...@gmail.com>, "Marshall, Peter" < > peter_marsh...@sn.scee.net> > Cc: "reviews+d14358+public+d3104135076f0...@reviews.llvm.org" > <reviews+d14358+public+d3104135076f0...@reviews.llvm.org>, > "cfe-commits (cfe-commits@lists.llvm.org)" <cfe-commits@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:dblai...@gmail.com <dblai...@gmail.com>] > * Sent:* Monday, November 09, 2015 4:08 PM > * To:* Robinson, Paul > * Cc:* reviews+d14358+public+d3104135076f0...@reviews.llvm.org; > cfe-commits (cfe-commits@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_robin...@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:dblai...@gmail.com] > * Sent:* Monday, November 09, 2015 1:46 PM > > * To:* Robinson, Paul > * Cc:* reviews+d14358+public+d3104135076f0...@reviews.llvm.org; > cfe-commits (cfe-commits@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_robin...@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:dblai...@gmail.com] > * Sent:* Thursday, November 05, 2015 12:10 AM > * To:* Robinson, Paul > * Cc:* reviews+d14358+public+d3104135076f0...@reviews.llvm.org; > cfe-commits (cfe-commits@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_robin...@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:dblai...@gmail.com] > * Sent:* Wednesday, November 04, 2015 8:30 PM > * To:* reviews+d14358+public+d3104135076f0...@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@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@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 postmas...@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* > > > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits