[Lldb-commits] [PATCH] D58330: 01/03: new SectionPart for Section subranges (for effective .debug_types concatenation)

Mon Feb 18 00:28:57 PST 2019

jankratochvil added a comment.

In D58330#1400812 <https://reviews.llvm.org/D58330#1400812>, @labath wrote:

> It also appears that we will still end up mmapping the object file, and then copying all of the debug info out of it into a heap buffer.

In all usual cases at least on Linux LLDB should use just the mmap part, without any copying - see the block on line 691 of `SymbolFileDWARF::get_debug_info_data()`: https://reviews.llvm.org/D51578#C1361566NL682

The question is how hard to optimize the corner cases of `IsInMemory()`, `SHF_COMPRESSED` with `.debug_types` etc.  Originally I did not optimize it but I think this is what @clayborg did not like: https://reviews.llvm.org/D51578#1383458

> What's the reason we're trying so hard to concatenate things? IIRC, it was because it makes things appear DWARF5-like, but this is now creating a lot of infrastructure that will be completely unused in the dwarf5 case, so I think we're missing that goal.

In part yes, I agree. I cannot much agree with the "//completely unused in the dwarf5 case//" as for DWZ I need the same so if we do not implement it for `.debug_types` I need to implement it for DWZ anyway. Which is why I also do that - to justify some way such refactorization for DWZ. Red Hat does not use `.debug_types` and DWARF-5 does not have this problem anyway as you say.

Currently DWZ solves it on line 164 - DWZRedirect(): https://reviews.llvm.org/D40474#C1183882NL164
Although there the two sections to be merged (main `.debug_info` and `.debug_info` from DWZ common file - from `/usr/lib/debug/.dwz/` directory) come from two different files.  I haven't coded that yet.
Currently DWZ also needs D40473 <https://reviews.llvm.org/D40473> full of `GetMainDWARF()` vs. original `GetDWARF()` and I hope I could drop this by also merging `.debug_abbrev` (and maybe some others).  But I haven't yet tried to rebase the DWZ patchset on top of this merging patchset if there isn't some catch.

>From the higher point of view: `.debug_types` is not needed thanks to DWARF-5 and Red Hat could replace DWZ with `-fdebug-types-section` or at least just not to use DWZ `-m|--multifile` option (the DWZ common files so we would stay at just one file). But then we live in a real world and people want to debug existing code built with `.debug_types` and RHELs deployed out there do use DWZ with `-m|--multifile`. And both do work in GDB now.

> Can't we just admit that we are dealing with two sections here, and use one bit from the user_id_t (or whereever it's needed) to tell which one are we talking about?

Been there, done that - with `offset_t` (although by offseting it and not by one bit but that is similar) - the `FORWARDER()` dispatching of `DWARFDataExtractor`: https://reviews.llvm.org/D51578?id=170342#C1214469NL1
The problem is that it has some percents of performance impact which @clayborg did not like.
I think `user_id_t` is too high for this functionality as all the inter-DWARF references already use only `offset_t` inside the DWARF code.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58330