[4/8] Store the comp_unit instead of the FDE table
Commit Message
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(-)
Comments
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
>
>
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.
>>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes:
> - unit = XOBNEW (&objfile->objfile_obstack, comp_unit);
> + unit = new comp_unit;
Christian> Move the declaration here?
> unit->abfd = objfile->obfd;
> unit->objfile = objfile;
Christian> These two could be constructor arguments?
I made both of these changes.
Tom
>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:
>> + comp_unit *unit = dwarf2_frame_objfile_data.get (objfile);
>> + if (unit == NULL)
>> {
Luis> 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);
Luis> NULL -> nullptr
I made these changes.
Tom
On 2020-02-08 10:27 a.m., Tom Tromey wrote:
> @@ -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;
Not mandatory, but I'd suggest using an std::unique_ptr here, to hold this
variable, and then "release" it into the per-objfile map (since it can only
hold bare pointers).
Simon
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>> - unit = XOBNEW (&objfile->objfile_obstack, comp_unit);
>> + unit = new comp_unit;
Simon> Not mandatory, but I'd suggest using an std::unique_ptr here, to hold this
Simon> variable, and then "release" it into the per-objfile map (since it can only
Simon> hold bare pointers).
Good idea, I did this.
Tom
@@ -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;
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;
-
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'. */