Re: [PATCH] dwarf: Make sect_offset 64-bits

[Simon Marchi <simon.marchi@ericsson.com>]

Hi Pedro,

They are all very good questions, I should have given that info in the first
place, but here it is.

On 2018-02-20 11:49 AM, Pedro Alves wrote:
> Does this change the type of any field of the structures that form the
> indexes?  I.e., does it affect binary compatibility there?  I.e.,
> can you save a gdb index with pre-patch gdb, and load it back in
> post-patch gdb.  Likewise viceversa.  Also DWARF5 indexes.

I generated both kinds of indexes (using the gdb binary itself as a guinea pig)
with and without the patch applied, all produced files had the same checksum in
both cases.  I don't think we ever use sizeof on sect_offset or a variable of
that type, so its size should not change anything (except cause an overflow
if it's 32 bits and we need it to be larger).

All reads/writes I could find were either with an hardcoded number of bytes,
or a number of bytes based of whether we're using dwarf64 or not (4 or
8 bytes).

> Also, just for the record, can we assume that this doesn't increase
> memory usage considerably when debugging bigger programs?  I assume that
> this will create some padding holes in some structures and that we can
> probably win back the memory by just changing order of some fields
> for better packing.

Indeed it adds a few bytes to some structures.  The numbers are different
if you compile GDB as a 64 bits or 32 bits program.  Some structures have
no holes on 32 bits have holes on 64 bits (because of the different pointer
size/alignment), and a bigger sect_offset field just fills that hole, so
there's no net increase.  In other cases, the bigger sect_offset field just
take the place of some padding.  For example, this structure:

struct foo
{
  uint64_t a;
  sect_offset b;
};

takes 16 bytes when built on 64 bits, regardless of the size of sect_offset.
Here are some tables that summarize the impact.  The notation is S/H, where
S is the size of the structure in bytes (including padding), and H is the
total size of holes (excluding padding at the end, because ptype/o does not
show it).  This value is expressed as B.b, where B is the number of integral
bytes and b the number of bits (up to 7).

64 bits:
                                 before   after
                 comp_unit_head  48/0     64/8
             dwarf2_per_cu_data  56/5.2   56/1.2
                signatured_type  96/5.2   104/5.2
                 stmt_list_hash  16/0     16/0
                       dwo_unit  40/0     40/0
                    line_header  104/8.7  112/12.7
               partial_die_info  96/4.1   104/4.1
                   abbrev_table  104/4    104/0
                       die_info  56/4.6   56/0.6
               tu_abbrev_offset  16/0     16/0
  dwarf2_per_cu_offset_and_type  24/4     24/0


32 bits:
                                 before   after
                 comp_unit_head  44/0     52/0
             dwarf2_per_cu_data  32/1.2   36/1.2
                signatured_type  60/1.2   68/1.2
                 stmt_list_hash  8/0      12/0
                       dwo_unit  28/0     32/0
                    line_header  64/8.7   72/8.7
               partial_die_info  52/0.1   64/0.1
                   abbrev_table  52/0     56/0
                       die_info  36/0.6   40/0.6
               tu_abbrev_offset  8/0      12/0
  dwarf2_per_cu_offset_and_type  12/0     16/0

Among those, I think that partial_die_info and die_info are probably the
most critical, since they are allocated the most often.  Loading gdb in gdb
allocates about 2023000 partial_die_info.  Loading with -readnow allocates
about 7479000 die_info.  However, I think that those two structures are not
kept around for very long, only while reading in the CU, so the increase in
memory usage is not really noticeable.  I compared the memory usage after
having read the symbols from the gdb binary (both with and without -readnow),
and it seemed the same with and without the patch.  comp_unit_head and
line_header have a similar lifetime it seems.

signatured_type instances are allocated on the objfile obstack, so they last
for a long time.  I don't think in practice you can have enough types described
by signatured_type for it to make a difference though.

> Offhand, I know about code in dwarf2read.c that assumes that offset_type
> is 32-bit for both of the reasons above -- memory usage (e.g.,
> struct name_component), and also as type used in indexes.
> I'm not seeing anything for sect_offset, but can you double check?

Simon