[0/3] Move dwarf2_cu to its own file

Message ID 20210327195604.3371214-1-tom@tromey.com
Headers show
Series
  • Move dwarf2_cu to its own file
Related show

Message

Tom Tromey March 27, 2021, 7:56 p.m.
This short series moves dwarf2_cu to a new header, and then moves some
of the methods and helper functions to a new .c file as well.

This is just a minor cleanup coming from another series I'm working
on.  I think that in general we should be splitting up dwarf2/read.c.
It is still the largest source file in gdb by a large margin.

Regression tested on x86-64 Fedora 32.

Tom

Comments

Mike Frysinger via Gdb-patches March 28, 2021, 3:16 p.m. | #1
On 2021-03-27 3:56 p.m., Tom Tromey wrote:
> This short series moves dwarf2_cu to a new header, and then moves some

> of the methods and helper functions to a new .c file as well.

> 

> This is just a minor cleanup coming from another series I'm working

> on.  I think that in general we should be splitting up dwarf2/read.c.

> It is still the largest source file in gdb by a large margin.


I recently attempted to move everything index-reading-related from
dwarf2/read.c to other files:

- dwarf2/index-gdb.{c,h}
- dwarf2/index-debug-names.{c,h}
- dwarf2/index-common.{c,h}

I think this would be nice because the index-reading stuff makes a big
chunk of dwarf2/read.c.  And it's not always clear to me that some
functions with a "generic" name, like dw2_map_expand_apply, are only
used for index-reading.

The only problem was dw2_get_file_names, which uses cutu_reader to read
some DIEs... cutu_reader is local to dwarf2/read.c, and I was not sure
if it would be a good idea to try to export it.

I am wondering if you attempted to do this before.

Simon
Tom Tromey March 28, 2021, 5:42 p.m. | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:


Simon> I recently attempted to move everything index-reading-related from
Simon> dwarf2/read.c to other files:

Simon> - dwarf2/index-gdb.{c,h}
Simon> - dwarf2/index-debug-names.{c,h}
Simon> - dwarf2/index-common.{c,h}

Simon> I think this would be nice because the index-reading stuff makes a big
Simon> chunk of dwarf2/read.c.  And it's not always clear to me that some
Simon> functions with a "generic" name, like dw2_map_expand_apply, are only
Simon> used for index-reading.

Simon> The only problem was dw2_get_file_names, which uses cutu_reader to read
Simon> some DIEs... cutu_reader is local to dwarf2/read.c, and I was not sure
Simon> if it would be a good idea to try to export it.

Simon> I am wondering if you attempted to do this before.

I don't think I attempted that specifically, but I have taken stabs at
moving other bits out, without real success.

I find the DWARF reader kind of depressingly spaghetti-like.
cutu_reader seems to overlap with die_reader_specs and with dwarf2_cu
itself -- they are all objects created temporarily while reading in a
CU.  However, they have different lifetimes and are passed to different
things.  Parts of dwarf2_cu are used by psymtab creation, but parts only
by full CU expansion.  The cutu_reader startup code is difficult to
follow.  Etc.

I've tried a few times to clean this up, but there are a lot of
scenarios to handle (fission, dwz, debug_types) and it's hard to be sure
that one is doing the right thing.

I do have a bunch of small cleanup patches kicking around, though I
haven't prepared them for submission.  (For example I have some patches
to try to clean up the comp-unit-head stuff in dwarf2_per_cu_data, and
avoid all the redundant fields in that object; but here in the end I
think I convinced myself that the dwarf2_cu still needs a separate copy
due to Fission, and kind of lost heart for finishing the work.)


Lately I've been working on replacing the DWARF psymtab reader.  My goal
here is to speed up gdb startup, and so far this approach looks promising.

I wanted to make this new scanner multi-threaded, but due to the
cutu_reader / dwarf2_cu stuff, this looks a bit tricky.

I did take one step toward this by having gdb read all the abbrevs up
front, so there aren't abbrev table lifetime issues.  I don't know if
this is advisable, though.  Maybe it's better to have one abbrev table
cache per thread instead.

To tie this back to the current discussion, a lot of this code ended up
in dwarf2/read.c again, simply because of the need for a cutu_reader.
So, moving that out, if possible, would at least let us break up read.c
a little.

I think my ideal here would be that preparing to scan DIEs from a CU
would be simple.  There could be some kind of base class that the reader
(either the CU expansion reader, or the psymtab reader / new scanner)
could subclass.  This would take a dwarf2_per_cu_data parameter and the
lifetime handling would be up to the user.  (That is, no mandatory cache
aging stuff.)  No idea if this is practical.

Tom