[llvm] r296253 - [DebugInfo] Skip implicit_const attributes when dumping .debug_info. NFC.
Victor Leschuk via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 27 11:32:30 PST 2017
Ok than, will fix, provide tests and submit to phab.
On 02/27/2017 10:21 PM, Eric Christopher wrote:
> FWIW I agree completely with Dave here. We should show the constness
> of the data explicitly in the dump.
>
> Thanks!
>
> -eric
>
> On Mon, Feb 27, 2017 at 11:13 AM David Blaikie <dblaikie at gmail.com
> <mailto:dblaikie at gmail.com>> wrote:
>
> I tend to think it's more legible if we include the values there,
> even if no bytes were required to produce them. I don't generally
> look at the abbreviations when reading a dump & expect to see the
> data in the debug_info/debug_types output.
>
> ( +Adrian Prantl <mailto:aprantl at apple.com> +Eric Christopher
> <mailto:echristo at gmail.com> in case they've got thoughts on the
> subject)
>
>
> On Mon, Feb 27, 2017 at 11:09 AM Victor Leschuk
> <vleschuk at accesssoftek.com <mailto:vleschuk at accesssoftek.com>> wrote:
>
> Yeah, I see, I was wrong. I agree that we need to make
> behavior uniform here. However I still think we shouldn't
> include the data which is not really present into dump. It
> seems more logical for me to not print flag_present values either.
>
> What do you think?
>
> On 02/27/2017 09:54 PM, David Blaikie wrote:
>>
>>
>> On Mon, Feb 27, 2017 at 10:30 AM Victor Leschuk
>> <vleschuk at accesssoftek.com
>> <mailto:vleschuk at accesssoftek.com>> wrote:
>>
>> I think flag_present and implicit_const attributes
>> shouldn't be treated as similar here: flag_present
>> attributes physically do have a record in .debug_info
>> section,
>>
>>
>> I think one of us is confused, because my understanding is
>> that flag_present attributes do not have any bit/bytes in the
>> debug_info representation.
>>
>> To take the same example as before and look at the assembly:
>>
>> this is the debug_info assembly for that subprogram:
>>
>> .byte 2 # Abbrev [2] 0x2a:0x19
>> DW_TAG_subprogram
>> .quad .Lfunc_begin0 # DW_AT_low_pc
>> .long .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
>> .byte 1 # DW_AT_frame_base
>> .byte 86
>> .long .Linfo_string3 # DW_AT_linkage_name
>> .long .Linfo_string4 # DW_AT_name
>> .byte 1 # DW_AT_decl_file
>> .byte 1 # DW_AT_decl_line
>> # DW_AT_external
>> .byte 0 # End Of Children Mark
>>
>> Note that no bytes were produced for the DW_AT_external
>> (using DW_FORM_flag_present in the abbrev) attribute.
>>
>> From the DWARF4 spec:
>>
>> "A flag is represented explicitly as a single byte of data
>> (DW_FORM_flag) or implicitly (DW_FORM_flag_present). In the
>> first case, if the flag has value zero, it indicates the
>> absence of the attribute; if the flag has a non-zero value,
>> it indicates the presence of the attribute. In the second
>> case, the attribute is implicitly indicated as present, and
>> no value is encoded in the debugging information entry itself."
>>
>> while implicit_const attributes are not physically
>> mentioned there. Printing implicit_const values when
>> dumping .debug_info can be more convenient for reader but
>> it will actually violate the definition of dump. When
>> dumping section contents we expect to see what is
>> actually there, don't we?
>>
>>
>> On 02/27/2017 09:04 PM, David Blaikie wrote:
>>>
>>>
>>> On Mon, Feb 27, 2017 at 10:00 AM Victor Leschuk
>>> <vleschuk at accesssoftek.com
>>> <mailto:vleschuk at accesssoftek.com>> wrote:
>>>
>>>
>>>
>>> On 02/27/2017 08:04 PM, David Blaikie wrote:
>>>> Skipping these values doesn't seem to be correct -
>>>> in the case of flag_present, I think the value is
>>>> still rendered in the dump, is it not?
>>> Do you mean that the value is somehow mentioned in
>>> .debug_info section? It is not.
>>>
>>>
>>> Looks to me like it is:
>>>
>>> dumping debug info for this code: void f1() { }
>>>
>>> produces this:
>>>
>>> .debug_abbrev contents:
>>> ...
>>> [2] DW_TAG_subprogram DW_CHILDREN_no
>>> ...
>>> DW_AT_external DW_FORM_flag_present
>>>
>>> .debug_info contents:
>>> ...
>>> 0x0000002a: DW_TAG_subprogram [2]
>>> ...
>>> DW_AT_external [DW_FORM_flag_present]
>>> (true)
>>>
>>> I would expect similar behavior for const_value - that
>>> the attribute and its constant value is printed in the
>>> debug_info dump.
>>>
>>>
>>>> I would expect the same for implicit_const values.
>>>> (also, this needs test coverage either way)
>>> Yes, you are correct. I will create tests when we
>>> decide what's right behavior here.
>>>
>>> I think skipping is correct here: we are dumping
>>> .debug_info data based on attributes stored in
>>> AbbrevDecl, this code was based on assumption that
>>> every attribute has representation in .debug_info
>>> section, that was before implicit_const form was
>>> introduced.
>>>
>>>>
>>>> On Sat, Feb 25, 2017 at 5:27 AM Victor Leschuk via
>>>> llvm-commits <llvm-commits at lists.llvm.org
>>>> <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>>
>>>> Author: vleschuk
>>>> Date: Sat Feb 25 07:15:57 2017
>>>> New Revision: 296253
>>>>
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project?rev=296253&view=rev
>>>> Log:
>>>> [DebugInfo] Skip implicit_const attributes when
>>>> dumping .debug_info. NFC.
>>>>
>>>> When dumping .debug_info section we loop
>>>> through all attributes mentioned in
>>>> .debug_abbrev section and dump values using
>>>> DWARFFormValue::extractValue().
>>>> We need to skip implicit_const attributes here
>>>> as their values are not
>>>> really located in .debug_info but directly in
>>>> .debug_abbrev. This patch fixes
>>>> triggered assert() in
>>>> DWARFFormValue::extractValue() caused by trying to
>>>> access implicit_const values from .debug_info.
>>>>
>>>> Modified:
>>>> llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp
>>>>
>>>> Modified:
>>>> llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp?rev=296253&r1=296252&r2=296253&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp
>>>> (original)
>>>> +++ llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp
>>>> Sat Feb 25 07:15:57 2017
>>>> @@ -329,6 +329,12 @@ void
>>>> DWARFDie::dump(raw_ostream &OS, uns
>>>>
>>>> // Dump all data in the DIE for the
>>>> attributes.
>>>> for (const auto &AttrSpec :
>>>> AbbrevDecl->attributes()) {
>>>> + if (AttrSpec.Form ==
>>>> DW_FORM_implicit_const) {
>>>> + // We are dumping .debug_info
>>>> section ,
>>>> + // implicit_const attribute values
>>>> are not really stored here,
>>>> + // but in .debug_abbrev section.
>>>> So we just skip such attrs.
>>>> + continue;
>>>> + }
>>>> dumpAttribute(OS, *this, &offset,
>>>> AttrSpec.Attr, AttrSpec.Form,
>>>> Indent);
>>>> }
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> <mailto:llvm-commits at lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>> --
>>> Best Regards,
>>> Victor
>>>
>>
>> --
>> Best Regards,
>> Victor
>>
>
> --
> Best Regards,
> Victor
>
--
Best Regards,
Victor
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170227/f2d5e8aa/attachment.html>
More information about the llvm-commits
mailing list