From patchwork Thu Dec 12 23:50:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 36819 Received: (qmail 129446 invoked by alias); 12 Dec 2019 23:56:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 129343 invoked by uid 89); 12 Dec 2019 23:56:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy= X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Dec 2019 23:56:52 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id B705820490; Thu, 12 Dec 2019 18:50:22 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [8.43.85.239]) by mx1.osci.io (Postfix) with ESMTP id 31DB520C1F; Thu, 12 Dec 2019 18:50:11 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id DCC2328176; Thu, 12 Dec 2019 18:50:10 -0500 (EST) X-Gerrit-PatchSet: 2 Date: Thu, 12 Dec 2019 18:50:10 -0500 From: "Sourceware to Gerrit sync (Code Review)" To: Tom Tromey , gdb-patches@sourceware.org Auto-Submitted: auto-generated X-Gerrit-MessageType: merged Subject: [pushed] Manage objfiles with shared_ptr X-Gerrit-Change-Id: I6fb7fbf06efb7cb7474c525908365863eae27eb3 X-Gerrit-Change-Number: 501 X-Gerrit-ChangeURL: X-Gerrit-Commit: 7d7167ce1b93f8bb151daa2572314987eaeb9e3c In-Reply-To: References: Reply-To: noreply@gnutoolchain-gerrit.osci.io, tromey@sourceware.org, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191212235010.DCC2328176@gnutoolchain-gerrit.osci.io> Sourceware to Gerrit sync has submitted this change. 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-12-12 Tom Tromey * progspace.h (objfile_list): New typedef. (class unwrapping_objfile_iterator) (struct unwrapping_objfile_range): Newl (struct program_space) : Change type. : Change return type. : Change type of "objfile" parameter. : 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 9c1322d..8c26bb0 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,21 @@ 2019-12-12 Tom Tromey + * progspace.h (objfile_list): New typedef. + (class unwrapping_objfile_iterator) + (struct unwrapping_objfile_range): Newl + (struct program_space) : Change type. + : Change return type. + : Change type of "objfile" parameter. + : 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 + * symfile.c (symbol_file_clear): Update. * progspace.h (struct program_space) : 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 (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 3424055..f0ee803 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -28,12 +28,14 @@ #include "registry.h" #include "gdb_bfd.h" #include "psymtab.h" +#include #include #include #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, + 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> objfile_list; + +/* An iterator that wraps an iterator over std::shared_ptr, + 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 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_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, + 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 objfiles_list; + std::list> objfiles_list; /* The set of target sections matching the sections mapped into this program space. Managed by both exec_ops and solib.c. */