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

[Victor Leschuk via llvm-commits]

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, 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170227/4c80162e/attachment.html>