From patchwork Mon Nov 4 01:34:52 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: 35601 Received: (qmail 81323 invoked by alias); 4 Nov 2019 01:40:58 -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 81270 invoked by uid 89); 4 Nov 2019 01:40:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.3 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=blow 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; Mon, 04 Nov 2019 01:40:55 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id C163C20D4C; Sun, 3 Nov 2019 20:34:56 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [8.43.85.239]) by mx1.osci.io (Postfix) with ESMTP id EE5BF203AC for ; Sun, 3 Nov 2019 20:34:52 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id D027D2816D for ; Sun, 3 Nov 2019 20:34:52 -0500 (EST) X-Gerrit-PatchSet: 1 Date: Sun, 3 Nov 2019 20:34:52 -0500 From: "Tom Tromey (Code Review)" To: gdb-patches@sourceware.org Message-ID: Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] Make the objfile destructor private X-Gerrit-Change-Id: I934bee70b26b8b24e1735828fb1e60fe8a05714f X-Gerrit-Change-Number: 494 X-Gerrit-ChangeURL: X-Gerrit-Commit: 1b1763544b91d093b0340804d2261cc9e8223490 References: Reply-To: tromey@sourceware.org, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-75-g9005159e5d Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/494 ...................................................................... Make the objfile destructor private The idea behind this is that, in the long run, some code will need to be able to hold onto an objfile after it is unlinked from the program space. In particular, this is needed for some functionality to be moved to worker threads -- otherwise the objfile can be deleted while still in use. So, this makes ~objfile private, replacing it with an "unlink" method, making it more obvious which operation is intended at the calling points. gdb/ChangeLog 2019-11-03 Tom Tromey * symfile.c (syms_from_objfile_1): Use objfile_up. (syms_from_objfile_1, remove_symbol_file_command): Call unlink method. (reread_symbols): Use objfile_up. * solib.c (update_solib_list, reload_shared_libraries_1): Call unlink method. * objfiles.h (struct objfile) <~objfile>: Now private. : New method. (struct objfile_deleter): New. (objfile_up): New typedef. * objfiles.c (objfile::unlink): New method. (free_objfile_separate_debug, free_all_objfiles) (objfile_purge_solibs): Use it. * jit.c (jit_unregister_code): Remove. (jit_inferior_exit_hook, jit_event_handler): Call unlink on objfile. * compile/compile-object-run.c (do_module_cleanup): Call unlink on objfile. * compile/compile-object-load.c (compile_object_load): Use objfile_up. Change-Id: I934bee70b26b8b24e1735828fb1e60fe8a05714f --- M gdb/ChangeLog M gdb/compile/compile-object-load.c M gdb/compile/compile-object-run.c M gdb/jit.c M gdb/objfiles.c M gdb/objfiles.h M gdb/solib.c M gdb/symfile.c 8 files changed, 66 insertions(+), 25 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index fbcc3de..94dcac6 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,28 @@ 2019-11-03 Tom Tromey + * symfile.c (syms_from_objfile_1): Use objfile_up. + (syms_from_objfile_1, remove_symbol_file_command): Call unlink + method. + (reread_symbols): Use objfile_up. + * solib.c (update_solib_list, reload_shared_libraries_1): Call + unlink method. + * objfiles.h (struct objfile) <~objfile>: Now private. + : New method. + (struct objfile_deleter): New. + (objfile_up): New typedef. + * objfiles.c (objfile::unlink): New method. + (free_objfile_separate_debug, free_all_objfiles) + (objfile_purge_solibs): Use it. + * jit.c (jit_unregister_code): Remove. + (jit_inferior_exit_hook, jit_event_handler): Call unlink on + objfile. + * compile/compile-object-run.c (do_module_cleanup): Call unlink on + objfile. + * compile/compile-object-load.c (compile_object_load): Use + objfile_up. + +2019-11-03 Tom Tromey + * symfile.c (symbol_file_add_with_addrs): Pass "parent" to objfile::make. * objfiles.h (struct objjfile) : No longer inline. diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c index a30c557..e219e29 100644 --- a/gdb/compile/compile-object-load.c +++ b/gdb/compile/compile-object-load.c @@ -632,9 +632,9 @@ /* SYMFILE_VERBOSE is not passed even if FROM_TTY, user is not interested in "Reading symbols from ..." message for automatically generated file. */ - std::unique_ptr objfile_holder - (symbol_file_add_from_bfd (abfd.get (), filename.get (), - 0, NULL, 0, NULL)); + objfile_up objfile_holder (symbol_file_add_from_bfd (abfd.get (), + filename.get (), + 0, NULL, 0, NULL)); objfile = objfile_holder.get (); func_sym = lookup_global_symbol_from_objfile (objfile, diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c index 8173cfe..72f3060 100644 --- a/gdb/compile/compile-object-run.c +++ b/gdb/compile/compile-object-run.c @@ -86,7 +86,7 @@ if ((objfile->flags & OBJF_USERLOADED) == 0 && (strcmp (objfile_name (objfile), data->objfile_name_string) == 0)) { - delete objfile; + objfile->unlink (); /* It may be a bit too pervasive in this dummy_frame dtor callback. */ clear_symtab_users (0); diff --git a/gdb/jit.c b/gdb/jit.c index 970f6c6..d50e2d9 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -952,15 +952,6 @@ jit_bfd_try_read_symtab (code_entry, entry_addr, gdbarch); } -/* This function unregisters JITed code and frees the corresponding - objfile. */ - -static void -jit_unregister_code (struct objfile *objfile) -{ - delete objfile; -} - /* Look up the objfile with this code entry address. */ static struct objfile * @@ -1378,7 +1369,7 @@ = (struct jit_objfile_data *) objfile_data (objf, jit_objfile_data); if (objf_data != NULL && objf_data->addr != 0) - jit_unregister_code (objf); + objf->unlink (); } } @@ -1412,7 +1403,7 @@ "entry at address: %s\n"), paddress (gdbarch, entry_addr)); else - jit_unregister_code (objf); + objf->unlink (); break; default: diff --git a/gdb/objfiles.c b/gdb/objfiles.c index 0ee5720..a635f77 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -553,6 +553,14 @@ return result; } +/* See objfiles.h. */ + +void +objfile::unlink () +{ + delete this; +} + /* Free all separate debug objfile of OBJFILE, but don't free OBJFILE itself. */ @@ -564,7 +572,7 @@ for (child = objfile->separate_debug_objfile; child;) { struct objfile *next_child = child->separate_debug_objfile_link; - delete child; + child->unlink (); child = next_child; } } @@ -687,7 +695,7 @@ gdb_assert (so->objfile == NULL); for (objfile *objfile : current_program_space->objfiles_safe ()) - delete objfile; + objfile->unlink (); clear_symtab_users (0); } @@ -996,7 +1004,7 @@ be soon. */ if (!(objf->flags & OBJF_USERLOADED) && (objf->flags & OBJF_SHARED)) - delete objf; + objf->unlink (); } } diff --git a/gdb/objfiles.h b/gdb/objfiles.h index f658a22..bd7077e 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -399,13 +399,18 @@ /* 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: /* Create an objfile. */ static objfile *make (bfd *bfd_, const char *name_, objfile_flags flags_, objfile *parent = nullptr); - ~objfile (); + /* Remove an objfile from the current program space, and free + it. */ + void unlink (); DISABLE_COPY_AND_ASSIGN (objfile); @@ -637,6 +642,20 @@ htab_up static_links; }; +/* A deleter for objfile. */ + +struct objfile_deleter +{ + void operator() (objfile *ptr) const + { + ptr->unlink (); + } +}; + +/* A unique pointer that holds an objfile. */ + +typedef std::unique_ptr objfile_up; + /* Declarations for functions defined in objfiles.c */ extern struct gdbarch *get_objfile_arch (const struct objfile *); diff --git a/gdb/solib.c b/gdb/solib.c index 17d0c4c..8141c16 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -836,7 +836,7 @@ /* Unless the user loaded it explicitly, free SO's objfile. */ if (gdb->objfile && ! (gdb->objfile->flags & OBJF_USERLOADED) && !solib_used (gdb)) - delete gdb->objfile; + gdb->objfile->unlink (); /* Some targets' section tables might be referring to sections from so->abfd; remove them. */ @@ -1318,7 +1318,7 @@ { if (so->objfile && ! (so->objfile->flags & OBJF_USERLOADED) && !solib_used (so)) - delete so->objfile; + so->objfile->unlink (); remove_target_sections (so); clear_so (so); } diff --git a/gdb/symfile.c b/gdb/symfile.c index a5d8504..eca15b4 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -940,7 +940,7 @@ if an error occurs during symbol reading. */ gdb::optional defer_clear_users; - std::unique_ptr objfile_holder (objfile); + objfile_up objfile_holder (objfile); /* If ADDRS is NULL, put together a dummy address list. We now establish the convention that an addr of zero means @@ -958,7 +958,7 @@ if (symfile_objfile != NULL) { - delete symfile_objfile; + symfile_objfile->unlink (); gdb_assert (symfile_objfile == NULL); } @@ -2438,7 +2438,7 @@ objfile_name (objf))) error (_("Not confirmed.")); - delete objf; + objf->unlink (); clear_symtab_users (0); } @@ -2495,7 +2495,7 @@ /* If we get an error, blow away this objfile (not sure if that is the correct response for things like shared libraries). */ - std::unique_ptr objfile_holder (objfile); + objfile_up objfile_holder (objfile); /* We need to do this whenever any symbols go away. */ clear_symtab_users_cleanup defer_clear_users (0);