[review] Manage objfiles with shared_ptr
Tom Tromey (Code Review)
gerrit@gnutoolchain-gerrit.osci.io
Mon Nov 4 01:35:00 GMT 2019
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/501
......................................................................
Manage objfiles with shared_ptr
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-11-03 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
---
M gdb/ChangeLog
M gdb/objfiles.c
M gdb/objfiles.h
M gdb/progspace.c
M gdb/progspace.h
5 files changed, 123 insertions(+), 17 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3f2870a..bbfddb0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,21 @@
2019-11-03 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-11-03 Tom Tromey <tom@tromey.com>
+
* symfile.c (symbol_file_clear): Update.
* progspace.h (struct program_space) <free_all_objfiles>: Declare
method.
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 56854cc..81e8212 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -486,7 +486,10 @@
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 @@
objfile::unlink ()
{
current_program_space->remove_objfile (this);
- delete this;
}
/* Free all separate debug objfile of OBJFILE, but don't free OBJFILE
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index ac3037e..f825621 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -28,12 +28,14 @@
#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 @@
/* 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);
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 3cb0d4c..1d8aaea 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -175,16 +175,20 @@
/* 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 @@
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);
diff --git a/gdb/progspace.h b/gdb/progspace.h
index 6945e38..5fe2f6c 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -38,6 +38,79 @@
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 @@
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 @@
/* 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 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. */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I6fb7fbf06efb7cb7474c525908365863eae27eb3
Gerrit-Change-Number: 501
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newchange
More information about the Gdb-patches
mailing list