[14/14] Share DWARF partial symtabs

Message ID 20200215165444.32653-15-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.
This changes the DWARF reader to share partial symtabs (or indices if
they are available) across objfiles.  This has a few parts.

* The dwarf2_unshareable object is created per-objfile and is stored
  on the objfile using a registry key.  dwarf2_enter_objfile is
  modified to save and restore this pointer in the dwarf2_per_objfile.

* objfile::partial_symtabs is changed to be a shared_ptr again.  This
  lets us stash a second reference in dwarf2_per_objfile; if the DWARF
  data is being shared, we can simply copy this value to the new
  objfile.

* The dwarf2_per_objfile itself may be shared via the BFD -- using a
  new per-BFD registry key -- or not.  This depends both on whether
  some other symbol reader has found symbols, and whether any BFD
  sections require relocation.

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

	* objfiles.h (struct objfile) <partial_symtabs>: Now a
	shared_ptr.
	* dwarf2/read.h (struct dwarf2_per_objfile) <partial_symtabs>: New
	member.
	(class dwarf2_enter_objfile): Constructor no longer inline.
	<m_restore_unshared>: New member.
	* dwarf2/read.c (dwarf2_unshared_data_key, dwarf2_bfd_data_key):
	New globals.
	(dwarf2_objfile_data_key): Add comment.
	(get_dwarf2_per_objfile): Rewrite.
	(dwarf2_enter_objfile::dwarf2_enter_objfile): Move from read.h.
	Initialize m_restore_unshared.
	(dwarf2_per_objfile::dwarf2_per_objfile): Add "unshared"
	parameter.  Don't initialize objfile member.
	(dwarf2_per_objfile::~dwarf2_per_objfile): Don't deleted
	"unshareable".
	(dwarf2_has_info): Check dwarf2_bfd_data_key and
	dwarf2_unshared_data_key.
	(dwarf2_get_section_info): Use get_dwarf2_per_objfile.
	(dwarf2_build_psymtabs): Set objfile::partial_symtabs and
	short-circuit when sharing.
	(dwarf2_build_psymtabs): Set dwarf2_per_objfile::partial_symtabs.
	(dwarf2_psymtab::expand_psymtab): Use free_cached_comp_units.
---
 gdb/ChangeLog     | 26 ++++++++++++++++++
 gdb/dwarf2/read.c | 70 +++++++++++++++++++++++++++++++++++++----------
 gdb/dwarf2/read.h | 12 ++++----
 gdb/objfiles.h    |  2 +-
 4 files changed, 89 insertions(+), 21 deletions(-)

-- 
2.17.2

Comments

Luis Machado Feb. 18, 2020, 12:26 p.m. | #1
On 2/15/20 1:54 PM, Tom Tromey wrote:
> This changes the DWARF reader to share partial symtabs (or indices if

> they are available) across objfiles.  This has a few parts.

> 

> * The dwarf2_unshareable object is created per-objfile and is stored

>    on the objfile using a registry key.  dwarf2_enter_objfile is

>    modified to save and restore this pointer in the dwarf2_per_objfile.

> 

> * objfile::partial_symtabs is changed to be a shared_ptr again.  This

>    lets us stash a second reference in dwarf2_per_objfile; if the DWARF

>    data is being shared, we can simply copy this value to the new

>    objfile.

> 

> * The dwarf2_per_objfile itself may be shared via the BFD -- using a

>    new per-BFD registry key -- or not.  This depends both on whether

>    some other symbol reader has found symbols, and whether any BFD

>    sections require relocation.

> 

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

> 

> 	* objfiles.h (struct objfile) <partial_symtabs>: Now a

> 	shared_ptr.

> 	* dwarf2/read.h (struct dwarf2_per_objfile) <partial_symtabs>: New

> 	member.

> 	(class dwarf2_enter_objfile): Constructor no longer inline.

> 	<m_restore_unshared>: New member.

> 	* dwarf2/read.c (dwarf2_unshared_data_key, dwarf2_bfd_data_key):

> 	New globals.

> 	(dwarf2_objfile_data_key): Add comment.

> 	(get_dwarf2_per_objfile): Rewrite.

> 	(dwarf2_enter_objfile::dwarf2_enter_objfile): Move from read.h.

> 	Initialize m_restore_unshared.

> 	(dwarf2_per_objfile::dwarf2_per_objfile): Add "unshared"

> 	parameter.  Don't initialize objfile member.

> 	(dwarf2_per_objfile::~dwarf2_per_objfile): Don't deleted

> 	"unshareable".


"Don't delete..."

> 	(dwarf2_has_info): Check dwarf2_bfd_data_key and

> 	dwarf2_unshared_data_key.

> 	(dwarf2_get_section_info): Use get_dwarf2_per_objfile.

> 	(dwarf2_build_psymtabs): Set objfile::partial_symtabs and

> 	short-circuit when sharing.

> 	(dwarf2_build_psymtabs): Set dwarf2_per_objfile::partial_symtabs.

> 	(dwarf2_psymtab::expand_psymtab): Use free_cached_comp_units.

> ---

>   gdb/ChangeLog     | 26 ++++++++++++++++++

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

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

>   gdb/objfiles.h    |  2 +-

>   4 files changed, 89 insertions(+), 21 deletions(-)

> 

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

> index 15813b72005..7be57661296 100644

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

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

> @@ -100,7 +100,15 @@ static bool check_physname = false;

>   /* When true, do not reject deprecated .gdb_index sections.  */

>   static bool use_deprecated_index_sections = false;

>   

> -static const struct objfile_key<dwarf2_per_objfile> dwarf2_objfile_data_key;

> +/* This is used to store the data that is always per objfile.  */

> +static const objfile_key<dwarf2_unshareable> dwarf2_unshared_data_key;

> +

> +/* These are used to store the dwarf2_per_objfile; either on the

> +   objfile or on the BFD.  */

> +static const struct objfile_key<dwarf2_per_objfile>

> +  dwarf2_objfile_data_key;

> +static const struct bfd_key<dwarf2_per_objfile>

> +  dwarf2_bfd_data_key;

>   

>   /* The "aclass" indices for various kinds of computed DWARF symbols.  */

>   

> @@ -274,7 +282,18 @@ struct mapped_debug_names final : public mapped_index_base

>   dwarf2_per_objfile *

>   get_dwarf2_per_objfile (struct objfile *objfile)

>   {

> -  return dwarf2_objfile_data_key.get (objfile);

> +  dwarf2_per_objfile *result = dwarf2_bfd_data_key.get (objfile->obfd);

> +  if (result == nullptr)

> +    result = dwarf2_objfile_data_key.get (objfile);

> +  return result;

> +}

> +

> +dwarf2_enter_objfile::dwarf2_enter_objfile (struct objfile *objfile)

> +  : m_per_objfile (get_dwarf2_per_objfile (objfile)),

> +    m_restore_objfile (&m_per_objfile->objfile, objfile),

> +    m_restore_unshared (&m_per_objfile->unshareable,

> +			dwarf2_unshared_data_key.get (objfile))

> +{

>   }

>   

>   /* Default names of the debugging sections.  */

> @@ -1754,16 +1773,12 @@ line_header_eq_voidp (const void *item_lhs, const void *item_rhs)

>   dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,

>   					const dwarf2_debug_sections *names,

>   					bool can_copy_)

> -  : objfile (objfile_),

> -    can_copy (can_copy_),

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

> -       sharing yet.  */

> -    unshareable (new dwarf2_unshareable)

> +  : can_copy (can_copy_)

>   {

>     if (names == NULL)

>       names = &dwarf2_elf_names;

>   

> -  bfd *obfd = objfile->obfd;

> +  bfd *obfd = objfile_->obfd;

>   

>     for (asection *sec = obfd->sections; sec != NULL; sec = sec->next)

>       locate_sections (obfd, sec, *names);

> @@ -1780,8 +1795,6 @@ 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.  */

>   }

>   

> @@ -1841,13 +1854,26 @@ dwarf2_has_info (struct objfile *objfile,

>     if (objfile->flags & OBJF_READNEVER)

>       return 0;

>   

> +  struct dwarf2_unshareable *unshared = dwarf2_unshared_data_key.get (objfile);

> +  if (unshared == nullptr)

> +    unshared = dwarf2_unshared_data_key.emplace (objfile);

> +

>     struct dwarf2_per_objfile *dwarf2_per_objfile

>       = get_dwarf2_per_objfile (objfile);

>   

>     if (dwarf2_per_objfile == NULL)

> -    dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile,

> -							  names,

> -							  can_copy);

> +    {

> +      dwarf2_per_objfile = new ::dwarf2_per_objfile (objfile, names,

> +						     can_copy);

> +      /* We can share this if the objfile doesn't require relocations

> +	 and if there aren't partial symbols from some other

> +	 reader.  */

> +      if (!objfile_has_partial_symbols (objfile)

> +	  && !gdb_bfd_requires_relocations (objfile->obfd))

> +	dwarf2_bfd_data_key.set (objfile->obfd, dwarf2_per_objfile);

> +      else

> +	dwarf2_objfile_data_key.set (objfile, dwarf2_per_objfile);

> +    }

>   

>     return (!dwarf2_per_objfile->info.is_virtual

>   	  && dwarf2_per_objfile->info.s.section != NULL

> @@ -2006,7 +2032,7 @@ dwarf2_get_section_info (struct objfile *objfile,

>                            asection **sectp, const gdb_byte **bufp,

>                            bfd_size_type *sizep)

>   {

> -  struct dwarf2_per_objfile *data = dwarf2_objfile_data_key.get (objfile);

> +  struct dwarf2_per_objfile *data = get_dwarf2_per_objfile (objfile);

>     struct dwarf2_section_info *info;

>   

>     /* We may see an objfile without any DWARF, in which case we just

> @@ -5862,6 +5888,15 @@ dwarf2_build_psymtabs (struct objfile *objfile)

>     struct dwarf2_per_objfile *dwarf2_per_objfile

>       = get_dwarf2_per_objfile (objfile);

>   

> +  if (dwarf2_per_objfile->partial_symtabs != nullptr)

> +    {

> +      /* Partial symbols were already read, so now we can simply

> +	 attach them.  */

> +      objfile->partial_symtabs = dwarf2_per_objfile->partial_symtabs;

> +      dwarf2_resize_unshareable (dwarf2_per_objfile);

> +      return;

> +    }

> +

>     init_psymbol_list (objfile, 1024);

>   

>     try

> @@ -5882,6 +5917,11 @@ dwarf2_build_psymtabs (struct objfile *objfile)

>       {

>         exception_print (gdb_stderr, except);

>       }

> +

> +  /* Finish by setting the local reference to partial symtabs, so that

> +     we don't try to read again.  If we can't in fact share, this

> +     won't make a difference anyway.  */

> +  dwarf2_per_objfile->partial_symtabs = objfile->partial_symtabs;

>   }

>   

>   /* Find the base address of the compilation unit for range lists and

> @@ -8915,8 +8955,8 @@ dwarf2_psymtab::expand_psymtab (struct objfile *objfile)

>     if (symtab.has_value ())

>       return;

>   

> +  free_cached_comp_units freer (dwarf2_per_objfile);

>     read_dependencies (objfile);

> -

>     dw2_do_instantiate_symtab (per_cu_data, false);

>   }

>   


Spurious line removal, but it doesn't matter much.

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

> index 241f3c8e71e..891a844a673 100644

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

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

> @@ -299,6 +299,11 @@ public:

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

>     std::queue<dwarf2_queue_item> queue;

>   

> +  /* We keep a separate reference to the partial symtabs, in case we

> +     are sharing them between objfiles.  This is only set after

> +     partial symbols have been read the first time.  */

> +  std::shared_ptr<psymtab_storage> partial_symtabs;

> +

>     /* The total number of per_cu and signatured_type objects that have

>        been created for this reader.  */

>     size_t num_psymtabs = 0;

> @@ -321,11 +326,7 @@ class dwarf2_enter_objfile

>   {

>   public:

>   

> -  dwarf2_enter_objfile (struct objfile *objfile)

> -    : m_per_objfile (get_dwarf2_per_objfile (objfile)),

> -      m_restore_objfile (&m_per_objfile->objfile, objfile)

> -  {

> -  }

> +  dwarf2_enter_objfile (struct objfile *objfile);

>   

>     ~dwarf2_enter_objfile () = default;

>   

> @@ -335,6 +336,7 @@ private:

>   

>     dwarf2_per_objfile *m_per_objfile;

>     scoped_restore_tmpl<struct objfile *> m_restore_objfile;

> +  scoped_restore_tmpl<dwarf2_unshareable *> m_restore_unshared;

>   };

>   

>   /* A partial symtab specialized for DWARF.  */

> diff --git a/gdb/objfiles.h b/gdb/objfiles.h

> index b71a8a9edb8..12892f80f71 100644

> --- a/gdb/objfiles.h

> +++ b/gdb/objfiles.h

> @@ -558,7 +558,7 @@ public:

>   

>     /* The partial symbol tables.  */

>   

> -  std::unique_ptr<psymtab_storage> partial_symtabs;

> +  std::shared_ptr<psymtab_storage> partial_symtabs;

>   

>     /* The object file's BFD.  Can be null if the objfile contains only

>        minimal symbols, e.g. the run time common symbols for SunOS4.  */

> 


I've gone through the series and didn't see anything obviously wrong. I 
suppose this will get more thoroughly exercised when running in 
multi-process/multi-target mode with quite a few objfiles and libraries.

Is it currently possible to exercise such a scenario automatically? I 
suppose manually loading something like a web browser will be a good 
test as well.
Tom Tromey Feb. 21, 2020, 11:03 p.m. | #2
>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:


Luis> Is it currently possible to exercise such a scenario automatically? I
Luis> suppose manually loading something like a web browser will be a good 
Luis> test as well.

FWIW just running the gdb.multi tests caught a few bugs.

Tom

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 15813b72005..7be57661296 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -100,7 +100,15 @@  static bool check_physname = false;
 /* When true, do not reject deprecated .gdb_index sections.  */
 static bool use_deprecated_index_sections = false;
 
-static const struct objfile_key<dwarf2_per_objfile> dwarf2_objfile_data_key;
+/* This is used to store the data that is always per objfile.  */
+static const objfile_key<dwarf2_unshareable> dwarf2_unshared_data_key;
+
+/* These are used to store the dwarf2_per_objfile; either on the
+   objfile or on the BFD.  */
+static const struct objfile_key<dwarf2_per_objfile>
+  dwarf2_objfile_data_key;
+static const struct bfd_key<dwarf2_per_objfile>
+  dwarf2_bfd_data_key;
 
 /* The "aclass" indices for various kinds of computed DWARF symbols.  */
 
@@ -274,7 +282,18 @@  struct mapped_debug_names final : public mapped_index_base
 dwarf2_per_objfile *
 get_dwarf2_per_objfile (struct objfile *objfile)
 {
-  return dwarf2_objfile_data_key.get (objfile);
+  dwarf2_per_objfile *result = dwarf2_bfd_data_key.get (objfile->obfd);
+  if (result == nullptr)
+    result = dwarf2_objfile_data_key.get (objfile);
+  return result;
+}
+
+dwarf2_enter_objfile::dwarf2_enter_objfile (struct objfile *objfile)
+  : m_per_objfile (get_dwarf2_per_objfile (objfile)),
+    m_restore_objfile (&m_per_objfile->objfile, objfile),
+    m_restore_unshared (&m_per_objfile->unshareable,
+			dwarf2_unshared_data_key.get (objfile))
+{
 }
 
 /* Default names of the debugging sections.  */
@@ -1754,16 +1773,12 @@  line_header_eq_voidp (const void *item_lhs, const void *item_rhs)
 dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
 					const dwarf2_debug_sections *names,
 					bool can_copy_)
-  : objfile (objfile_),
-    can_copy (can_copy_),
-    /* Temporarily just allocate one per objfile -- we don't have
-       sharing yet.  */
-    unshareable (new dwarf2_unshareable)
+  : can_copy (can_copy_)
 {
   if (names == NULL)
     names = &dwarf2_elf_names;
 
-  bfd *obfd = objfile->obfd;
+  bfd *obfd = objfile_->obfd;
 
   for (asection *sec = obfd->sections; sec != NULL; sec = sec->next)
     locate_sections (obfd, sec, *names);
@@ -1780,8 +1795,6 @@  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.  */
 }
 
@@ -1841,13 +1854,26 @@  dwarf2_has_info (struct objfile *objfile,
   if (objfile->flags & OBJF_READNEVER)
     return 0;
 
+  struct dwarf2_unshareable *unshared = dwarf2_unshared_data_key.get (objfile);
+  if (unshared == nullptr)
+    unshared = dwarf2_unshared_data_key.emplace (objfile);
+
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
   if (dwarf2_per_objfile == NULL)
-    dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile,
-							  names,
-							  can_copy);
+    {
+      dwarf2_per_objfile = new ::dwarf2_per_objfile (objfile, names,
+						     can_copy);
+      /* We can share this if the objfile doesn't require relocations
+	 and if there aren't partial symbols from some other
+	 reader.  */
+      if (!objfile_has_partial_symbols (objfile)
+	  && !gdb_bfd_requires_relocations (objfile->obfd))
+	dwarf2_bfd_data_key.set (objfile->obfd, dwarf2_per_objfile);
+      else
+	dwarf2_objfile_data_key.set (objfile, dwarf2_per_objfile);
+    }
 
   return (!dwarf2_per_objfile->info.is_virtual
 	  && dwarf2_per_objfile->info.s.section != NULL
@@ -2006,7 +2032,7 @@  dwarf2_get_section_info (struct objfile *objfile,
                          asection **sectp, const gdb_byte **bufp,
                          bfd_size_type *sizep)
 {
-  struct dwarf2_per_objfile *data = dwarf2_objfile_data_key.get (objfile);
+  struct dwarf2_per_objfile *data = get_dwarf2_per_objfile (objfile);
   struct dwarf2_section_info *info;
 
   /* We may see an objfile without any DWARF, in which case we just
@@ -5862,6 +5888,15 @@  dwarf2_build_psymtabs (struct objfile *objfile)
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
+  if (dwarf2_per_objfile->partial_symtabs != nullptr)
+    {
+      /* Partial symbols were already read, so now we can simply
+	 attach them.  */
+      objfile->partial_symtabs = dwarf2_per_objfile->partial_symtabs;
+      dwarf2_resize_unshareable (dwarf2_per_objfile);
+      return;
+    }
+
   init_psymbol_list (objfile, 1024);
 
   try
@@ -5882,6 +5917,11 @@  dwarf2_build_psymtabs (struct objfile *objfile)
     {
       exception_print (gdb_stderr, except);
     }
+
+  /* Finish by setting the local reference to partial symtabs, so that
+     we don't try to read again.  If we can't in fact share, this
+     won't make a difference anyway.  */
+  dwarf2_per_objfile->partial_symtabs = objfile->partial_symtabs;
 }
 
 /* Find the base address of the compilation unit for range lists and
@@ -8915,8 +8955,8 @@  dwarf2_psymtab::expand_psymtab (struct objfile *objfile)
   if (symtab.has_value ())
     return;
 
+  free_cached_comp_units freer (dwarf2_per_objfile);
   read_dependencies (objfile);
-
   dw2_do_instantiate_symtab (per_cu_data, false);
 }
 
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 241f3c8e71e..891a844a673 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -299,6 +299,11 @@  public:
   /* CUs that are queued to be read.  */
   std::queue<dwarf2_queue_item> queue;
 
+  /* We keep a separate reference to the partial symtabs, in case we
+     are sharing them between objfiles.  This is only set after
+     partial symbols have been read the first time.  */
+  std::shared_ptr<psymtab_storage> partial_symtabs;
+
   /* The total number of per_cu and signatured_type objects that have
      been created for this reader.  */
   size_t num_psymtabs = 0;
@@ -321,11 +326,7 @@  class dwarf2_enter_objfile
 {
 public:
 
-  dwarf2_enter_objfile (struct objfile *objfile)
-    : m_per_objfile (get_dwarf2_per_objfile (objfile)),
-      m_restore_objfile (&m_per_objfile->objfile, objfile)
-  {
-  }
+  dwarf2_enter_objfile (struct objfile *objfile);
 
   ~dwarf2_enter_objfile () = default;
 
@@ -335,6 +336,7 @@  private:
 
   dwarf2_per_objfile *m_per_objfile;
   scoped_restore_tmpl<struct objfile *> m_restore_objfile;
+  scoped_restore_tmpl<dwarf2_unshareable *> m_restore_unshared;
 };
 
 /* A partial symtab specialized for DWARF.  */
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index b71a8a9edb8..12892f80f71 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -558,7 +558,7 @@  public:
 
   /* The partial symbol tables.  */
 
-  std::unique_ptr<psymtab_storage> partial_symtabs;
+  std::shared_ptr<psymtab_storage> partial_symtabs;
 
   /* The object file's BFD.  Can be null if the objfile contains only
      minimal symbols, e.g. the run time common symbols for SunOS4.  */