[PATCH] D26567: Improve DWARF parsing speed by improving DWARFAbbreviationDeclaration

[Greg Clayton via llvm-commits]

clayborg added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp:68
+          ++FixedAttributeSize->NumAddrs;
+        else if (F == DW_FORM_ref_addr)
+          ++FixedAttributeSize->NumRefAddrs;
----------------
probinson wrote:
> Is there some reason you're only considering FORM_ref_addr here?  I'd think you would want to count all of the format-dependent reference forms here (ref_addr, strp, sec_offset, and all the similar new ones in DWARF 5).  I'd especially think FORM_strp would be important as that's used by pretty much every abbreviation that has a DW_AT_name.
Yep, I need to fix this. Good catch.


================
Comment at: lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp:68
+          ++FixedAttributeSize->NumAddrs;
+        else if (F == DW_FORM_ref_addr)
+          ++FixedAttributeSize->NumRefAddrs;
----------------
clayborg wrote:
> probinson wrote:
> > Is there some reason you're only considering FORM_ref_addr here?  I'd think you would want to count all of the format-dependent reference forms here (ref_addr, strp, sec_offset, and all the similar new ones in DWARF 5).  I'd especially think FORM_strp would be important as that's used by pretty much every abbreviation that has a DW_AT_name.
> Yep, I need to fix this. Good catch.
Actually isn't DW_FORM_ref_addr special? It is AddrSize in DWARF 2, and in DWARF 3 and later it is 4 bytes for DWARF32 and 8 for DWARF64. So I need an extra field in FixedSizeInfo that counts the DWARF32/DWARF64 only variants. Am I correct in that the following:

```
    case DW_FORM_strp:
    case DW_FORM_GNU_ref_alt:
    case DW_FORM_GNU_strp_alt:
    case DW_FORM_line_strp:
    case DW_FORM_sec_offset:
    case DW_FORM_strp_sup:
    case DW_FORM_ref_sup:
```

They are not AddrSize in DWARF 2 right? It just switches off of DWARF32/DWARF64.



https://reviews.llvm.org/D26567