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

[Luis Machado <luis dot machado at linaro dot org>]

On 2/8/20 12:27 PM, Tom Tromey 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)
>   	{

NULL -> nullptr.

>   	  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);

NULL -> nullptr

>   
> +      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;
>     unit->abfd = objfile->obfd;
>     unit->objfile = objfile;
> -  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;
> -

Ok. I mentioned the stale comment in another patch, so please ignore it.

>     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'.  */

Otherwise LGTM.