[dragonegg PATCH] Fix debug info for std::nullptr_t

[Greg Clayton]

On Jul 1, 2013, at 7:10 PM, David Blaikie <dblaikie at gmail.com> wrote:

> 
> On Jul 1, 2013 6:30 PM, "Greg Clayton" <gclayton at apple.com> wrote:
> >
> >
> > On Jul 1, 2013, at 2:13 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > > On Mon, Jul 1, 2013 at 2:03 PM, Greg Clayton <gclayton at apple.com> wrote:
> > >>
> > >> On Jun 26, 2013, at 2:38 PM, Eric Christopher <echristo at gmail.com> wrote:
> > >>
> > >>>>>
> > >>>>> The short answer is that DW_TAG_unspecified_type can be used for more
> > >>>>> than just nullptr_t.
> > >>>>
> > >>>> So shouldn't the function be called createUnspecifiedType (possibly
> > >>>> with a createNullPtrType convenience function)?
> > >>>>
> > >>>
> > >>> Yep. Feel free to make this change. (Don't worry about
> > >>> createNullPtrType unless you really want it).
> > >>>
> > >>>>> The long answer is:
> > >>>>>
> > >>>>> "An unspecified (implicit, unknown, ambiguous or nonexistent) type is
> > >>>>> represented by a debugging information entry with the tag
> > >>>>> DW_TAG_unspecified_type. If a name has been given to the type, then
> > >>>>> the corresponding unspecified type entry has a DW_AT_name attribute
> > >>>>> whose value is a null-terminated string containing the name as it
> > >>>>> appears in the source program. The interpretation of this debugging
> > >>>>> information entry is intentionally left flexible to allow it to be
> > >>>>> interpreted appropriately in different languages. For example, in C
> > >>>>> and C++ the language implementation can provide an unspecified type
> > >>>>> entry with the name “void” which can be referenced by the type
> > >>>>> attribute of pointer types and typedef declarations for 'void' (see
> > >>>>> Sections 0 and 5.3, respectively). As another example, in Ada such an
> > >>>>> unspecified type entry can be referred to by the type attribute of an
> > >>>>> access type where the denoted type is incomplete (the name is declared
> > >>>>> as a type but the definition is deferred to a separate compilation
> > >>>>> unit)."
> > >>>>
> > >>>> So the name is left unspecified.  Unsurprisingly, Clang and GCC have
> > >>>> diverged here -- Clang uses "nullptr_t" as the name while GCC uses
> > >>>> "decltype(nullptr)".  A standards proposal of sorts [1] proposes that
> > >>>> "decltype(nullptr)" be used.  And it looks like LLDB only recognises
> > >>>> the "nullptr_t" name [2].  What a mess.  Would changing Clang (and
> > >>>> DragonEgg) to use "decltype(nullptr)", and making LLDB recognise it
> > >>>> as well, present too many compatibility issues?
> > >>>>
> > >>>
> > >>> Sounds good to me.
> > >>>
> > >>> Greg: Would you mind handling this on the LLDB end please?
> > >>>
> > >> % svn diff
> > >> Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
> > >> ===================================================================
> > >> --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (revision 185381)
> > >> +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (working copy)
> > >> @@ -5740,7 +5740,8 @@
> > >>                         break;
> > >>
> > >>                     case DW_TAG_unspecified_type:
> > >> -                        if (strcmp(type_name_cstr, "nullptr_t") == 0)
> > >> +                        if (strcmp(type_name_cstr, "nullptr_t") == 0 ||
> > >> +                            strcmp(type_name_cstr, "decltype(nullptr)") == 0 )
> > >
> > > Sounds good.
> > >
> > > (though a little surprised you're dealing with raw c strings here,
> > > rather than at least StringRef or somesuch)
> >
> >
> > The strings are from the .debug_str in DWARF. We tend not to StringRef all strings in the DWARF string table because of the space that would require (pointer + size_t).
> >
> > For higher performance we can switch to lldb_private::ConstString which puts and uniques strings in a constant string pool, but then we pay the cost of hashing and getting the unique string just to do the compare. C++ tends to make very very very large string pools out of the DWARF .debug_str and we really can't afford the extra space for StringRef permanent storage. Nor do we want every string table access to the .debug_str to incur the cost of calling strlen() for every string each time we access it.
> >
> > Rest assured if these areas ever show up as performance issues we will take care of the issues.
> 
> The specific issue that springs to mind here is that strcmp is sort of inefficient for equality if you happen to know the length already. If that's never/not often the case here then that's a non-issue.

Yes, we could do better in this function by calling strlen on "type_name_cstr" and then doing smarter string compares by using a preprocessor macro to automatically determine the string length and compare only if needed.

I will make that change.

Greg