[05/14] Introduce dwarf2_unshareable and move die_type_hash

Message ID 20200215165444.32653-6-tom@tromey.com
State New
Headers show
Series
  • Share DWARF partial symtabs between objfiles
Related show

Commit Message

Tom Tromey Feb. 15, 2020, 4:54 p.m.
The goal of this series is to share dwarf2_per_objfile across
objfiles.  However, this is difficult to do, because it has some
members that are inherently per-objfile.

This patch introduces a new "unshareable" object.  The idea here is
that we'll have one such object per objfile, and we'll temporarily
update the pointer when needed.  This is a bit ugly, but it's
convenient, because it doesn't require a huge rewriting of the DWARF
code.  (I would be in favor of such a rewriting, but ... it's huge.)

This patch further moves one such member, the DIE type hash, to the
new structure.

In this patch, the new object is managed by the dwarf2_per_objfile.
However, by the end of the series, this will change.

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

	* dwarf2/read.h (struct dwarf2_unshareable): New type.
	(struct dwarf2_per_objfile) <die_type_hash>: Move to
	dwarf2_unshareable.
	<unshareable>: New member.
	* dwarf2/read.c (dwarf2_per_objfile::dwarf2_per_objfile):
	Initialize "unshareable".
	(dwarf2_per_objfile::~dwarf2_per_objfile): Delete unshareable.
	(set_die_type, get_die_type_at_offset): Update.
---
 gdb/ChangeLog     | 11 +++++++++++
 gdb/dwarf2/read.c | 19 +++++++++++++------
 gdb/dwarf2/read.h | 28 +++++++++++++++++++++++-----
 3 files changed, 47 insertions(+), 11 deletions(-)

-- 
2.17.2

Comments

Luis Machado Feb. 18, 2020, 11:22 a.m. | #1
Just one nit...

On 2/15/20 1:54 PM, Tom Tromey wrote:
> The goal of this series is to share dwarf2_per_objfile across

> objfiles.  However, this is difficult to do, because it has some

> members that are inherently per-objfile.

> 

> This patch introduces a new "unshareable" object.  The idea here is

> that we'll have one such object per objfile, and we'll temporarily

> update the pointer when needed.  This is a bit ugly, but it's

> convenient, because it doesn't require a huge rewriting of the DWARF

> code.  (I would be in favor of such a rewriting, but ... it's huge.)

> 

> This patch further moves one such member, the DIE type hash, to the

> new structure.

> 

> In this patch, the new object is managed by the dwarf2_per_objfile.

> However, by the end of the series, this will change.

> 

> gdb/ChangeLog

> 2020-02-15  Tom Tromey  <tom@tromey.com>

> 

> 	* dwarf2/read.h (struct dwarf2_unshareable): New type.

> 	(struct dwarf2_per_objfile) <die_type_hash>: Move to

> 	dwarf2_unshareable.

> 	<unshareable>: New member.

> 	* dwarf2/read.c (dwarf2_per_objfile::dwarf2_per_objfile):

> 	Initialize "unshareable".

> 	(dwarf2_per_objfile::~dwarf2_per_objfile): Delete unshareable.

> 	(set_die_type, get_die_type_at_offset): Update.

> ---

>   gdb/ChangeLog     | 11 +++++++++++

>   gdb/dwarf2/read.c | 19 +++++++++++++------

>   gdb/dwarf2/read.h | 28 +++++++++++++++++++++++-----

>   3 files changed, 47 insertions(+), 11 deletions(-)

> 

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c

> index a897b11693a..cb04eb19beb 100644

> --- a/gdb/dwarf2/read.c

> +++ b/gdb/dwarf2/read.c

> @@ -1774,7 +1774,10 @@ dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,

>   					const dwarf2_debug_sections *names,

>   					bool can_copy_)

>     : objfile (objfile_),

> -    can_copy (can_copy_)

> +    can_copy (can_copy_),

> +    /* Temporarily just allocate one per objfile -- we don't have

> +       sharing yet.  */

> +    unshareable (new dwarf2_unshareable)

>   {

>     if (names == NULL)

>       names = &dwarf2_elf_names;

> @@ -1796,6 +1799,8 @@ dwarf2_per_objfile::~dwarf2_per_objfile ()

>     for (signatured_type *sig_type : all_type_units)

>       sig_type->per_cu.imported_symtabs_free ();

>   

> +  delete unshareable;

> +

>     /* Everything else should be on the objfile obstack.  */

>   }

>   

> @@ -24434,8 +24439,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)

>   			    cu->per_cu->addr_type ()))

>       add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);

>   

> -  if (dwarf2_per_objfile->die_type_hash == NULL)

> -    dwarf2_per_objfile->die_type_hash

> +  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)

> +    dwarf2_per_objfile->unshareable->die_type_hash

>         = htab_up (htab_create_alloc (127,

>   				    per_cu_offset_and_type_hash,

>   				    per_cu_offset_and_type_eq,

> @@ -24445,7 +24450,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)

>     ofs.sect_off = die->sect_off;

>     ofs.type = type;

>     slot = (struct dwarf2_per_cu_offset_and_type **)

> -    htab_find_slot (dwarf2_per_objfile->die_type_hash.get (), &ofs, INSERT);

> +    htab_find_slot (dwarf2_per_objfile->unshareable->die_type_hash.get (),

> +		    &ofs, INSERT);

>     if (*slot)

>       complaint (_("A problem internal to GDB: DIE %s has type already set"),

>   	       sect_offset_str (die->sect_off));

> @@ -24465,13 +24471,14 @@ get_die_type_at_offset (sect_offset sect_off,

>     struct dwarf2_per_cu_offset_and_type *slot, ofs;

>     struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;

>   

> -  if (dwarf2_per_objfile->die_type_hash == NULL)

> +  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)

>       return NULL;

>   

>     ofs.per_cu = per_cu;

>     ofs.sect_off = sect_off;

>     slot = ((struct dwarf2_per_cu_offset_and_type *)

> -	  htab_find (dwarf2_per_objfile->die_type_hash.get (), &ofs));

> +	  htab_find (dwarf2_per_objfile->unshareable->die_type_hash.get (),

> +		     &ofs));

>     if (slot)

>       return slot->type;

>     else

> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h

> index 5fa8dec9bfb..06a0fa3a19a 100644

> --- a/gdb/dwarf2/read.h

> +++ b/gdb/dwarf2/read.h

> @@ -66,6 +66,24 @@ struct dwarf2_queue_item

>     enum language pretend_language;

>   };

>   

> +/* Some DWARF data cannot (currently) be shared across objfiles.  Such

> +   data is stored in this object.

> +

> +   dwarf2_per_objfile holds a pointer to an instance of this object.

> +   This pointer is temporarily set when expanding CUs.  This hackish

> +   approach is due to the history of the DWARF reader.  In the past,

> +   all objects were stored per-objfile, and this object was introduced

> +   in the process of separating out the shareable and per-objfile

> +   state.  */

> +

> +struct dwarf2_unshareable

> +{

> +  /* Table mapping type DIEs to their struct type *.

> +     This is NULL if not allocated yet.


"This is nullptr"

> +     The mapping is done via (CU/TU + DIE offset) -> type.  */

> +  htab_up die_type_hash;

> +};

> +

>   /* Collection of data recorded per objfile.

>      This hangs off of dwarf2_objfile_data_key.  */

>   

> @@ -214,11 +232,6 @@ public:

>        symbols.  */

>     bool reading_partial_symbols = false;

>   

> -  /* Table mapping type DIEs to their struct type *.

> -     This is NULL if not allocated yet.

> -     The mapping is done via (CU/TU + DIE offset) -> type.  */

> -  htab_up die_type_hash;

> -

>     /* The CUs we recently read.  */

>     std::vector<dwarf2_per_cu_data *> just_read_cus;

>   

> @@ -241,6 +254,11 @@ public:

>   

>     /* CUs that are queued to be read.  */

>     std::queue<dwarf2_queue_item> queue;

> +

> +  /* State that cannot be shared across objfiles.  This is normally

> +     nullptr and is temporarily set to the correct value at the entry

> +     points of the reader.  */

> +  dwarf2_unshareable *unshareable = nullptr;

>   };

>   

>   /* Get the dwarf2_per_objfile associated to OBJFILE.  */

>
Simon Marchi Feb. 19, 2020, 4:20 a.m. | #2
On 2020-02-15 11:54 a.m., Tom Tromey wrote:
> The goal of this series is to share dwarf2_per_objfile across

> objfiles.  However, this is difficult to do, because it has some

> members that are inherently per-objfile.

> 

> This patch introduces a new "unshareable" object.  The idea here is

> that we'll have one such object per objfile, and we'll temporarily

> update the pointer when needed.  This is a bit ugly, but it's

> convenient, because it doesn't require a huge rewriting of the DWARF

> code.  (I would be in favor of such a rewriting, but ... it's huge.)

> 

> This patch further moves one such member, the DIE type hash, to the

> new structure.

> 

> In this patch, the new object is managed by the dwarf2_per_objfile.

> However, by the end of the series, this will change.


So, in the end we'll have dwarf2_per_objfile, of which there will be only
one and will be shared between the objfiles, and dwarf2_unshareable of which
there will be N (for N objfiles), which won't be shared between the objfiles?

I find it a bit confusing, because now the object called "per_objfile" is not
per objfile.

Simon
Tom Tromey Feb. 21, 2020, 10:41 p.m. | #3
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


We discussed this online but I thought I'd reply for the record.

>> In this patch, the new object is managed by the dwarf2_per_objfile.

>> However, by the end of the series, this will change.


Simon> So, in the end we'll have dwarf2_per_objfile, of which there will be only
Simon> one and will be shared between the objfiles, and dwarf2_unshareable of which
Simon> there will be N (for N objfiles), which won't be shared between the objfiles?

Yes.

Simon> I find it a bit confusing, because now the object called "per_objfile" is not
Simon> per objfile.

Yeah.  In the cover letter I mentioned perhaps renaming this object.
That could be done with sed I suppose.

Tom

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a897b11693a..cb04eb19beb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1774,7 +1774,10 @@  dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
 					const dwarf2_debug_sections *names,
 					bool can_copy_)
   : objfile (objfile_),
-    can_copy (can_copy_)
+    can_copy (can_copy_),
+    /* Temporarily just allocate one per objfile -- we don't have
+       sharing yet.  */
+    unshareable (new dwarf2_unshareable)
 {
   if (names == NULL)
     names = &dwarf2_elf_names;
@@ -1796,6 +1799,8 @@  dwarf2_per_objfile::~dwarf2_per_objfile ()
   for (signatured_type *sig_type : all_type_units)
     sig_type->per_cu.imported_symtabs_free ();
 
+  delete unshareable;
+
   /* Everything else should be on the objfile obstack.  */
 }
 
@@ -24434,8 +24439,8 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 			    cu->per_cu->addr_type ()))
     add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);
 
-  if (dwarf2_per_objfile->die_type_hash == NULL)
-    dwarf2_per_objfile->die_type_hash
+  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)
+    dwarf2_per_objfile->unshareable->die_type_hash
       = htab_up (htab_create_alloc (127,
 				    per_cu_offset_and_type_hash,
 				    per_cu_offset_and_type_eq,
@@ -24445,7 +24450,8 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   ofs.sect_off = die->sect_off;
   ofs.type = type;
   slot = (struct dwarf2_per_cu_offset_and_type **)
-    htab_find_slot (dwarf2_per_objfile->die_type_hash.get (), &ofs, INSERT);
+    htab_find_slot (dwarf2_per_objfile->unshareable->die_type_hash.get (),
+		    &ofs, INSERT);
   if (*slot)
     complaint (_("A problem internal to GDB: DIE %s has type already set"),
 	       sect_offset_str (die->sect_off));
@@ -24465,13 +24471,14 @@  get_die_type_at_offset (sect_offset sect_off,
   struct dwarf2_per_cu_offset_and_type *slot, ofs;
   struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
 
-  if (dwarf2_per_objfile->die_type_hash == NULL)
+  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)
     return NULL;
 
   ofs.per_cu = per_cu;
   ofs.sect_off = sect_off;
   slot = ((struct dwarf2_per_cu_offset_and_type *)
-	  htab_find (dwarf2_per_objfile->die_type_hash.get (), &ofs));
+	  htab_find (dwarf2_per_objfile->unshareable->die_type_hash.get (),
+		     &ofs));
   if (slot)
     return slot->type;
   else
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 5fa8dec9bfb..06a0fa3a19a 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -66,6 +66,24 @@  struct dwarf2_queue_item
   enum language pretend_language;
 };
 
+/* Some DWARF data cannot (currently) be shared across objfiles.  Such
+   data is stored in this object.
+
+   dwarf2_per_objfile holds a pointer to an instance of this object.
+   This pointer is temporarily set when expanding CUs.  This hackish
+   approach is due to the history of the DWARF reader.  In the past,
+   all objects were stored per-objfile, and this object was introduced
+   in the process of separating out the shareable and per-objfile
+   state.  */
+
+struct dwarf2_unshareable
+{
+  /* Table mapping type DIEs to their struct type *.
+     This is NULL if not allocated yet.
+     The mapping is done via (CU/TU + DIE offset) -> type.  */
+  htab_up die_type_hash;
+};
+
 /* Collection of data recorded per objfile.
    This hangs off of dwarf2_objfile_data_key.  */
 
@@ -214,11 +232,6 @@  public:
      symbols.  */
   bool reading_partial_symbols = false;
 
-  /* Table mapping type DIEs to their struct type *.
-     This is NULL if not allocated yet.
-     The mapping is done via (CU/TU + DIE offset) -> type.  */
-  htab_up die_type_hash;
-
   /* The CUs we recently read.  */
   std::vector<dwarf2_per_cu_data *> just_read_cus;
 
@@ -241,6 +254,11 @@  public:
 
   /* CUs that are queued to be read.  */
   std::queue<dwarf2_queue_item> queue;
+
+  /* State that cannot be shared across objfiles.  This is normally
+     nullptr and is temporarily set to the correct value at the entry
+     points of the reader.  */
+  dwarf2_unshareable *unshareable = nullptr;
 };
 
 /* Get the dwarf2_per_objfile associated to OBJFILE.  */