[PATCH] [dwarfdump] Implement extraction of file information referenced in .debug_info.

[David Blaikie]

On Mon, Oct 6, 2014 at 11:19 AM, Frédéric Riss <friss at apple.com> wrote:

>
> On Sep 30, 2014, at 5:43 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Sep 4, 2014 at 8:53 AM, Frédéric Riss <friss at apple.com> wrote:
>
>>
>> On 04 Sep 2014, at 17:40, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>>
>> On Thu, Sep 4, 2014 at 8:32 AM, Frédéric Riss <friss at apple.com> wrote:
>>
>>>
>>> On 04 Sep 2014, at 17:19, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>
>>>
>>>
>>> On Thu, Sep 4, 2014 at 6:55 AM, Frederic Riss <friss at apple.com> wrote:
>>>
>>>> Hi dblaikie, echristo, aprantl,
>>>>
>>>> This is the merge of 2 patches:
>>>>
>>>> Pass the DWARFUnit to DWARFDebugLine::getFileNameByIndex().
>>>>
>>>> This way it can query the compilation dir when it is referenced (when a
>>>> file references directory index 0, it refers to the compilation dir).
>>>>
>>>
>>> Alternatively (Though not necessarily better - open to
>>> opinions/preferences) DWARFUnit could have a wrapper for getFileNameByIndex
>>> and could be returned a zero/default/something that the DWARFUnit could
>>> detect and return the CU.
>>>
>>> Or the DWARFDebugLine could be constructed with its unit, or just with
>>> the comp_dir of its unit so it didn't have to be passed in/queried for on
>>> every lookup?
>>>
>>>
>>> I’m currently working on an alternative where I turn
>>> DWARFContext.cpp::getFileNameForUnit() and getFileLineInfoForCompileUnit()
>>> into LineTable methods. This way I reduce code duplication with what I’ve
>>> added to DWARFDebugInfoEntry::dump. I’ll see how that turns out.
>>>
>>>
>>>> and:
>>>>
>>>> [dwarfdump] Implement extraction of file information referenced in
>>>> .debug_info.
>>>>
>>>> This patch pretty prints the contents of the DW_AT_decl_file,
>>>> DW_AT_call_file, DW_AT_decl_line and DW_AT_call line attributes.
>>>> Drop the const on the passed DWARFUnit to be able to call
>>>> getLineTableForUnit (the line table construction is lazy, thus the getter
>>>> might modify the Unit).
>>>>
>>>
>>> What pretty printing occurs for lines? (obviously for files this would
>>> allow you to print the file name rather than the file number - but
>>> DW_AT_decl/call_line already contain the line number, right?)
>>>
>>>
>>> It’s just that they’re printed as decimal numbers. Which is indisputably
>>> more pretty for line numbers :-)
>>>
>>
>> Agreed - though should we just print numbers as decimal generally? Which
>> numbers benefit from being printed as hex I wonder?
>>
>>
>> All unsigned Forms get printed as hex by FormValue. As this covers
>> addresses for example, I wouldn’t call that a bad choice. In fact, I can’t
>> come up right now with another attribute I’d like to see printed as decimal
>> (but I’m sure there are some).
>>
>
> You can probably add DW_AT_lower_bound, DW_AT_upper_bound, and DW_AT_count
> to the list of attributes that might make more sense as (signed) decimal.
> (just something I came across in a test case)
>
>
> Are you sure about the (signed) part? I have been working on a patch to
> add this, but now I’m wondering… What makes it a signed entity, I’m not
> seeing anything in the standard that mandates it. It’s not a big deal
> because anything we generate should be OK, but I thought I’d ask before
> submitting the pacth (which is actually not that obvious as we don’t have
> getAsSignedConstant() right now, and getting that tested isn’t that easy).
>

Well count would be unsigned, but upper & lower bound can be negative -
apparently fortran has user-specified array bounds, so you can have an
array with indices [-5, 5] for example, as far as I understand it (which is
not very far at all - so I'm happy to be corrected)

FWIW - GCC seems to produce an upper bound of 0xffffffff (or whatever it is
- max 32 or 64 bit int) for arrays of unknown bound (flexible array members
in c99) or arrays of zero length. I'm not sure what the right way to render
that is, but -1 /might/ be nicer than MAX_UINT. That's the only case I've
seen in person, the Fortran case is something I know about only in the
abstract/according to the DWARF spec.

(arguably all these things should use FORM_sint/uint or whatever they're
called, so they communicate the kind of representation they're using - it's
just when they use the old _dataN (which I think GCC still does for the
upper_bound of unknown/zero size that I mentioned) that we have to choose a
sensible default)


>
> Fred
>
>
>> (might be worth splitting the line number part of the patch out - it
>> might be easier (as you say, it's simply printing them as decimal rather
>> than hex - doesn't depend on access to the line table you're adding here,
>> etc) or it might be harder/different (if we end up changing the default &
>> trying to decide if there are places where that new default would be
>> problematic, etc))
>>
>>
>> I’ll do that once I finish the current rework.
>>
>> Fred
>>
>>
>>> Fred
>>>
>>>
>>>> http://reviews.llvm.org/D5192
>>>>
>>>> Files:
>>>>   lib/DebugInfo/DWARFContext.cpp
>>>>   lib/DebugInfo/DWARFDebugInfoEntry.cpp
>>>>   lib/DebugInfo/DWARFDebugInfoEntry.h
>>>>   lib/DebugInfo/DWARFDebugLine.cpp
>>>>   lib/DebugInfo/DWARFDebugLine.h
>>>>   test/DebugInfo/X86/2011-09-26-GlobalVarContext.ll
>>>>   test/DebugInfo/X86/fission-cu.ll
>>>>   test/DebugInfo/namespace.ll
>>>>   test/MC/MachO/gen-dwarf.s
>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141006/0cb15c1b/attachment.html>