[PATCH] D137882: [DWARFLibrary] Add support to re-construct cu-index

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:868-871
+  // If we are parsing TU-index and for .debug_types section we don't need
+  // to do anything.
+  if (isParseSuccessful && TUIndex->getVersion() != 2)
+    fixupIndex(*DObj, *this, *TUIndex.get());
----------------
ayermolo wrote:
> dblaikie wrote:
> > ayermolo wrote:
> > > dblaikie wrote:
> > > > Any idea what this comment/code were about checking the type index version? I don't see any reason this fixup wouldn't be relevant to DWARFv4 type unit indexes - though the fixup code would have to take the section to parse as a parameter, since it's currently hardcoded to the debug_info sections, not the debug_types sections.
> > > I don't quite remember why. I think it was to narrow the scope.
> > > Also don't we also need to call forEachTypesDWOSections if we want to handle .debug_types section?
> > > Why do you see overflows in that section also?
> > We aren't having an issue here - since it's only DWARFv4+type units, and we're using DWARFv5 these days.
> > 
> > I've been looking at adding DWARFv5 overflow recovery (which can be more robust than DWARFv4, since DWARFv5 can get the DWOID/type signature without needing the abbrev section) and just looking more closely at this code/seeing these quirks & wanted to understand it better.
> > 
> > But, yeah, maybe just not worth supporting *shrug* no worries, thanks!
> Hopefully we will be moving to DWARF5 his year also. :D  
> The code should work with DWARF5 + debug types, but you are right it could be made more robust by specializing and taking advantage of DWO ID being in a header. Not sure it's worth it either. 
> 
> Also I am not sure how often debug-types are used with DWARF5 since one has to choose between them and .debug_names. Although looking at https://reviews.llvm.org/D49420 should be easy to turn on now that llvm generates DWARF5 type units.
> Hopefully we will be moving to DWARF5 his year also. :D

*fingers crossed*

> The code should work with DWARF5 + debug types, but you are right it could be made more robust by specializing and taking advantage of DWO ID being in a header. Not sure it's worth it either.

You mean DWARF4+debug types? It does already work with DWARF5 and debug types.

> Also I am not sure how often debug-types are used with DWARF5 since one has to choose between them and .debug_names. Although looking at https://reviews.llvm.org/D49420 should be easy to turn on now that llvm generates DWARF5 type units.

It's what we're doing at Google, at least - we're still using gdb_index, not lldb - and when we use lldb it's without any index solution, so, yeah, slow... 

Hadn't realized there was that outstanding patch for debug_names for type units. I'll have to take a look... 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137882/new/

https://reviews.llvm.org/D137882