[llvm] r296253 - [DebugInfo] Skip implicit_const attributes when dumping .debug_info. NFC.

Victor Leschuk via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 28 00:55:25 PST 2017


Done: https://reviews.llvm.org/D30448


On 02/27/2017 10:32 PM, Victor Leschuk via llvm-commits wrote:
>
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 
Best Regards,
Victor

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170228/848ae2ec/attachment.html>


More information about the llvm-commits mailing list