Re: [PATCH 4/8] Store the comp_unit instead of the FDE table

["Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>]

On Sat, Feb 8, 2020, 10:28 Tom Tromey <tom@tromey.com> wrote:

> This changes the DWARF frame code to store the comp_unit on the
> objfile, rather than storing the FDE table.  It also changes the
> comp_unit to be heap-allocated using "new".
>
> This change makes it simpler for a later patch to add a field to the
> comp_unit, and to have deallaction work properly.  This in turn is
> important for making the frame data be independent of the objfile.
>
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
>
>         * dwarf2/frame.c (struct comp_unit): Add initializers.
>         (dwarf2_frame_objfile_data): Store a comp_unit.
>         (dwarf2_frame_find_fde): Update.
>         (dwarf2_build_frame_info): Use "new".
>
> Change-Id: Id339892824caf47e7909ed5ee7e2f0556d645dba
> ---
>  gdb/ChangeLog      |  7 +++++++
>  gdb/dwarf2/frame.c | 43 ++++++++++++++++++++-----------------------
>  2 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index 6d87e598345..0e74b8e7e68 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -137,24 +137,27 @@ typedef std::vector<dwarf2_fde *> dwarf2_fde_table;
>  struct comp_unit
>  {
>    /* Keep the bfd convenient.  */
> -  bfd *abfd;
> +  bfd *abfd = nullptr;
>
> -  struct objfile *objfile;
> +  struct objfile *objfile = nullptr;
>
>    /* Pointer to the .debug_frame section loaded into memory.  */
> -  const gdb_byte *dwarf_frame_buffer;
> +  const gdb_byte *dwarf_frame_buffer = nullptr;
>
>    /* Length of the loaded .debug_frame section.  */
> -  bfd_size_type dwarf_frame_size;
> +  bfd_size_type dwarf_frame_size = 0;
>
>    /* Pointer to the .debug_frame section.  */
> -  asection *dwarf_frame_section;
> +  asection *dwarf_frame_section = nullptr;
>
>    /* Base for DW_EH_PE_datarel encodings.  */
> -  bfd_vma dbase;
> +  bfd_vma dbase = 0;
>
>    /* Base for DW_EH_PE_textrel encodings.  */
> -  bfd_vma tbase;
> +  bfd_vma tbase = 0;
> +
> +  /* The FDE table.  */
> +  dwarf2_fde_table fde_table;
>  };
>
>  static struct dwarf2_fde *dwarf2_frame_find_fde (CORE_ADDR *pc,
> @@ -1467,7 +1470,7 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
>    return get_frame_base (this_frame);
>  }
>
> -const struct objfile_key<dwarf2_fde_table> dwarf2_frame_objfile_data;
> +const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
>
>
>
> @@ -1630,18 +1633,18 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR
> *out_offset)
>  {
>    for (objfile *objfile : current_program_space->objfiles ())
>      {
> -      dwarf2_fde_table *fde_table;
>        CORE_ADDR offset;
>        CORE_ADDR seek_pc;
>
> -      fde_table = dwarf2_frame_objfile_data.get (objfile);
> -      if (fde_table == NULL)
> +      comp_unit *unit = dwarf2_frame_objfile_data.get (objfile);
> +      if (unit == NULL)
>         {
>           dwarf2_build_frame_info (objfile);
> -         fde_table = dwarf2_frame_objfile_data.get (objfile);
> +         unit = dwarf2_frame_objfile_data.get (objfile);
>         }
> -      gdb_assert (fde_table != NULL);
> +      gdb_assert (unit != NULL);
>
> +      dwarf2_fde_table *fde_table = &unit->fde_table;
>        if (fde_table->empty ())
>         continue;
>
> @@ -2120,14 +2123,11 @@ dwarf2_build_frame_info (struct objfile *objfile)
>    const gdb_byte *frame_ptr;
>    dwarf2_cie_table cie_table;
>    dwarf2_fde_table fde_table;
> -  dwarf2_fde_table *fde_table2;
>
>    /* Build a minimal decoding of the DWARF2 compilation unit.  */
> -  unit = XOBNEW (&objfile->objfile_obstack, comp_unit);
> +  unit = new comp_unit;
>

Move the declaration here?

   unit->abfd = objfile->obfd;
>    unit->objfile = objfile;
>

These two could be constructor arguments?

-  unit->dbase = 0;
> -  unit->tbase = 0;
>
>    if (objfile->separate_debug_objfile_backlink == NULL)
>      {
> @@ -2202,9 +2202,6 @@ dwarf2_build_frame_info (struct objfile *objfile)
>         }
>      }
>
> -  /* Copy fde_table to obstack: it is needed at runtime.  */
> -  fde_table2 = new dwarf2_fde_table;
> -
>    if (!fde_table.empty ())
>      {
>        struct dwarf2_fde *fde_prev = NULL;
> @@ -2249,13 +2246,13 @@ dwarf2_build_frame_info (struct objfile *objfile)
>               && fde_prev->initial_location == fde->initial_location)
>             continue;
>
> -         fde_table2->push_back (fde);
> +         unit->fde_table.push_back (fde);
>           fde_prev = fde;
>         }
> -      fde_table2->shrink_to_fit ();
> +      unit->fde_table.shrink_to_fit ();
>      }
>
> -  dwarf2_frame_objfile_data.set (objfile, fde_table2);
> +  dwarf2_frame_objfile_data.set (objfile, unit);
>  }
>
>  /* Handle 'maintenance show dwarf unwinders'.  */
> --
> 2.17.2
>
>