[PATCH 0/8] Share DWARF frame information across inferiors

[PATCH 0/8] Share DWARF frame information across inferiors

[Tom Tromey]

I realized recently that the DWARF frame information is, generally,
independent of the objfile.  Unlike symbols, it doesn't incorporate
text offsets into the data; and it is largely independent of other
parts of the DWARF code.

This series arranges to share this data across objfiles, when
appropriate.  This is done in the usual way, namely attaching the data
to the BFD when such sharing is possible.

Tom

[PATCH 6/8] Remove a use of the comp_unit backlink

[Tom Tromey]

The DWARF frame comp_unit object still has a backlink to the objfile.
In order to be truly objfile-independent, this must be removed.

This patch removes one such use, by passing the gdbarch to
decode_frame_entry directly.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* dwarf2/frame.c (decode_frame_entry_1): Add gdbarch parameter.
	(decode_frame_entry): Likewise.
	(dwarf2_build_frame_info): Update.

Change-Id: I51126a468a012540eaf002be64dba7cbcb111b4d
---
 gdb/ChangeLog      |  6 ++++++
 gdb/dwarf2/frame.c | 21 +++++++++++++--------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 7e1a744513b..d4f6165b75c 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -1695,7 +1695,8 @@ enum eh_frame_type
   EH_CIE_OR_FDE_TYPE_ID = EH_CIE_TYPE_ID | EH_FDE_TYPE_ID
 };
 
-static const gdb_byte *decode_frame_entry (struct comp_unit *unit,
+static const gdb_byte *decode_frame_entry (struct gdbarch *gdbarch,
+					   struct comp_unit *unit,
 					   const gdb_byte *start,
 					   int eh_frame_p,
 					   dwarf2_cie_table &cie_table,
@@ -1706,13 +1707,13 @@ static const gdb_byte *decode_frame_entry (struct comp_unit *unit,
    Return NULL if invalid input, otherwise the next byte to be processed.  */
 
 static const gdb_byte *
-decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
+decode_frame_entry_1 (struct gdbarch *gdbarch,
+		      struct comp_unit *unit, const gdb_byte *start,
 		      int eh_frame_p,
                       dwarf2_cie_table &cie_table,
                       dwarf2_fde_table *fde_table,
                       enum eh_frame_type entry_type)
 {
-  struct gdbarch *gdbarch = get_objfile_arch (unit->objfile);
   const gdb_byte *buf, *end;
   ULONGEST length;
   unsigned int bytes_read;
@@ -1957,7 +1958,8 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
       fde->cie = find_cie (cie_table, cie_pointer);
       if (fde->cie == NULL)
 	{
-	  decode_frame_entry (unit, unit->dwarf_frame_buffer + cie_pointer,
+	  decode_frame_entry (gdbarch, unit,
+			      unit->dwarf_frame_buffer + cie_pointer,
 			      eh_frame_p, cie_table, fde_table,
 			      EH_CIE_TYPE_ID);
 	  fde->cie = find_cie (cie_table, cie_pointer);
@@ -2008,7 +2010,8 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
    expect an FDE or a CIE.  */
 
 static const gdb_byte *
-decode_frame_entry (struct comp_unit *unit, const gdb_byte *start,
+decode_frame_entry (struct gdbarch *gdbarch,
+		    struct comp_unit *unit, const gdb_byte *start,
 		    int eh_frame_p,
 		    dwarf2_cie_table &cie_table,
                     dwarf2_fde_table *fde_table,
@@ -2020,7 +2023,7 @@ decode_frame_entry (struct comp_unit *unit, const gdb_byte *start,
 
   while (1)
     {
-      ret = decode_frame_entry_1 (unit, start, eh_frame_p,
+      ret = decode_frame_entry_1 (gdbarch, unit, start, eh_frame_p,
 				  cie_table, fde_table, entry_type);
       if (ret != NULL)
 	break;
@@ -2127,6 +2130,8 @@ dwarf2_build_frame_info (struct objfile *objfile)
   dwarf2_cie_table cie_table;
   dwarf2_fde_table fde_table;
 
+  struct gdbarch *gdbarch = get_objfile_arch (objfile);
+
   /* Build a minimal decoding of the DWARF2 compilation unit.  */
   unit = new comp_unit;
   unit->abfd = objfile->obfd;
@@ -2162,7 +2167,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
 	    {
 	      frame_ptr = unit->dwarf_frame_buffer;
 	      while (frame_ptr < unit->dwarf_frame_buffer + unit->dwarf_frame_size)
-		frame_ptr = decode_frame_entry (unit, frame_ptr, 1,
+		frame_ptr = decode_frame_entry (gdbarch, unit, frame_ptr, 1,
 						cie_table, &fde_table,
 						EH_CIE_OR_FDE_TYPE_ID);
 	    }
@@ -2192,7 +2197,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
 	{
 	  frame_ptr = unit->dwarf_frame_buffer;
 	  while (frame_ptr < unit->dwarf_frame_buffer + unit->dwarf_frame_size)
-	    frame_ptr = decode_frame_entry (unit, frame_ptr, 0,
+	    frame_ptr = decode_frame_entry (gdbarch, unit, frame_ptr, 0,
 					    cie_table, &fde_table,
 					    EH_CIE_OR_FDE_TYPE_ID);
 	}
-- 
2.17.2

[PATCH 1/8] Don't forward-declare struct objfile in frame.h

[Tom Tromey]

dwarf2/frame.h forward-declares struct objfile, but there's no need
for this.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* dwarf2/frame.h (struct objfile): Don't forward declare.

Change-Id: I4d54d46ac9422eeb64dc5f0b934792e77a875aa5
---
 gdb/ChangeLog      | 4 ++++
 gdb/dwarf2/frame.h | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h
index a0ec3615a45..dd7315c8515 100644
--- a/gdb/dwarf2/frame.h
+++ b/gdb/dwarf2/frame.h
@@ -23,7 +23,6 @@
 #define DWARF2_FRAME_H 1
 
 struct gdbarch;
-struct objfile;
 struct frame_info;
 struct dwarf2_per_cu_data;
 struct agent_expr;
-- 
2.17.2

Re: [PATCH 1/8] Don't forward-declare struct objfile in frame.h

[Simon Marchi]

On 2020-02-08 10:27 a.m., Tom Tromey wrote:
> dwarf2/frame.h forward-declares struct objfile, but there's no need
> for this.
> 
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/frame.h (struct objfile): Don't forward declare.
> 
> Change-Id: I4d54d46ac9422eeb64dc5f0b934792e77a875aa5
> ---
>  gdb/ChangeLog      | 4 ++++
>  gdb/dwarf2/frame.h | 1 -
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h
> index a0ec3615a45..dd7315c8515 100644
> --- a/gdb/dwarf2/frame.h
> +++ b/gdb/dwarf2/frame.h
> @@ -23,7 +23,6 @@
>  #define DWARF2_FRAME_H 1
>  
>  struct gdbarch;
> -struct objfile;
>  struct frame_info;
>  struct dwarf2_per_cu_data;
>  struct agent_expr;
> -- 
> 2.17.2
> 

I'd say "dwarf2/frame.h" in the subject, since there's also a frame.h in
the gdb directory.  But other than that, I think you could push patches like
this one right away.

Simon

Re: [PATCH 1/8] Don't forward-declare struct objfile in frame.h

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I'd say "dwarf2/frame.h" in the subject, since there's also a frame.h in
Simon> the gdb directory.

Good idea, I did that.

Simon> But other than that, I think you could push patches like
Simon> this one right away.

Yeah, I will try to do that, since there's little point in anybody
having to review these.

Tom

[PATCH 8/8] Move the frame data to the BFD when possible

[Tom Tromey]

Now that comp_unit and the remaining frame data are all independent of
the objfile, it can all be stored on the BFD and shared across
inferiors.

As with other code doing this same thing, care must be taken to not
share the data when the objfile requires relocations.  So, two keys
are used: one for the BFD and one for the objfile, and
gdb_bfd_requires_relocations is used to differentiate between the two
cases.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* dwarf2/frame.c (dwarf2_frame_bfd_data): New global.
	(dwarf2_frame_objfile_data): Add comment.
	(find_comp_unit, set_comp_unit): New functions.
	(dwarf2_frame_find_fde): Use find_comp_unit.
	(dwarf2_build_frame_info): Use set_comp_unit.

Change-Id: I50f5e1220c3f6b2992a15d5112fe474fb2904511
---
 gdb/ChangeLog      |  8 ++++++++
 gdb/dwarf2/frame.c | 37 +++++++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 2c35016e2d9..25dddb7b1ae 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -1475,8 +1475,14 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
   return get_frame_base (this_frame);
 }
 \f
-const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
+/* We store the frame data on the BFD, when it is independent of the
+   address space and so can be shared.  */
+const struct bfd_key<comp_unit> dwarf2_frame_bfd_data;
 
+/* If any BFD sections require relocations (note; really should be if
+   any debug info requires relocations), then we store the frame data
+   on the objfile instead, and do not share it.  */
+const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
 \f
 
 /* Pointer encoding helper functions.  */
@@ -1630,6 +1636,29 @@ bsearch_fde_cmp (const dwarf2_fde *fde, CORE_ADDR seek_pc)
   return 1;
 }
 
+/* Find an existing comp_unit for an objfile, if any.  */
+
+static comp_unit *
+find_comp_unit (struct objfile *objfile)
+{
+  bfd *abfd = objfile->obfd;
+  if (gdb_bfd_requires_relocations (abfd))
+    return dwarf2_frame_bfd_data.get (abfd);
+  return dwarf2_frame_objfile_data.get (objfile);
+}
+
+/* Store the comp_unit on OBJFILE, or the corresponding BFD, as
+   appropriate.  */
+
+static void
+set_comp_unit (struct objfile *objfile, struct comp_unit *unit)
+{
+  bfd *abfd = objfile->obfd;
+  if (gdb_bfd_requires_relocations (abfd))
+    return dwarf2_frame_bfd_data.set (abfd, unit);
+  return dwarf2_frame_objfile_data.set (objfile, unit);
+}
+
 /* Find the FDE for *PC.  Return a pointer to the FDE, and store the
    initial location associated with it into *PC.  */
 
@@ -1641,11 +1670,11 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
       CORE_ADDR offset;
       CORE_ADDR seek_pc;
 
-      comp_unit *unit = dwarf2_frame_objfile_data.get (objfile);
+      comp_unit *unit = find_comp_unit (objfile);
       if (unit == NULL)
 	{
 	  dwarf2_build_frame_info (objfile);
-	  unit = dwarf2_frame_objfile_data.get (objfile);
+	  unit = find_comp_unit (objfile);
 	}
       gdb_assert (unit != NULL);
 
@@ -2261,7 +2290,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
       unit->fde_table.shrink_to_fit ();
     }
 
-  dwarf2_frame_objfile_data.set (objfile, unit);
+  set_comp_unit (objfile, unit);
 }
 
 /* Handle 'maintenance show dwarf unwinders'.  */
-- 
2.17.2

Re: [PATCH 8/8] Move the frame data to the BFD when possible

[Luis Machado]

On 2/8/20 12:27 PM, Tom Tromey wrote:
> Now that comp_unit and the remaining frame data are all independent of
> the objfile, it can all be stored on the BFD and shared across
> inferiors.
> 
> As with other code doing this same thing, care must be taken to not
> share the data when the objfile requires relocations.  So, two keys
> are used: one for the BFD and one for the objfile, and
> gdb_bfd_requires_relocations is used to differentiate between the two
> cases.
> 
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/frame.c (dwarf2_frame_bfd_data): New global.
> 	(dwarf2_frame_objfile_data): Add comment.
> 	(find_comp_unit, set_comp_unit): New functions.
> 	(dwarf2_frame_find_fde): Use find_comp_unit.
> 	(dwarf2_build_frame_info): Use set_comp_unit.
> 
> Change-Id: I50f5e1220c3f6b2992a15d5112fe474fb2904511
> ---
>   gdb/ChangeLog      |  8 ++++++++
>   gdb/dwarf2/frame.c | 37 +++++++++++++++++++++++++++++++++----
>   2 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index 2c35016e2d9..25dddb7b1ae 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -1475,8 +1475,14 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
>     return get_frame_base (this_frame);
>   }
>   \f
> -const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
> +/* We store the frame data on the BFD, when it is independent of the

when -> where?

> +   address space and so can be shared.  */
> +const struct bfd_key<comp_unit> dwarf2_frame_bfd_data;
>   
> +/* If any BFD sections require relocations (note; really should be if
> +   any debug info requires relocations), then we store the frame data
> +   on the objfile instead, and do not share it.  */
> +const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
>   \f
>   
>   /* Pointer encoding helper functions.  */
> @@ -1630,6 +1636,29 @@ bsearch_fde_cmp (const dwarf2_fde *fde, CORE_ADDR seek_pc)
>     return 1;
>   }
>   
> +/* Find an existing comp_unit for an objfile, if any.  */
> +
> +static comp_unit *
> +find_comp_unit (struct objfile *objfile)
> +{
> +  bfd *abfd = objfile->obfd;
> +  if (gdb_bfd_requires_relocations (abfd))
> +    return dwarf2_frame_bfd_data.get (abfd);
> +  return dwarf2_frame_objfile_data.get (objfile);
> +}
> +
> +/* Store the comp_unit on OBJFILE, or the corresponding BFD, as
> +   appropriate.  */
> +
> +static void
> +set_comp_unit (struct objfile *objfile, struct comp_unit *unit)
> +{
> +  bfd *abfd = objfile->obfd;
> +  if (gdb_bfd_requires_relocations (abfd))
> +    return dwarf2_frame_bfd_data.set (abfd, unit);
> +  return dwarf2_frame_objfile_data.set (objfile, unit);
> +}
> +
>   /* Find the FDE for *PC.  Return a pointer to the FDE, and store the
>      initial location associated with it into *PC.  */
>   
> @@ -1641,11 +1670,11 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
>         CORE_ADDR offset;
>         CORE_ADDR seek_pc;
>   
> -      comp_unit *unit = dwarf2_frame_objfile_data.get (objfile);
> +      comp_unit *unit = find_comp_unit (objfile);
>         if (unit == NULL)
>   	{
>   	  dwarf2_build_frame_info (objfile);
> -	  unit = dwarf2_frame_objfile_data.get (objfile);
> +	  unit = find_comp_unit (objfile);
>   	}
>         gdb_assert (unit != NULL);
>   
> @@ -2261,7 +2290,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
>         unit->fde_table.shrink_to_fit ();
>       }
>   
> -  dwarf2_frame_objfile_data.set (objfile, unit);
> +  set_comp_unit (objfile, unit);
>   }
>   
>   /* Handle 'maintenance show dwarf unwinders'.  */
> 

Otherwise LGTM.

Re: [PATCH 8/8] Move the frame data to the BFD when possible

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

>> -const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
>> +/* We store the frame data on the BFD, when it is independent of the

Luis> when -> where?

More like "in the case where", but I just reworded it to:

/* We store the frame data on the BFD.  This is only done if it is
   independent of the address space and so can be shared.  */
const struct bfd_key<comp_unit> dwarf2_frame_bfd_data;

Tom

Re: [PATCH 8/8] Move the frame data to the BFD when possible

[Simon Marchi]

On 2020-02-08 10:27 a.m., Tom Tromey wrote:
> Now that comp_unit and the remaining frame data are all independent of
> the objfile, it can all be stored on the BFD and shared across
> inferiors.
> 
> As with other code doing this same thing, care must be taken to not
> share the data when the objfile requires relocations.  So, two keys
> are used: one for the BFD and one for the objfile, and
> gdb_bfd_requires_relocations is used to differentiate between the two
> cases.

FYI, there's a regression linked to this patch.

https://sourceware.org/bugzilla/show_bug.cgi?id=26876

Simon

[PATCH 3/8] Change fde table to a vector

[Tom Tromey]

This removes struct dwarf2_fde_table, replacing it with a typedef of
std::vector.  This simplifies the code somewhat.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* dwarf2/frame.c (struct dwarf2_fde_table): Remove.
	(dwarf2_fde_table): Typedef for std::vector.
	(dwarf2_frame_objfile_data): Remove the deleter.
	(dwarf2_frame_find_fde, add_fde, decode_frame_entry_1)
	(decode_frame_entry): Update.
	(dwarf2_build_frame_info): Use "new".

Change-Id: Ie31c7413dce2865b76f5f70374ba7a559112ce5e
---
 gdb/ChangeLog      |   9 ++++
 gdb/dwarf2/frame.c | 102 ++++++++++++---------------------------------
 2 files changed, 35 insertions(+), 76 deletions(-)

diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index db8e5cd25f3..6d87e598345 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -129,11 +129,7 @@ struct dwarf2_fde
   unsigned char eh_frame_p;
 };
 
-struct dwarf2_fde_table
-{
-  int num_entries;
-  struct dwarf2_fde **entries;
-};
+typedef std::vector<dwarf2_fde *> dwarf2_fde_table;
 
 /* A minimal decoding of DWARF2 compilation units.  We only decode
    what's needed to get to the call frame information.  */
@@ -1471,9 +1467,7 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
   return get_frame_base (this_frame);
 }
 \f
-const struct objfile_key<dwarf2_fde_table,
-			 gdb::noop_deleter<dwarf2_fde_table>>
-  dwarf2_frame_objfile_data;
+const struct objfile_key<dwarf2_fde_table> dwarf2_frame_objfile_data;
 
 \f
 
@@ -1636,7 +1630,7 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
 {
   for (objfile *objfile : current_program_space->objfiles ())
     {
-      struct dwarf2_fde_table *fde_table;
+      dwarf2_fde_table *fde_table;
       CORE_ADDR offset;
       CORE_ADDR seek_pc;
 
@@ -1648,20 +1642,20 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
 	}
       gdb_assert (fde_table != NULL);
 
-      if (fde_table->num_entries == 0)
+      if (fde_table->empty ())
 	continue;
 
       gdb_assert (!objfile->section_offsets.empty ());
       offset = objfile->text_section_offset ();
 
-      gdb_assert (fde_table->num_entries > 0);
-      if (*pc < offset + fde_table->entries[0]->initial_location)
+      gdb_assert (!fde_table->empty ());
+      if (*pc < offset + (*fde_table)[0]->initial_location)
         continue;
 
       seek_pc = *pc - offset;
-      auto end = fde_table->entries + fde_table->num_entries;
-      auto it = gdb::binary_search (fde_table->entries, end, seek_pc, bsearch_fde_cmp);
-      if (it != end)
+      auto it = gdb::binary_search (fde_table->begin (), fde_table->end (),
+				    seek_pc, bsearch_fde_cmp);
+      if (it != fde_table->end ())
         {
           *pc = (*it)->initial_location + offset;
 	  if (out_offset)
@@ -1674,16 +1668,13 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
 
 /* Add a pointer to new FDE to the FDE_TABLE, allocating space for it.  */
 static void
-add_fde (struct dwarf2_fde_table *fde_table, struct dwarf2_fde *fde)
+add_fde (dwarf2_fde_table *fde_table, struct dwarf2_fde *fde)
 {
   if (fde->address_range == 0)
     /* Discard useless FDEs.  */
     return;
 
-  fde_table->num_entries += 1;
-  fde_table->entries = XRESIZEVEC (struct dwarf2_fde *, fde_table->entries,
-				   fde_table->num_entries);
-  fde_table->entries[fde_table->num_entries - 1] = fde;
+  fde_table->push_back (fde);
 }
 
 #define DW64_CIE_ID 0xffffffffffffffffULL
@@ -1702,7 +1693,7 @@ static const gdb_byte *decode_frame_entry (struct comp_unit *unit,
 					   const gdb_byte *start,
 					   int eh_frame_p,
 					   dwarf2_cie_table &cie_table,
-					   struct dwarf2_fde_table *fde_table,
+					   dwarf2_fde_table *fde_table,
 					   enum eh_frame_type entry_type);
 
 /* Decode the next CIE or FDE, entry_type specifies the expected type.
@@ -1712,7 +1703,7 @@ static const gdb_byte *
 decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
 		      int eh_frame_p,
                       dwarf2_cie_table &cie_table,
-                      struct dwarf2_fde_table *fde_table,
+                      dwarf2_fde_table *fde_table,
                       enum eh_frame_type entry_type)
 {
   struct gdbarch *gdbarch = get_objfile_arch (unit->objfile);
@@ -2014,7 +2005,7 @@ static const gdb_byte *
 decode_frame_entry (struct comp_unit *unit, const gdb_byte *start,
 		    int eh_frame_p,
 		    dwarf2_cie_table &cie_table,
-                    struct dwarf2_fde_table *fde_table,
+                    dwarf2_fde_table *fde_table,
                     enum eh_frame_type entry_type)
 {
   enum { NONE, ALIGN4, ALIGN8, FAIL } workaround = NONE;
@@ -2128,11 +2119,8 @@ dwarf2_build_frame_info (struct objfile *objfile)
   struct comp_unit *unit;
   const gdb_byte *frame_ptr;
   dwarf2_cie_table cie_table;
-  struct dwarf2_fde_table fde_table;
-  struct dwarf2_fde_table *fde_table2;
-
-  fde_table.num_entries = 0;
-  fde_table.entries = NULL;
+  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);
@@ -2181,12 +2169,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
 	      warning (_("skipping .eh_frame info of %s: %s"),
 		       objfile_name (objfile), e.what ());
 
-	      if (fde_table.num_entries != 0)
-		{
-                  xfree (fde_table.entries);
-		  fde_table.entries = NULL;
-		  fde_table.num_entries = 0;
-		}
+	      fde_table.clear ();
 	      /* The cie_table is discarded below.  */
 	    }
 
@@ -2200,7 +2183,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
                            &unit->dwarf_frame_size);
   if (unit->dwarf_frame_size)
     {
-      int num_old_fde_entries = fde_table.num_entries;
+      size_t num_old_fde_entries = fde_table.size ();
 
       try
 	{
@@ -2215,42 +2198,20 @@ dwarf2_build_frame_info (struct objfile *objfile)
 	  warning (_("skipping .debug_frame info of %s: %s"),
 		   objfile_name (objfile), e.what ());
 
-	  if (fde_table.num_entries != 0)
-	    {
-	      fde_table.num_entries = num_old_fde_entries;
-	      if (num_old_fde_entries == 0)
-		{
-		  xfree (fde_table.entries);
-		  fde_table.entries = NULL;
-		}
-	      else
-		{
-		  fde_table.entries
-		    = XRESIZEVEC (struct dwarf2_fde *, fde_table.entries,
-				  fde_table.num_entries);
-		}
-	    }
-	  fde_table.num_entries = num_old_fde_entries;
+	  fde_table.resize (num_old_fde_entries);
 	}
     }
 
   /* Copy fde_table to obstack: it is needed at runtime.  */
-  fde_table2 = XOBNEW (&objfile->objfile_obstack, struct dwarf2_fde_table);
+  fde_table2 = new dwarf2_fde_table;
 
-  if (fde_table.num_entries == 0)
-    {
-      fde_table2->entries = NULL;
-      fde_table2->num_entries = 0;
-    }
-  else
+  if (!fde_table.empty ())
     {
       struct dwarf2_fde *fde_prev = NULL;
       struct dwarf2_fde *first_non_zero_fde = NULL;
-      int i;
 
       /* Prepare FDE table for lookups.  */
-      std::sort (fde_table.entries, fde_table.entries + fde_table.num_entries,
-		 fde_is_less_than);
+      std::sort (fde_table.begin (), fde_table.end (), fde_is_less_than);
 
       /* Check for leftovers from --gc-sections.  The GNU linker sets
 	 the relevant symbols to zero, but doesn't zero the FDE *end*
@@ -2264,10 +2225,8 @@ dwarf2_build_frame_info (struct objfile *objfile)
 	 Start by finding the first FDE with non-zero start.  Below
 	 we'll discard all FDEs that start at zero and overlap this
 	 one.  */
-      for (i = 0; i < fde_table.num_entries; i++)
+      for (struct dwarf2_fde *fde : fde_table)
 	{
-	  struct dwarf2_fde *fde = fde_table.entries[i];
-
 	  if (fde->initial_location != 0)
 	    {
 	      first_non_zero_fde = fde;
@@ -2278,11 +2237,8 @@ dwarf2_build_frame_info (struct objfile *objfile)
       /* Since we'll be doing bsearch, squeeze out identical (except
 	 for eh_frame_p) fde entries so bsearch result is predictable.
 	 Also discard leftovers from --gc-sections.  */
-      fde_table2->num_entries = 0;
-      for (i = 0; i < fde_table.num_entries; i++)
+      for (struct dwarf2_fde *fde : fde_table)
 	{
-	  struct dwarf2_fde *fde = fde_table.entries[i];
-
 	  if (fde->initial_location == 0
 	      && first_non_zero_fde != NULL
 	      && (first_non_zero_fde->initial_location
@@ -2293,16 +2249,10 @@ dwarf2_build_frame_info (struct objfile *objfile)
 	      && fde_prev->initial_location == fde->initial_location)
 	    continue;
 
-	  obstack_grow (&objfile->objfile_obstack, &fde_table.entries[i],
-			sizeof (fde_table.entries[0]));
-	  ++fde_table2->num_entries;
+	  fde_table2->push_back (fde);
 	  fde_prev = fde;
 	}
-      fde_table2->entries
-	= (struct dwarf2_fde **) obstack_finish (&objfile->objfile_obstack);
-
-      /* Discard the original fde_table.  */
-      xfree (fde_table.entries);
+      fde_table2->shrink_to_fit ();
     }
 
   dwarf2_frame_objfile_data.set (objfile, fde_table2);
-- 
2.17.2

Re: [PATCH 3/8] Change fde table to a vector

[Luis Machado]

Great cleanup. Just a few comments...

On 2/8/20 12:27 PM, Tom Tromey wrote:
> This removes struct dwarf2_fde_table, replacing it with a typedef of
> std::vector.  This simplifies the code somewhat.
> 
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/frame.c (struct dwarf2_fde_table): Remove.
> 	(dwarf2_fde_table): Typedef for std::vector.
> 	(dwarf2_frame_objfile_data): Remove the deleter.
> 	(dwarf2_frame_find_fde, add_fde, decode_frame_entry_1)
> 	(decode_frame_entry): Update.
> 	(dwarf2_build_frame_info): Use "new".
> 
> Change-Id: Ie31c7413dce2865b76f5f70374ba7a559112ce5e
> ---
>   gdb/ChangeLog      |   9 ++++
>   gdb/dwarf2/frame.c | 102 ++++++++++++---------------------------------
>   2 files changed, 35 insertions(+), 76 deletions(-)
> 
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index db8e5cd25f3..6d87e598345 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -129,11 +129,7 @@ struct dwarf2_fde
>     unsigned char eh_frame_p;
>   };
>   
> -struct dwarf2_fde_table
> -{
> -  int num_entries;
> -  struct dwarf2_fde **entries;
> -};
> +typedef std::vector<dwarf2_fde *> dwarf2_fde_table;
>   
>   /* A minimal decoding of DWARF2 compilation units.  We only decode
>      what's needed to get to the call frame information.  */
> @@ -1471,9 +1467,7 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
>     return get_frame_base (this_frame);
>   }
>   \f
> -const struct objfile_key<dwarf2_fde_table,
> -			 gdb::noop_deleter<dwarf2_fde_table>>
> -  dwarf2_frame_objfile_data;
> +const struct objfile_key<dwarf2_fde_table> dwarf2_frame_objfile_data;
>   
>   \f
>   
> @@ -1636,7 +1630,7 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
>   {
>     for (objfile *objfile : current_program_space->objfiles ())
>       {
> -      struct dwarf2_fde_table *fde_table;
> +      dwarf2_fde_table *fde_table;
>         CORE_ADDR offset;
>         CORE_ADDR seek_pc;
>   
> @@ -1648,20 +1642,20 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
>   	}
>         gdb_assert (fde_table != NULL);
>   
> -      if (fde_table->num_entries == 0)
> +      if (fde_table->empty ())
>   	continue;
>   
>         gdb_assert (!objfile->section_offsets.empty ());
>         offset = objfile->text_section_offset ();
>   
> -      gdb_assert (fde_table->num_entries > 0);
> -      if (*pc < offset + fde_table->entries[0]->initial_location)
> +      gdb_assert (!fde_table->empty ());
> +      if (*pc < offset + (*fde_table)[0]->initial_location)
>           continue;
>   
>         seek_pc = *pc - offset;
> -      auto end = fde_table->entries + fde_table->num_entries;
> -      auto it = gdb::binary_search (fde_table->entries, end, seek_pc, bsearch_fde_cmp);
> -      if (it != end)
> +      auto it = gdb::binary_search (fde_table->begin (), fde_table->end (),
> +				    seek_pc, bsearch_fde_cmp);
> +      if (it != fde_table->end ())
>           {
>             *pc = (*it)->initial_location + offset;
>   	  if (out_offset)
> @@ -1674,16 +1668,13 @@ dwarf2_frame_find_fde (CORE_ADDR *pc, CORE_ADDR *out_offset)
>   
>   /* Add a pointer to new FDE to the FDE_TABLE, allocating space for it.  */
>   static void

We don't quite allocate space for it anymore, but we do behind the 
scenes. The comment seems slightly stale. Plus the sentence reads a bit 
funny. I'd update this, but i'm fine keeping it too.

> -add_fde (struct dwarf2_fde_table *fde_table, struct dwarf2_fde *fde)
> +add_fde (dwarf2_fde_table *fde_table, struct dwarf2_fde *fde)
>   {
>     if (fde->address_range == 0)
>       /* Discard useless FDEs.  */
>       return;
>   
> -  fde_table->num_entries += 1;
> -  fde_table->entries = XRESIZEVEC (struct dwarf2_fde *, fde_table->entries,
> -				   fde_table->num_entries);
> -  fde_table->entries[fde_table->num_entries - 1] = fde;
> +  fde_table->push_back (fde);
>   }
>   
>   #define DW64_CIE_ID 0xffffffffffffffffULL
> @@ -1702,7 +1693,7 @@ static const gdb_byte *decode_frame_entry (struct comp_unit *unit,
>   					   const gdb_byte *start,
>   					   int eh_frame_p,
>   					   dwarf2_cie_table &cie_table,
> -					   struct dwarf2_fde_table *fde_table,
> +					   dwarf2_fde_table *fde_table,
>   					   enum eh_frame_type entry_type);
>   
>   /* Decode the next CIE or FDE, entry_type specifies the expected type.
> @@ -1712,7 +1703,7 @@ static const gdb_byte *
>   decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
>   		      int eh_frame_p,
>                         dwarf2_cie_table &cie_table,
> -                      struct dwarf2_fde_table *fde_table,
> +                      dwarf2_fde_table *fde_table,
>                         enum eh_frame_type entry_type)
>   {
>     struct gdbarch *gdbarch = get_objfile_arch (unit->objfile);
> @@ -2014,7 +2005,7 @@ static const gdb_byte *
>   decode_frame_entry (struct comp_unit *unit, const gdb_byte *start,
>   		    int eh_frame_p,
>   		    dwarf2_cie_table &cie_table,
> -                    struct dwarf2_fde_table *fde_table,
> +                    dwarf2_fde_table *fde_table,
>                       enum eh_frame_type entry_type)
>   {
>     enum { NONE, ALIGN4, ALIGN8, FAIL } workaround = NONE;
> @@ -2128,11 +2119,8 @@ dwarf2_build_frame_info (struct objfile *objfile)
>     struct comp_unit *unit;
>     const gdb_byte *frame_ptr;
>     dwarf2_cie_table cie_table;
> -  struct dwarf2_fde_table fde_table;
> -  struct dwarf2_fde_table *fde_table2;
> -
> -  fde_table.num_entries = 0;
> -  fde_table.entries = NULL;
> +  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);
> @@ -2181,12 +2169,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
>   	      warning (_("skipping .eh_frame info of %s: %s"),
>   		       objfile_name (objfile), e.what ());
>   
> -	      if (fde_table.num_entries != 0)
> -		{
> -                  xfree (fde_table.entries);
> -		  fde_table.entries = NULL;
> -		  fde_table.num_entries = 0;
> -		}
> +	      fde_table.clear ();
>   	      /* The cie_table is discarded below.  */
>   	    }
>   
> @@ -2200,7 +2183,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
>                              &unit->dwarf_frame_size);
>     if (unit->dwarf_frame_size)
>       {
> -      int num_old_fde_entries = fde_table.num_entries;
> +      size_t num_old_fde_entries = fde_table.size ();
>   
>         try
>   	{
> @@ -2215,42 +2198,20 @@ dwarf2_build_frame_info (struct objfile *objfile)
>   	  warning (_("skipping .debug_frame info of %s: %s"),
>   		   objfile_name (objfile), e.what ());
>   
> -	  if (fde_table.num_entries != 0)
> -	    {
> -	      fde_table.num_entries = num_old_fde_entries;
> -	      if (num_old_fde_entries == 0)
> -		{
> -		  xfree (fde_table.entries);
> -		  fde_table.entries = NULL;
> -		}
> -	      else
> -		{
> -		  fde_table.entries
> -		    = XRESIZEVEC (struct dwarf2_fde *, fde_table.entries,
> -				  fde_table.num_entries);
> -		}
> -	    }
> -	  fde_table.num_entries = num_old_fde_entries;
> +	  fde_table.resize (num_old_fde_entries);

Took me a bit to figure out the code above. Quite convoluted in its use 
of the size variables. But the new code looks sane as far as i can tell.

>   	}
>       }
>   
>     /* Copy fde_table to obstack: it is needed at runtime.  */
> -  fde_table2 = XOBNEW (&objfile->objfile_obstack, struct dwarf2_fde_table);
> +  fde_table2 = new dwarf2_fde_table; >

The comment mentioning obstack is now obsolete and can be removed.

> -  if (fde_table.num_entries == 0)
> -    {
> -      fde_table2->entries = NULL;
> -      fde_table2->num_entries = 0;
> -    }
> -  else
> +  if (!fde_table.empty ())
>       {
>         struct dwarf2_fde *fde_prev = NULL;
>         struct dwarf2_fde *first_non_zero_fde = NULL;
> -      int i;
>   
>         /* Prepare FDE table for lookups.  */
> -      std::sort (fde_table.entries, fde_table.entries + fde_table.num_entries,
> -		 fde_is_less_than);
> +      std::sort (fde_table.begin (), fde_table.end (), fde_is_less_than);
>   
>         /* Check for leftovers from --gc-sections.  The GNU linker sets
>   	 the relevant symbols to zero, but doesn't zero the FDE *end*
> @@ -2264,10 +2225,8 @@ dwarf2_build_frame_info (struct objfile *objfile)
>   	 Start by finding the first FDE with non-zero start.  Below
>   	 we'll discard all FDEs that start at zero and overlap this
>   	 one.  */
> -      for (i = 0; i < fde_table.num_entries; i++)
> +      for (struct dwarf2_fde *fde : fde_table)
>   	{
> -	  struct dwarf2_fde *fde = fde_table.entries[i];
> -
>   	  if (fde->initial_location != 0)
>   	    {
>   	      first_non_zero_fde = fde;
> @@ -2278,11 +2237,8 @@ dwarf2_build_frame_info (struct objfile *objfile)
>         /* Since we'll be doing bsearch, squeeze out identical (except
>   	 for eh_frame_p) fde entries so bsearch result is predictable.
>   	 Also discard leftovers from --gc-sections.  */
> -      fde_table2->num_entries = 0;
> -      for (i = 0; i < fde_table.num_entries; i++)
> +      for (struct dwarf2_fde *fde : fde_table)
>   	{
> -	  struct dwarf2_fde *fde = fde_table.entries[i];
> -
>   	  if (fde->initial_location == 0
>   	      && first_non_zero_fde != NULL
>   	      && (first_non_zero_fde->initial_location
> @@ -2293,16 +2249,10 @@ dwarf2_build_frame_info (struct objfile *objfile)
>   	      && fde_prev->initial_location == fde->initial_location)
>   	    continue;
>   
> -	  obstack_grow (&objfile->objfile_obstack, &fde_table.entries[i],
> -			sizeof (fde_table.entries[0]));
> -	  ++fde_table2->num_entries;
> +	  fde_table2->push_back (fde);
>   	  fde_prev = fde;
>   	}
> -      fde_table2->entries
> -	= (struct dwarf2_fde **) obstack_finish (&objfile->objfile_obstack);
> -
> -      /* Discard the original fde_table.  */
> -      xfree (fde_table.entries);
> +      fde_table2->shrink_to_fit ();
>       }
>   
>     dwarf2_frame_objfile_data.set (objfile, fde_table2);
> 

Otherwise LGTM.

Re: [PATCH 3/8] Change fde table to a vector

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

Luis> We don't quite allocate space for it anymore, but we do behind the
Luis> scenes. The comment seems slightly stale. Plus the sentence reads a
Luis> bit funny. I'd update this, but i'm fine keeping it too.

I simplified it to just:

    /* Add FDE to FDE_TABLE.  */

>> /* Copy fde_table to obstack: it is needed at runtime.  */
>> -  fde_table2 = XOBNEW (&objfile->objfile_obstack, struct dwarf2_fde_table);
>> +  fde_table2 = new dwarf2_fde_table; >

Luis> The comment mentioning obstack is now obsolete and can be removed.

I've removed it.  I think this is the one you mentioned in another patch
review -- but it is simple enough to just zap it here and fix up the
conflict.

Tom

Re: [PATCH 3/8] Change fde table to a vector

[Simon Marchi]

On 2020-02-08 10:27 a.m., Tom Tromey wrote:
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index db8e5cd25f3..6d87e598345 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -129,11 +129,7 @@ struct dwarf2_fde
>    unsigned char eh_frame_p;
>  };
>  
> -struct dwarf2_fde_table
> -{
> -  int num_entries;
> -  struct dwarf2_fde **entries;
> -};
> +typedef std::vector<dwarf2_fde *> dwarf2_fde_table;

As a further change, I think we could consider make this a vector of objects, rather
than a vector of pointers.

>  /* A minimal decoding of DWARF2 compilation units.  We only decode
>     what's needed to get to the call frame information.  */
> @@ -1471,9 +1467,7 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
>    return get_frame_base (this_frame);
>  }
>  \f
> -const struct objfile_key<dwarf2_fde_table,
> -			 gdb::noop_deleter<dwarf2_fde_table>>
> -  dwarf2_frame_objfile_data;
> +const struct objfile_key<dwarf2_fde_table> dwarf2_frame_objfile_data;

Can you make this static, while at it?

> @@ -2215,42 +2198,20 @@ dwarf2_build_frame_info (struct objfile *objfile)
>  	  warning (_("skipping .debug_frame info of %s: %s"),
>  		   objfile_name (objfile), e.what ());
>  
> -	  if (fde_table.num_entries != 0)
> -	    {
> -	      fde_table.num_entries = num_old_fde_entries;
> -	      if (num_old_fde_entries == 0)
> -		{
> -		  xfree (fde_table.entries);
> -		  fde_table.entries = NULL;
> -		}
> -	      else
> -		{
> -		  fde_table.entries
> -		    = XRESIZEVEC (struct dwarf2_fde *, fde_table.entries,
> -				  fde_table.num_entries);
> -		}
> -	    }
> -	  fde_table.num_entries = num_old_fde_entries;
> +	  fde_table.resize (num_old_fde_entries);
>  	}
>      }
>  
>    /* Copy fde_table to obstack: it is needed at runtime.  */

I think that comment is stale.

> -  fde_table2 = XOBNEW (&objfile->objfile_obstack, struct dwarf2_fde_table);
> +  fde_table2 = new dwarf2_fde_table;
>  
> -  if (fde_table.num_entries == 0)
> -    {
> -      fde_table2->entries = NULL;
> -      fde_table2->num_entries = 0;
> -    }
> -  else
> +  if (!fde_table.empty ())

If you want to simplify the code even further, you could get rid of this check, since the
code below should cope well with an empty vector.

> @@ -2293,16 +2249,10 @@ dwarf2_build_frame_info (struct objfile *objfile)
>  	      && fde_prev->initial_location == fde->initial_location)
>  	    continue;
>  
> -	  obstack_grow (&objfile->objfile_obstack, &fde_table.entries[i],
> -			sizeof (fde_table.entries[0]));
> -	  ++fde_table2->num_entries;
> +	  fde_table2->push_back (fde);
>  	  fde_prev = fde;
>  	}
> -      fde_table2->entries
> -	= (struct dwarf2_fde **) obstack_finish (&objfile->objfile_obstack);
> -
> -      /* Discard the original fde_table.  */
> -      xfree (fde_table.entries);
> +      fde_table2->shrink_to_fit ();

That shrink_to_fit appears to be pointless, as fde_table2 is created empty and we only
push_back.  Maybe you meant to create it with an initial size?

Simon

Re: [PATCH 3/8] Change fde table to a vector

[Simon Marchi]

>> @@ -2215,42 +2198,20 @@ dwarf2_build_frame_info (struct objfile *objfile)
>>  	  warning (_("skipping .debug_frame info of %s: %s"),
>>  		   objfile_name (objfile), e.what ());
>>  
>> -	  if (fde_table.num_entries != 0)
>> -	    {
>> -	      fde_table.num_entries = num_old_fde_entries;
>> -	      if (num_old_fde_entries == 0)
>> -		{
>> -		  xfree (fde_table.entries);
>> -		  fde_table.entries = NULL;
>> -		}
>> -	      else
>> -		{
>> -		  fde_table.entries
>> -		    = XRESIZEVEC (struct dwarf2_fde *, fde_table.entries,
>> -				  fde_table.num_entries);
>> -		}
>> -	    }
>> -	  fde_table.num_entries = num_old_fde_entries;
>> +	  fde_table.resize (num_old_fde_entries);
>>  	}
>>      }
>>  
>>    /* Copy fde_table to obstack: it is needed at runtime.  */
> 
> I think that comment is stale.

Sorry, I just noticed that Luis had already pointed this out.

Simon

Re: [PATCH 3/8] Change fde table to a vector

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> As a further change, I think we could consider make this a vector
Simon> of objects, rather than a vector of pointers.

I think I'd rather leave this for a future change.
It makes the patch a lot less obvious, to me anyway.  find_fde returns a
pointer -- but is that safe?  I think so, but I am not 100% certain; but
with the current code it definitely is.

>> +const struct objfile_key<dwarf2_fde_table> dwarf2_frame_objfile_data;

Simon> Can you make this static, while at it?

I did this.

>> +  if (!fde_table.empty ())

Simon> If you want to simplify the code even further, you could get rid
Simon> of this check, since the code below should cope well with an
Simon> empty vector.

Thanks, I did this too.

>> +      fde_table2->shrink_to_fit ();

Simon> That shrink_to_fit appears to be pointless, as fde_table2 is
Simon> created empty and we only push_back.  Maybe you meant to create
Simon> it with an initial size?

IIUC push_back normally uses some resize factor; the shrink_to_fit is a
request to discard any excess memory allocated this way.

FWIW another thing that could be done in this code is to use the
remove_if to remove elements in-place from fde_table.  This is another
thing I'd rather do separately.

Tom

Re: [PATCH 3/8] Change fde table to a vector

[Simon Marchi]

On 2020-02-12 5:36 p.m., Tom Tromey wrote:
> Simon> That shrink_to_fit appears to be pointless, as fde_table2 is
> Simon> created empty and we only push_back.  Maybe you meant to create
> Simon> it with an initial size?
> 
> IIUC push_back normally uses some resize factor; the shrink_to_fit is a
> request to discard any excess memory allocated this way.

Of course, my bad.

Simon

[PATCH 2/8] Dont' allow copying of auto_obstack

[Tom Tromey]

Add DISABLE_COPY_AND_ASSIGN to struct auto_obstack, to prevent copying
it.  Copying an auto_obstack would be a bug.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* gdb_obstack.h (struct auto_obstack): Use
	DISABLE_COPY_AND_ASSIGN.

Change-Id: Ic9e5ab20acfcfa61c241fed4d99bbb1caefba3cd
---
 gdb/ChangeLog     | 5 +++++
 gdb/gdb_obstack.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/gdb/gdb_obstack.h b/gdb/gdb_obstack.h
index fb9295f5020..9b1d907678f 100644
--- a/gdb/gdb_obstack.h
+++ b/gdb/gdb_obstack.h
@@ -125,6 +125,8 @@ struct auto_obstack : obstack
   ~auto_obstack ()
   { obstack_free (this, NULL); }
 
+  DISABLE_COPY_AND_ASSIGN (auto_obstack);
+
   /* Free all memory in the obstack but leave it valid for further
      allocation.  */
   void clear ()
-- 
2.17.2

Re: [PATCH 2/8] Dont' allow copying of auto_obstack

[Christian Biesinger via gdb-patches]

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

> Add DISABLE_COPY_AND_ASSIGN to struct auto_obstack, to prevent copying
> it.  Copying an auto_obstack would be a bug.
>

Typo in the subject/first line (Dont' -> Don't)


> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
>
>         * gdb_obstack.h (struct auto_obstack): Use
>         DISABLE_COPY_AND_ASSIGN.
>
> Change-Id: Ic9e5ab20acfcfa61c241fed4d99bbb1caefba3cd
> ---
>  gdb/ChangeLog     | 5 +++++
>  gdb/gdb_obstack.h | 2 ++
>  2 files changed, 7 insertions(+)
>
> diff --git a/gdb/gdb_obstack.h b/gdb/gdb_obstack.h
> index fb9295f5020..9b1d907678f 100644
> --- a/gdb/gdb_obstack.h
> +++ b/gdb/gdb_obstack.h
> @@ -125,6 +125,8 @@ struct auto_obstack : obstack
>    ~auto_obstack ()
>    { obstack_free (this, NULL); }
>
> +  DISABLE_COPY_AND_ASSIGN (auto_obstack);
> +
>    /* Free all memory in the obstack but leave it valid for further
>       allocation.  */
>    void clear ()
> --
> 2.17.2
>
>

Re: [PATCH 2/8] Dont' allow copying of auto_obstack

[Tom Tromey]

>>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes:

> On Sat, Feb 8, 2020, 10:28 Tom Tromey <tom@tromey.com> wrote:
>  Add DISABLE_COPY_AND_ASSIGN to struct auto_obstack, to prevent copying
>  it.  Copying an auto_obstack would be a bug.

Christian> Typo in the subject/first line (Dont' -> Don't)

Thanks, I fixed this.

Tom

Re: [PATCH 2/8] Dont' allow copying of auto_obstack

[Simon Marchi]

On 2020-02-11 7:16 p.m., Tom Tromey wrote:
>>>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes:
> 
>> On Sat, Feb 8, 2020, 10:28 Tom Tromey <tom@tromey.com> wrote:
>>  Add DISABLE_COPY_AND_ASSIGN to struct auto_obstack, to prevent copying
>>  it.  Copying an auto_obstack would be a bug.
> 
> Christian> Typo in the subject/first line (Dont' -> Don't)
> 
> Thanks, I fixed this.
> 
> Tom
> 

That makes sense, I think you should push this one right away, independently
from this series.

Simon

Re: [PATCH 2/8] Dont' allow copying of auto_obstack

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> That makes sense, I think you should push this one right away, independently
Simon> from this series.

I'll push these first two.  Thanks.

Tom

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

[Tom Tromey]

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);
 }
 \f
-const struct objfile_key<dwarf2_fde_table> dwarf2_frame_objfile_data;
+const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
 
 \f
 
@@ -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'.  */
-- 
2.17.2

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

[Christian Biesinger via gdb-patches]

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

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

[Tom Tromey]

>>>>> "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

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

[Luis Machado]

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);
>   }
>   \f
> -const struct objfile_key<dwarf2_fde_table> dwarf2_frame_objfile_data;
> +const struct objfile_key<comp_unit> dwarf2_frame_objfile_data;
>   
>   \f
>   
> @@ -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.

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

[Tom Tromey]

>>>>> "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

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

[Simon Marchi]

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

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

[Tom Tromey]

>>>>> "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

[PATCH 7/8] Remove the objfile backlink from comp_unit

[Tom Tromey]

This removes the objfile backlink from comp_unit.  The only remaining
uses involved fetching the text offset from the objfile.  However,
this is already convenient computed at all the sites that call
execute_cfa_program, and so it can simply be passed in.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* dwarf2/frame.c (struct comp_unit) <objfile>: Remove.
	(execute_cfa_program): Add text_offset parameter.
	(execute_cfa_program_test, dwarf2_fetch_cfa_info)
	(dwarf2_frame_cache): Update.
	(dwarf2_build_frame_info): Don't set "objfile" member.

Change-Id: I81b1c82d514dad83b3d00053756a07ef54b19796
---
 gdb/ChangeLog      |  8 ++++++++
 gdb/dwarf2/frame.c | 25 +++++++++++++------------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index d4f6165b75c..2c35016e2d9 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -139,8 +139,6 @@ struct comp_unit
   /* Keep the bfd convenient.  */
   bfd *abfd = nullptr;
 
-  struct objfile *objfile = nullptr;
-
   /* Pointer to the .debug_frame section loaded into memory.  */
   const gdb_byte *dwarf_frame_buffer = nullptr;
 
@@ -349,7 +347,8 @@ Not implemented: computing unwound register using explicit value operator"));
 static const gdb_byte *
 execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
 		     const gdb_byte *insn_end, struct gdbarch *gdbarch,
-		     CORE_ADDR pc, struct dwarf2_frame_state *fs)
+		     CORE_ADDR pc, struct dwarf2_frame_state *fs,
+		     CORE_ADDR text_offset)
 {
   int eh_frame_p = fde->eh_frame_p;
   unsigned int bytes_read;
@@ -386,8 +385,8 @@ execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
 	      fs->pc = read_encoded_value (fde->cie->unit, fde->cie->encoding,
 					   fde->cie->ptr_size, insn_ptr,
 					   &bytes_read, fde->initial_location);
-	      /* Apply the objfile offset for relocatable objects.  */
-	      fs->pc += fde->cie->unit->objfile->text_section_offset ();
+	      /* Apply the text offset for relocatable objects.  */
+	      fs->pc += text_offset;
 	      insn_ptr += bytes_read;
 	      break;
 
@@ -646,7 +645,7 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
 
   const gdb_byte *insn_end = insns + sizeof (insns);
   const gdb_byte *out = execute_cfa_program (&fde, insns, insn_end, gdbarch,
-					     0, &fs);
+					     0, &fs, 0);
 
   SELF_CHECK (out == insn_end);
   SELF_CHECK (fs.pc == 0);
@@ -894,13 +893,14 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
 
   /* First decode all the insns in the CIE.  */
   execute_cfa_program (fde, fde->cie->initial_instructions,
-		       fde->cie->end, gdbarch, pc, &fs);
+		       fde->cie->end, gdbarch, pc, &fs, text_offset);
 
   /* Save the initialized register set.  */
   fs.initial = fs.regs;
 
   /* Then decode the insns in the FDE up to our target PC.  */
-  execute_cfa_program (fde, fde->instructions, fde->end, gdbarch, pc, &fs);
+  execute_cfa_program (fde, fde->instructions, fde->end, gdbarch, pc, &fs,
+		       text_offset);
 
   /* Calculate the CFA.  */
   switch (fs.regs.cfa_how)
@@ -1022,7 +1022,8 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
   /* First decode all the insns in the CIE.  */
   execute_cfa_program (fde, fde->cie->initial_instructions,
 		       fde->cie->end, gdbarch,
-		       get_frame_address_in_block (this_frame), &fs);
+		       get_frame_address_in_block (this_frame), &fs,
+		       cache->text_offset);
 
   /* Save the initialized register set.  */
   fs.initial = fs.regs;
@@ -1037,7 +1038,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
     {
       /* Decode the insns in the FDE up to the entry PC.  */
       instr = execute_cfa_program (fde, fde->instructions, fde->end, gdbarch,
-				   entry_pc, &fs);
+				   entry_pc, &fs, cache->text_offset);
 
       if (fs.regs.cfa_how == CFA_REG_OFFSET
 	  && (dwarf_reg_to_regnum (gdbarch, fs.regs.cfa_reg)
@@ -1052,7 +1053,8 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
 
   /* Then decode the insns in the FDE up to our target PC.  */
   execute_cfa_program (fde, instr, fde->end, gdbarch,
-		       get_frame_address_in_block (this_frame), &fs);
+		       get_frame_address_in_block (this_frame), &fs,
+		       cache->text_offset);
 
   try
     {
@@ -2135,7 +2137,6 @@ dwarf2_build_frame_info (struct objfile *objfile)
   /* Build a minimal decoding of the DWARF2 compilation unit.  */
   unit = new comp_unit;
   unit->abfd = objfile->obfd;
-  unit->objfile = objfile;
 
   if (objfile->separate_debug_objfile_backlink == NULL)
     {
-- 
2.17.2

Re: [PATCH 7/8] Remove the objfile backlink from comp_unit

[Christian Biesinger via gdb-patches]

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

> This removes the objfile backlink from comp_unit.  The only remaining
> uses involved fetching the text offset from the objfile.  However,
> this is already convenient computed at all the sites that call
>

convenient -> conveniently, I think?

execute_cfa_program, and so it can simply be passed in.
>
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
>
>         * dwarf2/frame.c (struct comp_unit) <objfile>: Remove.
>         (execute_cfa_program): Add text_offset parameter.
>         (execute_cfa_program_test, dwarf2_fetch_cfa_info)
>         (dwarf2_frame_cache): Update.
>         (dwarf2_build_frame_info): Don't set "objfile" member.
>
> Change-Id: I81b1c82d514dad83b3d00053756a07ef54b19796
> ---
>  gdb/ChangeLog      |  8 ++++++++
>  gdb/dwarf2/frame.c | 25 +++++++++++++------------
>  2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index d4f6165b75c..2c35016e2d9 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -139,8 +139,6 @@ struct comp_unit
>    /* Keep the bfd convenient.  */
>    bfd *abfd = nullptr;
>
> -  struct objfile *objfile = nullptr;
> -
>    /* Pointer to the .debug_frame section loaded into memory.  */
>    const gdb_byte *dwarf_frame_buffer = nullptr;
>
> @@ -349,7 +347,8 @@ Not implemented: computing unwound register using
> explicit value operator"));
>  static const gdb_byte *
>  execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
>                      const gdb_byte *insn_end, struct gdbarch *gdbarch,
> -                    CORE_ADDR pc, struct dwarf2_frame_state *fs)
> +                    CORE_ADDR pc, struct dwarf2_frame_state *fs,
> +                    CORE_ADDR text_offset)
>  {
>    int eh_frame_p = fde->eh_frame_p;
>    unsigned int bytes_read;
> @@ -386,8 +385,8 @@ execute_cfa_program (struct dwarf2_fde *fde, const
> gdb_byte *insn_ptr,
>               fs->pc = read_encoded_value (fde->cie->unit,
> fde->cie->encoding,
>                                            fde->cie->ptr_size, insn_ptr,
>                                            &bytes_read,
> fde->initial_location);
> -             /* Apply the objfile offset for relocatable objects.  */
> -             fs->pc += fde->cie->unit->objfile->text_section_offset ();
> +             /* Apply the text offset for relocatable objects.  */
> +             fs->pc += text_offset;
>               insn_ptr += bytes_read;
>               break;
>
> @@ -646,7 +645,7 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
>
>    const gdb_byte *insn_end = insns + sizeof (insns);
>    const gdb_byte *out = execute_cfa_program (&fde, insns, insn_end,
> gdbarch,
> -                                            0, &fs);
> +                                            0, &fs, 0);
>
>    SELF_CHECK (out == insn_end);
>    SELF_CHECK (fs.pc == 0);
> @@ -894,13 +893,14 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch,
> CORE_ADDR pc,
>
>    /* First decode all the insns in the CIE.  */
>    execute_cfa_program (fde, fde->cie->initial_instructions,
> -                      fde->cie->end, gdbarch, pc, &fs);
> +                      fde->cie->end, gdbarch, pc, &fs, text_offset);
>
>    /* Save the initialized register set.  */
>    fs.initial = fs.regs;
>
>    /* Then decode the insns in the FDE up to our target PC.  */
> -  execute_cfa_program (fde, fde->instructions, fde->end, gdbarch, pc,
> &fs);
> +  execute_cfa_program (fde, fde->instructions, fde->end, gdbarch, pc, &fs,
> +                      text_offset);
>
>    /* Calculate the CFA.  */
>    switch (fs.regs.cfa_how)
> @@ -1022,7 +1022,8 @@ dwarf2_frame_cache (struct frame_info *this_frame,
> void **this_cache)
>    /* First decode all the insns in the CIE.  */
>    execute_cfa_program (fde, fde->cie->initial_instructions,
>                        fde->cie->end, gdbarch,
> -                      get_frame_address_in_block (this_frame), &fs);
> +                      get_frame_address_in_block (this_frame), &fs,
> +                      cache->text_offset);
>
>    /* Save the initialized register set.  */
>    fs.initial = fs.regs;
> @@ -1037,7 +1038,7 @@ dwarf2_frame_cache (struct frame_info *this_frame,
> void **this_cache)
>      {
>        /* Decode the insns in the FDE up to the entry PC.  */
>        instr = execute_cfa_program (fde, fde->instructions, fde->end,
> gdbarch,
> -                                  entry_pc, &fs);
> +                                  entry_pc, &fs, cache->text_offset);
>
>        if (fs.regs.cfa_how == CFA_REG_OFFSET
>           && (dwarf_reg_to_regnum (gdbarch, fs.regs.cfa_reg)
> @@ -1052,7 +1053,8 @@ dwarf2_frame_cache (struct frame_info *this_frame,
> void **this_cache)
>
>    /* Then decode the insns in the FDE up to our target PC.  */
>    execute_cfa_program (fde, instr, fde->end, gdbarch,
> -                      get_frame_address_in_block (this_frame), &fs);
> +                      get_frame_address_in_block (this_frame), &fs,
> +                      cache->text_offset);
>
>    try
>      {
> @@ -2135,7 +2137,6 @@ dwarf2_build_frame_info (struct objfile *objfile)
>    /* Build a minimal decoding of the DWARF2 compilation unit.  */
>    unit = new comp_unit;
>    unit->abfd = objfile->obfd;
> -  unit->objfile = objfile;
>
>    if (objfile->separate_debug_objfile_backlink == NULL)
>      {
> --
> 2.17.2
>
>

Re: [PATCH 7/8] Remove the objfile backlink from comp_unit

[Tom Tromey]

>>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes:

> On Sat, Feb 8, 2020, 10:28 Tom Tromey <tom@tromey.com> wrote:
>  This removes the objfile backlink from comp_unit.  The only remaining
>  uses involved fetching the text offset from the objfile.  However,
>  this is already convenient computed at all the sites that call

Christian> convenient -> conveniently, I think?

Thanks, I fixed this.

Tom

Re: [PATCH 7/8] Remove the objfile backlink from comp_unit

[Luis Machado]

On 2/8/20 12:27 PM, Tom Tromey wrote:
> This removes the objfile backlink from comp_unit.  The only remaining
> uses involved fetching the text offset from the objfile.  However,
> this is already convenient computed at all the sites that call
> execute_cfa_program, and so it can simply be passed in.
> 
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/frame.c (struct comp_unit) <objfile>: Remove.
> 	(execute_cfa_program): Add text_offset parameter.
> 	(execute_cfa_program_test, dwarf2_fetch_cfa_info)
> 	(dwarf2_frame_cache): Update.
> 	(dwarf2_build_frame_info): Don't set "objfile" member.
> 
> Change-Id: I81b1c82d514dad83b3d00053756a07ef54b19796
> ---
>   gdb/ChangeLog      |  8 ++++++++
>   gdb/dwarf2/frame.c | 25 +++++++++++++------------
>   2 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index d4f6165b75c..2c35016e2d9 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -139,8 +139,6 @@ struct comp_unit
>     /* Keep the bfd convenient.  */
>     bfd *abfd = nullptr;
>   
> -  struct objfile *objfile = nullptr;
> -
>     /* Pointer to the .debug_frame section loaded into memory.  */
>     const gdb_byte *dwarf_frame_buffer = nullptr;
>   
> @@ -349,7 +347,8 @@ Not implemented: computing unwound register using explicit value operator"));
>   static const gdb_byte *
>   execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
>   		     const gdb_byte *insn_end, struct gdbarch *gdbarch,
> -		     CORE_ADDR pc, struct dwarf2_frame_state *fs)
> +		     CORE_ADDR pc, struct dwarf2_frame_state *fs,
> +		     CORE_ADDR text_offset)
>   {
>     int eh_frame_p = fde->eh_frame_p;
>     unsigned int bytes_read;
> @@ -386,8 +385,8 @@ execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
>   	      fs->pc = read_encoded_value (fde->cie->unit, fde->cie->encoding,
>   					   fde->cie->ptr_size, insn_ptr,
>   					   &bytes_read, fde->initial_location);
> -	      /* Apply the objfile offset for relocatable objects.  */
> -	      fs->pc += fde->cie->unit->objfile->text_section_offset ();
> +	      /* Apply the text offset for relocatable objects.  */
> +	      fs->pc += text_offset;
>   	      insn_ptr += bytes_read;
>   	      break;
>   
> @@ -646,7 +645,7 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
>   
>     const gdb_byte *insn_end = insns + sizeof (insns);
>     const gdb_byte *out = execute_cfa_program (&fde, insns, insn_end, gdbarch,
> -					     0, &fs);
> +					     0, &fs, 0);
>   
>     SELF_CHECK (out == insn_end);
>     SELF_CHECK (fs.pc == 0);
> @@ -894,13 +893,14 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
>   
>     /* First decode all the insns in the CIE.  */
>     execute_cfa_program (fde, fde->cie->initial_instructions,
> -		       fde->cie->end, gdbarch, pc, &fs);
> +		       fde->cie->end, gdbarch, pc, &fs, text_offset);
>   
>     /* Save the initialized register set.  */
>     fs.initial = fs.regs;
>   
>     /* Then decode the insns in the FDE up to our target PC.  */
> -  execute_cfa_program (fde, fde->instructions, fde->end, gdbarch, pc, &fs);
> +  execute_cfa_program (fde, fde->instructions, fde->end, gdbarch, pc, &fs,
> +		       text_offset);
>   
>     /* Calculate the CFA.  */
>     switch (fs.regs.cfa_how)
> @@ -1022,7 +1022,8 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
>     /* First decode all the insns in the CIE.  */
>     execute_cfa_program (fde, fde->cie->initial_instructions,
>   		       fde->cie->end, gdbarch,
> -		       get_frame_address_in_block (this_frame), &fs);
> +		       get_frame_address_in_block (this_frame), &fs,
> +		       cache->text_offset);
>   
>     /* Save the initialized register set.  */
>     fs.initial = fs.regs;
> @@ -1037,7 +1038,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
>       {
>         /* Decode the insns in the FDE up to the entry PC.  */
>         instr = execute_cfa_program (fde, fde->instructions, fde->end, gdbarch,
> -				   entry_pc, &fs);
> +				   entry_pc, &fs, cache->text_offset);
>   
>         if (fs.regs.cfa_how == CFA_REG_OFFSET
>   	  && (dwarf_reg_to_regnum (gdbarch, fs.regs.cfa_reg)
> @@ -1052,7 +1053,8 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
>   
>     /* Then decode the insns in the FDE up to our target PC.  */
>     execute_cfa_program (fde, instr, fde->end, gdbarch,
> -		       get_frame_address_in_block (this_frame), &fs);
> +		       get_frame_address_in_block (this_frame), &fs,
> +		       cache->text_offset);
>   
>     try
>       {
> @@ -2135,7 +2137,6 @@ dwarf2_build_frame_info (struct objfile *objfile)
>     /* Build a minimal decoding of the DWARF2 compilation unit.  */
>     unit = new comp_unit;
>     unit->abfd = objfile->obfd;
> -  unit->objfile = objfile;
>   
>     if (objfile->separate_debug_objfile_backlink == NULL)
>       {
> 

LGTM

[PATCH 5/8] Add per-unit obstack

[Tom Tromey]

This adds an auto_obstack to the DWARF frame comp_unit object, and
then changes the remaining code here to use the comp_unit obstack
rather than the objfile obstack.

At this point, all the storage for frame data is self-contained --
that is, it is independent of the objfile.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* dwarf2/frame.c (struct comp_unit) <obstack>: New member.
	(decode_frame_entry_1): Use the comp_unit obstack.

Change-Id: I8a1af090bcc2811762a38afbbea1512be7d952fb
---
 gdb/ChangeLog      | 5 +++++
 gdb/dwarf2/frame.c | 7 +++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 0e74b8e7e68..7e1a744513b 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -158,6 +158,9 @@ struct comp_unit
 
   /* The FDE table.  */
   dwarf2_fde_table fde_table;
+
+  /* Hold data used by this module.  */
+  auto_obstack obstack;
 };
 
 static struct dwarf2_fde *dwarf2_frame_find_fde (CORE_ADDR *pc,
@@ -1771,7 +1774,7 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
       if (find_cie (cie_table, cie_pointer))
 	return end;
 
-      cie = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_cie);
+      cie = XOBNEW (&unit->obstack, struct dwarf2_cie);
       cie->initial_instructions = NULL;
       cie->cie_pointer = cie_pointer;
 
@@ -1950,7 +1953,7 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
       if (cie_pointer >= unit->dwarf_frame_size)
 	return NULL;
 
-      fde = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_fde);
+      fde = XOBNEW (&unit->obstack, struct dwarf2_fde);
       fde->cie = find_cie (cie_table, cie_pointer);
       if (fde->cie == NULL)
 	{
-- 
2.17.2

Re: [PATCH 5/8] Add per-unit obstack

[Luis Machado]

On 2/8/20 12:27 PM, Tom Tromey wrote:
> This adds an auto_obstack to the DWARF frame comp_unit object, and
> then changes the remaining code here to use the comp_unit obstack
> rather than the objfile obstack.
> 
> At this point, all the storage for frame data is self-contained --
> that is, it is independent of the objfile.
> 
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/frame.c (struct comp_unit) <obstack>: New member.
> 	(decode_frame_entry_1): Use the comp_unit obstack.
> 
> Change-Id: I8a1af090bcc2811762a38afbbea1512be7d952fb
> ---
>   gdb/ChangeLog      | 5 +++++
>   gdb/dwarf2/frame.c | 7 +++++--
>   2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index 0e74b8e7e68..7e1a744513b 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -158,6 +158,9 @@ struct comp_unit
>   
>     /* The FDE table.  */
>     dwarf2_fde_table fde_table;
> +
> +  /* Hold data used by this module.  */
> +  auto_obstack obstack;
>   };
>   
>   static struct dwarf2_fde *dwarf2_frame_find_fde (CORE_ADDR *pc,
> @@ -1771,7 +1774,7 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
>         if (find_cie (cie_table, cie_pointer))
>   	return end;
>   
> -      cie = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_cie);
> +      cie = XOBNEW (&unit->obstack, struct dwarf2_cie);
>         cie->initial_instructions = NULL;
>         cie->cie_pointer = cie_pointer;
>   
> @@ -1950,7 +1953,7 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
>         if (cie_pointer >= unit->dwarf_frame_size)
>   	return NULL;
>   
> -      fde = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_fde);
> +      fde = XOBNEW (&unit->obstack, struct dwarf2_fde);
>         fde->cie = find_cie (cie_table, cie_pointer);
>         if (fde->cie == NULL)
>   	{
> 

LGTM.

Re: [PATCH 5/8] Add per-unit obstack

[Simon Marchi]

On 2020-02-08 10:27 a.m., Tom Tromey wrote:
> This adds an auto_obstack to the DWARF frame comp_unit object, and
> then changes the remaining code here to use the comp_unit obstack
> rather than the objfile obstack.
> 
> At this point, all the storage for frame data is self-contained --
> that is, it is independent of the objfile.

Would it be simpler to eliminate the obstack by making dwarf2_cie_table
and dwarf2_fde_table hold objects instead of pointers?  On top of that,
it should be good for performance as well, at least when doing lookups.

The only issue is that dwarf2_cie objects are referenced by address by
the FDEs, so we must make sure they don't get moved by the container.
But I believe std::unordered_map guarantees that (disabling copy and
assign on dwarf2_cie would verify it).  Or worst case, the it could
be an std::unordered_map of std::unique_ptr.

Simon

Re: [PATCH 5/8] Add per-unit obstack

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Would it be simpler to eliminate the obstack by making dwarf2_cie_table
Simon> and dwarf2_fde_table hold objects instead of pointers?  On top of that,
Simon> it should be good for performance as well, at least when doing lookups.

Simon> The only issue is that dwarf2_cie objects are referenced by address by
Simon> the FDEs, so we must make sure they don't get moved by the container.
Simon> But I believe std::unordered_map guarantees that (disabling copy and
Simon> assign on dwarf2_cie would verify it).  Or worst case, the it could
Simon> be an std::unordered_map of std::unique_ptr.

I think I would rather defer this change, like the other one, since it's
not directly related to the purpose of the series.

Tom

Re: [PATCH 5/8] Add per-unit obstack

[Simon Marchi]

On 2020-02-12 5:41 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> Would it be simpler to eliminate the obstack by making dwarf2_cie_table
> Simon> and dwarf2_fde_table hold objects instead of pointers?  On top of that,
> Simon> it should be good for performance as well, at least when doing lookups.
> 
> Simon> The only issue is that dwarf2_cie objects are referenced by address by
> Simon> the FDEs, so we must make sure they don't get moved by the container.
> Simon> But I believe std::unordered_map guarantees that (disabling copy and
> Simon> assign on dwarf2_cie would verify it).  Or worst case, the it could
> Simon> be an std::unordered_map of std::unique_ptr.
> 
> I think I would rather defer this change, like the other one, since it's
> not directly related to the purpose of the series.
> 
> Tom
> 

No problem, I suggested this in case it actually made things simpler, but I am
fine with what you have.

Simon

Re: [PATCH 5/8] Add per-unit obstack

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> No problem, I suggested this in case it actually made things
Simon> simpler, but I am fine with what you have.

Thanks.  I'm going to push this series now.

Tom

Re: [PATCH 0/8] Share DWARF frame information across inferiors

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This series arranges to share this data across objfiles, when
Tom> appropriate.  This is done in the usual way, namely attaching the data
Tom> to the BFD when such sharing is possible.

I forgot to mention this when composing the intro -- this series is
based on my earlier series to rearrange the DWARF code.

Tom