llvm dev - Jun 2018 - [lldb-dev] Adding DWARF5 accelerator table support to llvm

> On Jun 13, 2018, at 11:18 AM, Jonas Devlieghere via lldb-dev <lldb-dev
at lists.llvm.org> wrote:
> 
> Hi Pavel,
> 
>> On Jun 13, 2018, at 6:56 AM, Pavel Labath <labath at google.com
<mailto:labath at google.com>> wrote:
>> 
>> Hello again,
>> 
>> It's been nearly six months since my first email, so it's a
good time
>> to recap what has been done here so far. I am happy to report that
>> stages 1-3 (i.e. producer/consumer in llvm and integration with lldb)
>> of my original plan are now complete with one caveat.
> 
> Awesome!
> 
>> The caveat is that the .debug_names section is presently not a full
>> drop-in replacement for the .apple_*** sections. The reason for that
>> is that there is no equivalent to the .apple_objc section (which links
>> an objc class/category name  to all of its methods). I did not
>> implement that, because I do not see a way to embed that kind of
>> information to this section without some sort of an extension. Given
>> that this was not required for my use case, I felt it would be best to
>> leave this to the people working on objc support (*looks at Jonas*) to
>> work out the details of how to represent that.
> 
> Definitely :-) I plan to start working on this (+ dsymutil support) very
soon.
It would be great to add "dsymutil --update" support so it can handle
both mach-o files and ELF files. What this does is update the accelerator tables
of any files that are specified. That way the accelerator tables can be added to
any object file that contains DWARF after the fact.

As David Blaike suggested, maybe we can emit the .apple_objc section as a work
around for now.
> 
>> Nonetheless, I believe that the emitted .debug_names section contains
>> all the data that is required by the standard, and it is sufficient to
>> pass all tests in the lldb integration test suite on linux (this
>> doesn't include objc tests).
> 
> How did you (or do you plan to) add this (and DWARF5 in general) in the
lldb test suite? It sounds like this might require a new dimension in the test
matrix if we want to test all the variants with both Apple and DWARF style
accelerator tables.
> 
>> Simple benchmarks also show a large
>> performance improvement.I have some numbers to illustrate that
>> (measurements taken by using a release build of lldb to debug a debug
>> build of clang, clang was built with -mllvm -accel-tables=Dwarf to
>> enable the accelerator generation, usage of the tables was controlled
>> by a setting in lldb):
>> - setting a breakpoint on a non-existing function without the use of
>> accelerator tables:
>> real    0m5.554s
>> user    0m43.764s
>> sys     0m6.748s
>> (The majority of this time is spend on building a debug info index,
>> which is a one-shot thing. subsequent breakpoints would be fast)
>> 
>> - setting a breakpoint on a non-existing function with accelerator
tables:
>> real    0m3.517s
>> user    0m3.136s
>> sys     0m0.376s
>> (With the index already present, we are able to quickly determine that
>> there is no match and finish)
>> 
>> - setting a breakpoint on all "dump" functions without the
use of
>> accelerator tables:
>> real    0m21.544s
>> user    0m59.588s
>> sys     0m6.796s
>> (Apart from building the index, now we must also parse a bunch of
>> compile units and line tables to resolve the breakpoint locations)
>> 
>> - setting a breakpoint on all "dump" functions with
accelerator tables:
>> real    0m23.644s
>> user    0m22.692s
>> sys     0m0.948s
>> (Here we see that this extra work is actually the bottleneck now.
>> Preliminary analysis shows that majority of this time is spend
>> inserting line table entries into the middle of a vector, which means
>> it should be possible to fix this with a smarter implementation).
>> 
>> As far as object file sizes go, in the resulting clang binary (2.3GB),
>> the new .debug_names section takes up about 160MB (7%), which isn't
>> negligible, but considering that it supersedes the
>> .debug_pubnames/.debug_pubtypes tables whose combined size is 490MB
>> (21% of the binary), switching to this table (and dropping the other
>> two) will have a positive impact on the binary size. Further
>> reductions can be made by merging the individual indexes into one
>> large index as a part of the link step (which will also increase
>> debugger speed), but it's hard to quantify the exact impact of
that.
>> 
>> With all of this in mind, I'd like to encourage you to give the new
>> tables a try. All you need to do is pass -mllvm -accel-tables=Dwarf to
>> clang while building your project. lldb should use the generated
>> tables automatically. I'm particularly interested in the interop
>> scenario. I've checked that readelf is able to make sense of the
>> generated tables, but if you have any other producer/consumer of these
>> tables which is independent of llvm, I'd like to know whether we
are
>> compatible with it.
> 
> I know of one internal consumer but it ignores the accelerator tables so I
don’t expect any issues there.
> 
>> I'd also like to make the new functionality more easily accessible
to
>> users. I am not sure what our policy here is, but I was thinking of
>> either including this functionality in -glldb (on non-apple targets);
>> or by adding a separate -g flag for it (-gdebug-names-section?), with
>> the goal of eventual inclusion into -glldb. I exclude apple targets
>> because: a) they already have a thing that works and the lack of
>> .apple_objc would be a pessimization; b) the different debug info
>> distribution model means it requires more testing and code (dsymutil).
> 
> Changing the default on non-Apple targets sounds good. Once we have
Objective-C support I’ll do some additional (internal) testing after which we
can consider flipping the switch globally.
> 
>> For other targets this should bring a big performance improvement when
>> debugging with lldb. The lack of .apple_objc will mean lldb will have
>> to either index the objc compile units manually, or implement a more
>> complicated lookup using other information in the section. However,
>> Objective C is not that widespread outside of apple platforms, so the
>> impact of this should be minimal.
>> 
>> What do you think?
>> 
>> regards,
>> pavel
> 
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180613/23062f84/attachment.html>

Thank you all. I am going to try to reply to all comments in a single email.

Regarding the  .apple_objc idea, I am afraid the situation is not as
simple as just flipping a switch. (If it was, I don't think I would
have embarked on this adventure in the first place -- I would just
emit .apple_*** everywhere and call it done :)). The issue is that the
apple tables have assumptions about the macos debug info distribution
model hardcoded in them -- they assume they will either stay in the .o
file or be linked by a smart debug-info-aware linker (dsymutil). In
particular, this means they are not self-delimiting (no length field
as is typical for other dwarf artifacts), so if a linker which is not
aware of them would simply concatenate individual .o tables (which elf
linkers are really good at), the debugger would have no way to pry
them apart. And even if it somehow managed that, it still wouldn't
know if the indexes covered all of the compile units in the linked
file or only some of them (in case some of the object files were
compiled with the tables and some without).

In light of that, I don't think it's worth trying to combine
.apple_objc with .debug_names in some way, and it would be much
simpler to just extend .debug_names with the necessary information. I
think the simplest way of achieving this (one which would require
least amount of standard-bending) is to take the index entry for the
objc class and add a special attribute to it (DW_IDX_method_list?)
with form DW_FORM_blockXXX and just have the references to the method
DIEs in the block data. This should make the implementation an almost
drop-in for the current .apple_objc functionality (we would still need
to figure out what to do with category methods, but it's not clear to
me whether lldb actually uses those anywhere).

But, other options may be possible as well. What's not clear to me is
whether these tables couldn't be replaced by extra information in the
.debug_info section. It seems to me that these tables are trying to
work around the issue that there is no straight way to go from a
DW_TAG_structure type DIE describing an ObjC class to it's methods. If
these methods (their forward declarations) were be present as children
of the type DIE (as they are for c++ classes), then these tables may
not be necessary. But maybe (probably) that has already been
considered and deemed infeasible for some reason. In any case this
seemed like a thing best left for people who actually work on ObjC
support to figure out.


As far as the .debug_names size goes, I should also point out that the
binary in question was built with -fno-limit-debug-info, which isn't a
default setup on linux. I have tried measuring the sizes without that
flag and with fission enabled (-gsplit-dwarf) and the results are:
without compression:
- clang binary: 960 MB
- .debug_names: 130 MB (13%)
- debug_pubnames: 175 MB (18%)
- debug_pubtypes: 204 MB (21%)
- median time for setting a breakpoint on non-existent function
(variance +/- 2%):
real 0m3.526s
user 0m3.156s
sys 0m0.364s

with -Wl,--compress-debug-sections=zlib:
- clang binary: 440 MB
- .debug_names: 80MB (18%)
- .debug_pubnames: 31 MB (7.2%)
- .debug_pubtypes: 42MB (9.5%)
- median time for setting a breakpoint on non-existent function:
real 0m4.369s
user 0m3.948s
sys 0m0.416s

So, .debug_names indeed compresses worse than .debug_pubnames/types,
but that is not surprising as it has a more condensed encoding to
begin with (no inline strings). However, even in it's compressed form
its size is only slightly larger that the two other sections combined
(while being infinitely more useful). As for the compression, my
takeaway from this is that compression definitely has a measurable
impact on startup time, but, on the grand scale of things, the impact
is not actually that big. And if a user deliberately adds the
compression flag to his command line, I would assume he really cares
about binary size, and is willing to sacrifice some debug performance
in return. So, I would honor his request and compress .debug_names as
well.


I have tried David Anderson's dwarfdump (after Paul pointed it out to
me), but as far as I can tell, it has no support from printing out the
.debug_names section (the print_debug_names function is stubbed out).
**I think** I got the correct source repository
(git://git.code.sf.net/p/libdwarf/code) as the last commit there is
dated yesterday.


For testing on the lldb side I have been deliberately trying to avoid
adding another dimensions to the ever-growing test matrix. I don't
think this functionality is worth it, especially not if you view the
test suite as a regression test suite. The entire functionality of
this in lldb is encompassed in a single .cpp file which is about 250
LOC. The class has about a dozen entry points and most of them are
accessible through the lldb-test tool, which I've used to write
targeted regression tests for this (it could probably use more of
those). I did use the "dotest" suite as an integration test suite, but
I did that by simply passing --env CFLAGS_EXTRAS="-mllvm
-accel-tables=Dwarf" to dotest (I also tried hacking clang to always
emit the new tables to make sure I'm not missing anything).
Ironically, if you try that now, you will see one test failing, but
that's because I have already added one test passing that flag
explicitly (I couldn't find a way to test this functionality through
lldb-test) and clang then complains about a duplicate argument. This
should go away once we have better -g flag to control this behavior. I
haven't yet figured out whether I want to set up a bot to run the
tests in this configuration, but I know I don't want to inflict that
extra overhead on developers running tests during day-to-day
development.

regards,
pavel

> On Jun 14, 2018, at 7:01 AM, Pavel Labath via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Thank you all. I am going to try to reply to all comments in a single
email.
> 
> Regarding the  .apple_objc idea, I am afraid the situation is not as
> simple as just flipping a switch.
Jonas is currently working on adding the support for DWARF5-style Objective-C
accelerator tables to LLVM/LLDB/dsymutil. Based on the assumption that DWARF 4
and earlier are unaffected by any of this, I don't think it's necessary
to spend any effort of making the transition smooth. I'm fine with having
Objective-C on DWARF 5 broken on trunk for two weeks until Jonas is done adding
Objective-C support to the DWARF 5 implementation.

> (If it was, I don't think I would
> have embarked on this adventure in the first place -- I would just
> emit .apple_*** everywhere and call it done :)). The issue is that the
> apple tables have assumptions about the macos debug info distribution
> model hardcoded in them -- they assume they will either stay in the .o
> file or be linked by a smart debug-info-aware linker (dsymutil). In
> particular, this means they are not self-delimiting (no length field
> as is typical for other dwarf artifacts), so if a linker which is not
> aware of them would simply concatenate individual .o tables (which elf
> linkers are really good at), the debugger would have no way to pry
> them apart. And even if it somehow managed that, it still wouldn't
> know if the indexes covered all of the compile units in the linked
> file or only some of them (in case some of the object files were
> compiled with the tables and some without).
> 
> In light of that, I don't think it's worth trying to combine
> .apple_objc with .debug_names in some way, and it would be much
> simpler to just extend .debug_names with the necessary information. I
> think the simplest way of achieving this (one which would require
> least amount of standard-bending) is to take the index entry for the
> objc class and add a special attribute to it (DW_IDX_method_list?)
> with form DW_FORM_blockXXX and just have the references to the method
> DIEs in the block data. This should make the implementation an almost
> drop-in for the current .apple_objc functionality (we would still need
> to figure out what to do with category methods, but it's not clear to
> me whether lldb actually uses those anywhere).
> 
> But, other options may be possible as well. What's not clear to me is
> whether these tables couldn't be replaced by extra information in the
> .debug_info section. It seems to me that these tables are trying to
> work around the issue that there is no straight way to go from a
> DW_TAG_structure type DIE describing an ObjC class to it's methods. If
> these methods (their forward declarations) were be present as children
> of the type DIE (as they are for c++ classes), then these tables may
> not be necessary. But maybe (probably) that has already been
> considered and deemed infeasible for some reason. In any case this
> seemed like a thing best left for people who actually work on ObjC
> support to figure out.
That's really a question for Greg or Jim — I don't know why the current
representation has the Objective-C methods outside of the structs. One reason
might be that an interface's implementation can define more methods than are
visible in its public interface in the header file, but we already seem to be
aware of this and mark the implementation with DW_AT_APPLE_objc_complete_type. I
also am not sure that this is the *only* reason for the objc accelerator table.
But I'd like to learn.

-- adrian
> As far as the .debug_names size goes, I should also point out that the
> binary in question was built with -fno-limit-debug-info, which isn't a
> default setup on linux. I have tried measuring the sizes without that
> flag and with fission enabled (-gsplit-dwarf) and the results are:
> without compression:
> - clang binary: 960 MB
> - .debug_names: 130 MB (13%)
> - debug_pubnames: 175 MB (18%)
> - debug_pubtypes: 204 MB (21%)
> - median time for setting a breakpoint on non-existent function
> (variance +/- 2%):
> real 0m3.526s
> user 0m3.156s
> sys 0m0.364s
> 
> with -Wl,--compress-debug-sections=zlib:
> - clang binary: 440 MB
> - .debug_names: 80MB (18%)
> - .debug_pubnames: 31 MB (7.2%)
> - .debug_pubtypes: 42MB (9.5%)
> - median time for setting a breakpoint on non-existent function:
> real 0m4.369s
> user 0m3.948s
> sys 0m0.416s
> 
> So, .debug_names indeed compresses worse than .debug_pubnames/types,
> but that is not surprising as it has a more condensed encoding to
> begin with (no inline strings). However, even in it's compressed form
> its size is only slightly larger that the two other sections combined
> (while being infinitely more useful). As for the compression, my
> takeaway from this is that compression definitely has a measurable
> impact on startup time, but, on the grand scale of things, the impact
> is not actually that big. And if a user deliberately adds the
> compression flag to his command line, I would assume he really cares
> about binary size, and is willing to sacrifice some debug performance
> in return. So, I would honor his request and compress .debug_names as
> well.
> 
> 
> I have tried David Anderson's dwarfdump (after Paul pointed it out to
> me), but as far as I can tell, it has no support from printing out the
> .debug_names section (the print_debug_names function is stubbed out).
> **I think** I got the correct source repository
> (git://git.code.sf.net/p/libdwarf/code) as the last commit there is
> dated yesterday.
> 
> 
> For testing on the lldb side I have been deliberately trying to avoid
> adding another dimensions to the ever-growing test matrix. I don't
> think this functionality is worth it, especially not if you view the
> test suite as a regression test suite. The entire functionality of
> this in lldb is encompassed in a single .cpp file which is about 250
> LOC. The class has about a dozen entry points and most of them are
> accessible through the lldb-test tool, which I've used to write
> targeted regression tests for this (it could probably use more of
> those). I did use the "dotest" suite as an integration test
suite, but
> I did that by simply passing --env CFLAGS_EXTRAS="-mllvm
> -accel-tables=Dwarf" to dotest (I also tried hacking clang to always
> emit the new tables to make sure I'm not missing anything).
> Ironically, if you try that now, you will see one test failing, but
> that's because I have already added one test passing that flag
> explicitly (I couldn't find a way to test this functionality through
> lldb-test) and clang then complains about a duplicate argument. This
> should go away once we have better -g flag to control this behavior. I
> haven't yet figured out whether I want to set up a bot to run the
> tests in this configuration, but I know I don't want to inflict that
> extra overhead on developers running tests during day-to-day
> development.
> 
> regards,
> pavel
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev