[Lldb-commits] [lldb] r106045 - /lldb/trunk/include/lldb/Core/dwarf.h

[Greg Clayton]

Why not have a default parameter that has a prefix value that defaults to "DW_".

To answer Jason's "I have yet to see where an extra 3 characters..." comment: when you dump a very large DWARF file they tend to matter. Dumping WebCore and having an extra "DW_" in front of everything increases the size of the output by quite a bit...


On Jun 15, 2010, at 11:39 PM, Jason Molenda wrote:

> OK I'll look over the use of the printers in llvm tomorrow and make sure we don't get double DW_'s anywhere. ;)
> 
> Thanks,
> 
> J
> 
> On Jun 15, 2010, at 11:29 PM, Chris Lattner wrote:
> 
>> FWIW, I agree with Devang.  It would be nice to have the various dwarf constants all localized.  Imagine a world where the compiler and debugger agree on purpose instead of by accident! :)
>> 
>> -Chris
>> 
>> 
>> On Jun 15, 2010, at 11:26 PM, Devang Patel wrote:
>> 
>>> On Tue, Jun 15, 2010 at 8:27 PM, Jason Molenda <jmolenda at apple.com> wrote:
>>>> 
>>>> On Jun 15, 2010, at 2:43 PM, Chris Lattner wrote:
>>>> 
>>>>> Can Core/dwarf.h use (and extend) llvm/include/llvm/Support/Dwarf.h instead of duplicating much of it?
>>>> 
>>>> Yeah, I can do that.  I need to add the full set of DW_OP_{lit,reg,breg}{0..31} constants to include/llvm/Support/Dwarf.h and lib/Support/Dwarf.cpp at the very least.  The printers over in Dwarf.cpp are a little annoying - the DW_TAG ones include the "DW_" prefix, the rest do not.  It always grates one me when I see the prefix missing in program output - I have yet to see a context where the extra three characters would have killed anyone.  I'm adding functions like
>>>> 
>>>> const char *
>>>> DW_ACCESS_value_to_name (uint32_t val)
>>>> {
>>>> static char invalid[100];
>>>> const char *llvmstr = AccessibilityString (val);
>>>> if (llvmstr == NULL)
>>>> {
>>>>    snprintf (invalid, sizeof (invalid), "Unknown DW_ACCESS constant: 0x%x", val);
>>>>    return invalid;
>>>> }
>>>> 
>>>> std::string str;
>>>> if (strlen (llvmstr) < 3 || llvmstr[0] != 'D' || llvmstr[1] != 'W' || llvmstr[2] != '_')
>>>>  str = "DW_";
>>>> str += llvmstr;
>>>> ConstString const_str (str.c_str());
>>>> 
>>>> return const_str.GetCString();
>>>> }
>>> 
>>> Instead of doing this, feel free to update llvm/lib/Support/Dwarf.cpp
>>> printer routines to use DW_ prefix.
>>> 
>>>> to lldb's DWARFDefines.cpp to compensate.  We also have a "value to englishy name" utility function that removes all prefix chars (e.g. "DW_FORM_") and changes '_'s to ' ' for printing; they're used in a few places.  To reimplement those on top of the llvm printers I needed to add the modified string to lldb's const string pool and return a pointer to that.  But for the simple value_to_name printers, it's a lot of extra fussing about to ensure that the strings have "DW_" at the front.
>>>> 
>>>> 
>>>> In general I prefer the generated approach used for the header/printer functions in lldb -- there is a text file like
>>>> 
>>>> /* START: [7.5.4] Figure 16 "Tag Encodings" (pp. 125-127) in DWARFv3 draft 8 */
>>>> /* PREFIX: DW_TAG */
>>>> /* VALIDATTRIBS: DWARFv3 */
>>>> 
>>>> array_type 0x01
>>>> class_type 0x02
>>>> entry_point 0x03
>>>> enumeration_type 0x04
>>>> formal_parameter 0x05
>>>> imported_declaration 0x08
>>>> label 0x0a
>>>> lexical_block 0x0b
>>>> member 0x0d
>>>> pointer_type 0x0f
>>>> 
>>>> etc. so that only one table is maintained.  A simple script emits the header file and C source file that prints the texts.  In practice the contents change so rarely that it's not a big deal.
>>>> 
>>>> 
>>>> OK to check in the additional constant values to llvm?
>>> 
>>> Sure, feel free to add new constants in llvm/include/llvm/Support/Dwarf.h
>>> 
>>> -
>>> Devang
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits