Manage objfiles with shared_ptr
authorTom Tromey <tom@tromey.com>
Sun, 3 Nov 2019 21:47:55 +0000 (14:47 -0700)
committerTom Tromey <tom@tromey.com>
Thu, 12 Dec 2019 22:50:56 +0000 (15:50 -0700)
This changes objfiles to be managed using a shared_ptr.  shared_ptr is
chosen because it enables the use of objfiles in background threads.

The simplest way to do this was to introduce a new iterator that will
return the underlying objfile, rather than a shared_ptr.  (I also
tried changing the rest of gdb to use shared_ptr, but this was quite
large; and to using intrusive reference counting, but this also was
tricky.)

gdb/ChangeLog
2019-12-12  Tom Tromey  <tom@tromey.com>

* progspace.h (objfile_list): New typedef.
(class unwrapping_objfile_iterator)
(struct unwrapping_objfile_range): Newl
(struct program_space) <objfiles_range>: Change type.
<objfiles>: Change return type.
<add_objfile>: Change type of "objfile" parameter.
<objfiles_list>: Now a list of shared_ptr.
* progspace.c (program_space::add_objfile): Change type of
"objfile".  Update.
(program_space::remove_objfile): Update.
* objfiles.h (struct objfile) <~objfile>: Make public.
* objfiles.c (objfile::make): Update.
(objfile::unlink): Don't call delete.

Change-Id: I6fb7fbf06efb7cb7474c525908365863eae27eb3

gdb/ChangeLog
gdb/objfiles.c
gdb/objfiles.h
gdb/progspace.c
gdb/progspace.h

index 9c1322d2a0da10e6907bcea8146a427702de0733..8c26bb06add7d6ffde035cfa4e68314c895a685b 100644 (file)
@@ -1,3 +1,19 @@
+2019-12-12  Tom Tromey  <tom@tromey.com>
+
+       * progspace.h (objfile_list): New typedef.
+       (class unwrapping_objfile_iterator)
+       (struct unwrapping_objfile_range): Newl
+       (struct program_space) <objfiles_range>: Change type.
+       <objfiles>: Change return type.
+       <add_objfile>: Change type of "objfile" parameter.
+       <objfiles_list>: Now a list of shared_ptr.
+       * progspace.c (program_space::add_objfile): Change type of
+       "objfile".  Update.
+       (program_space::remove_objfile): Update.
+       * objfiles.h (struct objfile) <~objfile>: Make public.
+       * objfiles.c (objfile::make): Update.
+       (objfile::unlink): Don't call delete.
+
 2019-12-12  Tom Tromey  <tom@tromey.com>
 
        * symfile.c (symbol_file_clear): Update.
index 56854cc5c63bdf92de7263faad8f492970119134..81e82124297f088653574fe0da59f55f64189163 100644 (file)
@@ -486,7 +486,10 @@ objfile::make (bfd *bfd_, const char *name_, objfile_flags flags_,
   if (parent != nullptr)
     add_separate_debug_objfile (result, parent);
 
-  current_program_space->add_objfile (result, parent);
+  /* Using std::make_shared might be a bit nicer here, but that would
+     require making the constructor public.  */
+  current_program_space->add_objfile (std::shared_ptr<objfile> (result),
+                                     parent);
 
   /* Rebuild section map next time we need it.  */
   get_objfile_pspace_data (current_program_space)->new_objfiles_available = 1;
@@ -500,7 +503,6 @@ void
 objfile::unlink ()
 {
   current_program_space->remove_objfile (this);
-  delete this;
 }
 
 /* Free all separate debug objfile of OBJFILE, but don't free OBJFILE
index 34240558da3d8823fd6206c356f0b8652a2b0cc3..f0ee8037b6ddc374afef244479dafb71d57bdb15 100644 (file)
 #include "registry.h"
 #include "gdb_bfd.h"
 #include "psymtab.h"
+#include <atomic>
 #include <bitset>
 #include <vector>
 #include "gdbsupport/next-iterator.h"
 #include "gdbsupport/safe-iterator.h"
 #include "bcache.h"
 #include "gdbarch.h"
+#include "gdbsupport/refcounted-object.h"
 
 struct htab;
 struct objfile_data;
@@ -399,11 +401,16 @@ private:
   /* The only way to create an objfile is to call objfile::make.  */
   objfile (bfd *, const char *, objfile_flags);
 
-  /* The only way to free an objfile is via 'unlink'.  */
-  ~objfile ();
-
 public:
 
+  /* Normally you should not call delete.  Instead, call 'unlink' to
+     remove it from the program space's list.  In some cases, you may
+     need to hold a reference to an objfile that is independent of its
+     existence on the program space's list; for this case, the
+     destructor must be public so that shared_ptr can reference
+     it.  */
+  ~objfile ();
+
   /* Create an objfile.  */
   static objfile *make (bfd *bfd_, const char *name_, objfile_flags flags_,
                        objfile *parent = nullptr);
index 3cb0d4c61e3966eb571e18b92fdb7a0fa0c8906a..1d8aaea2caa8d13737ba1be5e021be422f7cc0eb 100644 (file)
@@ -175,16 +175,20 @@ program_space::free_all_objfiles ()
 /* See progspace.h.  */
 
 void
-program_space::add_objfile (struct objfile *objfile, struct objfile *before)
+program_space::add_objfile (std::shared_ptr<objfile> &&objfile,
+                           struct objfile *before)
 {
   if (before == nullptr)
-    objfiles_list.push_back (objfile);
+    objfiles_list.push_back (std::move (objfile));
   else
     {
-      auto iter = std::find (objfiles_list.begin (), objfiles_list.end (),
-                            before);
+      auto iter = std::find_if (objfiles_list.begin (), objfiles_list.end (),
+                               [=] (const std::shared_ptr<::objfile> &objf)
+                               {
+                                 return objf.get () == before;
+                               });
       gdb_assert (iter != objfiles_list.end ());
-      objfiles_list.insert (iter, objfile);
+      objfiles_list.insert (iter, std::move (objfile));
     }
 }
 
@@ -193,8 +197,11 @@ program_space::add_objfile (struct objfile *objfile, struct objfile *before)
 void
 program_space::remove_objfile (struct objfile *objfile)
 {
-  auto iter = std::find (objfiles_list.begin (), objfiles_list.end (),
-                        objfile);
+  auto iter = std::find_if (objfiles_list.begin (), objfiles_list.end (),
+                           [=] (const std::shared_ptr<::objfile> &objf)
+                           {
+                             return objf.get () == objfile;
+                           });
   gdb_assert (iter != objfiles_list.end ());
   objfiles_list.erase (iter);
 
index 6945e38eb921f41353a7699b386d2aaddb317a95..5fe2f6c755efd19491bc3701d2ba8aa105110164 100644 (file)
@@ -38,6 +38,79 @@ struct address_space;
 struct program_space_data;
 struct address_space_data;
 
+typedef std::list<std::shared_ptr<objfile>> objfile_list;
+
+/* An iterator that wraps an iterator over std::shared_ptr<objfile>,
+   and dereferences the returned object.  This is useful for iterating
+   over a list of shared pointers and returning raw pointers -- which
+   helped avoid touching a lot of code when changing how objfiles are
+   managed.  */
+
+class unwrapping_objfile_iterator
+{
+public:
+
+  typedef unwrapping_objfile_iterator self_type;
+  typedef typename ::objfile *value_type;
+  typedef typename ::objfile &reference;
+  typedef typename ::objfile **pointer;
+  typedef typename objfile_list::iterator::iterator_category iterator_category;
+  typedef typename objfile_list::iterator::difference_type difference_type;
+
+  unwrapping_objfile_iterator (const objfile_list::iterator &iter)
+    : m_iter (iter)
+  {
+  }
+
+  objfile *operator* () const
+  {
+    return m_iter->get ();
+  }
+
+  unwrapping_objfile_iterator operator++ ()
+  {
+    ++m_iter;
+    return *this;
+  }
+
+  bool operator!= (const unwrapping_objfile_iterator &other) const
+  {
+    return m_iter != other.m_iter;
+  }
+
+private:
+
+  /* The underlying iterator.  */
+  objfile_list::iterator m_iter;
+};
+
+
+/* A range that returns unwrapping_objfile_iterators.  */
+
+struct unwrapping_objfile_range
+{
+  typedef unwrapping_objfile_iterator iterator;
+
+  unwrapping_objfile_range (objfile_list &ol)
+    : m_list (ol)
+  {
+  }
+
+  iterator begin () const
+  {
+    return iterator (m_list.begin ());
+  }
+
+  iterator end () const
+  {
+    return iterator (m_list.end ());
+  }
+
+private:
+
+  objfile_list &m_list;
+};
+
 /* A program space represents a symbolic view of an address space.
    Roughly speaking, it holds all the data associated with a
    non-running-yet program (main executable, main symbols), and when
@@ -139,15 +212,15 @@ struct program_space
   program_space (address_space *aspace_);
   ~program_space ();
 
-  typedef std::list<struct objfile *> objfiles_range;
+  typedef unwrapping_objfile_range objfiles_range;
 
   /* Return an iterable object that can be used to iterate over all
      objfiles.  The basic use is in a foreach, like:
 
      for (objfile *objf : pspace->objfiles ()) { ... }  */
-  objfiles_range &objfiles ()
+  objfiles_range objfiles ()
   {
-    return objfiles_list;
+    return unwrapping_objfile_range (objfiles_list);
   }
 
   typedef basic_safe_range<objfiles_range> objfiles_safe_range;
@@ -167,7 +240,8 @@ struct program_space
   /* Add OBJFILE to the list of objfiles, putting it just before
      BEFORE.  If BEFORE is nullptr, it will go at the end of the
      list.  */
-  void add_objfile (struct objfile *objfile, struct objfile *before);
+  void add_objfile (std::shared_ptr<objfile> &&objfile,
+                   struct objfile *before);
 
   /* Remove OBJFILE from the list of objfiles.  */
   void remove_objfile (struct objfile *objfile);
@@ -234,7 +308,7 @@ struct program_space
   struct objfile *symfile_object_file = NULL;
 
   /* All known objfiles are kept in a linked list.  */
-  std::list<struct objfile *> objfiles_list;
+  std::list<std::shared_ptr<objfile>> objfiles_list;
 
   /* The set of target sections matching the sections mapped into
      this program space.  Managed by both exec_ops and solib.c.  */