From 57685738614a0089fba7f79669392b8d66d7c6af Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Sun, 9 Jul 2023 18:46:53 -0600 Subject: [PATCH] Remove target_close I noticed that target_close is only called in two places: solib-svr4.c, and target_ops_ref_policy::decref. This patch fixes the former by changing target_bfd_reopen to return a target_ops_up and then fixing the sole caller. Then it removes target_close by inlining its body into the decref method. The advantage of this approach is that targets are now automatically managed. Regression tested on x86-64 Fedora 38. Approved-By: Andrew Burgess --- gdb/bfd-target.c | 4 ++-- gdb/bfd-target.h | 2 +- gdb/solib-svr4.c | 16 ++++++---------- gdb/target.c | 25 ++++++++++--------------- gdb/target.h | 9 --------- 5 files changed, 19 insertions(+), 37 deletions(-) diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c index 6b8a839b286..9e1b980957c 100644 --- a/gdb/bfd-target.c +++ b/gdb/bfd-target.c @@ -94,10 +94,10 @@ target_bfd::target_bfd (const gdb_bfd_ref_ptr &abfd) { } -target_ops * +target_ops_up target_bfd_reopen (const gdb_bfd_ref_ptr &abfd) { - return new target_bfd (abfd); + return target_ops_up (new target_bfd (abfd)); } void diff --git a/gdb/bfd-target.h b/gdb/bfd-target.h index d77f0367fd5..eb33374c6ea 100644 --- a/gdb/bfd-target.h +++ b/gdb/bfd-target.h @@ -25,6 +25,6 @@ struct target_ops; /* Given an existing BFD, re-open it as a "struct target_ops". */ -struct target_ops *target_bfd_reopen (const gdb_bfd_ref_ptr &bfd); +target_ops_up target_bfd_reopen (const gdb_bfd_ref_ptr &bfd); #endif diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 0ef83726d62..f1fa437e31a 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -2425,7 +2425,7 @@ enable_break (struct svr4_info *info, int from_tty) CORE_ADDR load_addr = 0; int load_addr_found = 0; int loader_found_in_list = 0; - struct target_ops *tmp_bfd_target; + target_ops_up tmp_bfd_target; sym_addr = 0; @@ -2482,8 +2482,8 @@ enable_break (struct svr4_info *info, int from_tty) if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT)) { CORE_ADDR space_size = (CORE_ADDR) 1 << addr_bit; - CORE_ADDR tmp_entry_point = exec_entry_point (tmp_bfd.get (), - tmp_bfd_target); + CORE_ADDR tmp_entry_point + = exec_entry_point (tmp_bfd.get (), tmp_bfd_target.get ()); gdb_assert (load_addr < space_size); @@ -2512,7 +2512,8 @@ enable_break (struct svr4_info *info, int from_tty) inferior_ptid, target_gdbarch ()); load_addr = (regcache_read_pc (regcache) - - exec_entry_point (tmp_bfd.get (), tmp_bfd_target)); + - exec_entry_point (tmp_bfd.get (), + tmp_bfd_target.get ())); } if (!loader_found_in_list) @@ -2564,12 +2565,7 @@ enable_break (struct svr4_info *info, int from_tty) target, this will always produce an unrelocated value. */ sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch (), sym_addr, - tmp_bfd_target); - - /* We're done with both the temporary bfd and target. Closing - the target closes the underlying bfd, because it holds the - only remaining reference. */ - target_close (tmp_bfd_target); + tmp_bfd_target.get ()); if (sym_addr != 0) { diff --git a/gdb/target.c b/gdb/target.c index fecbc89d3ca..16f43d072cd 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -1173,7 +1173,16 @@ target_ops_ref_policy::decref (target_ops *t) { if (t->stratum () == process_stratum) connection_list_remove (as_process_stratum_target (t)); - target_close (t); + + for (inferior *inf : all_inferiors ()) + gdb_assert (!inf->target_is_pushed (t)); + + fileio_handles_invalidate_target (t); + + t->close (); + + if (targetdebug) + gdb_printf (gdb_stdlog, "closing target\n"); } } @@ -3752,20 +3761,6 @@ debug_target::info () const -void -target_close (struct target_ops *targ) -{ - for (inferior *inf : all_inferiors ()) - gdb_assert (!inf->target_is_pushed (targ)); - - fileio_handles_invalidate_target (targ); - - targ->close (); - - if (targetdebug) - gdb_printf (gdb_stdlog, "target_close ()\n"); -} - int target_thread_alive (ptid_t ptid) { diff --git a/gdb/target.h b/gdb/target.h index 5f9a2a2d97a..6ae400e2cc2 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1410,15 +1410,6 @@ extern target_ops *get_dummy_target (); extern const char *target_shortname (); -/* Does whatever cleanup is required for a target that we are no - longer going to be calling. This routine is automatically always - called after popping the target off the target stack - the target's - own methods are no longer available through the target vector. - Closing file descriptors and freeing all memory allocated memory are - typical things it should do. */ - -void target_close (struct target_ops *targ); - /* Find the correct target to use for "attach". If a target on the current stack supports attaching, then it is returned. Otherwise, the default run target is returned. */ -- 2.30.2