From c8181f706f5a481e9667195d1f5d1623db6cc7f2 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 22 Sep 2022 12:22:22 +0100 Subject: [PATCH] gdb: remove the pop_all_targets (and friends) global functions This commit removes the global functions pop_all_targets, pop_all_targets_above, and pop_all_targets_at_and_above, and makes them methods on the inferior class. As the pop_all_targets functions will unpush each target, which decrements the targets reference count, it is possible that the target might be closed. Right now, closing a target, in some cases, depends on the current inferior being set correctly, that is, to the inferior from which the target was popped. To facilitate this I have used switch_to_inferior_no_thread within the new methods. Previously it was the responsibility of the caller to ensure that the correct inferior was selected. In a couple of places (event-top.c and top.c) I have been able to remove a previous switch_to_inferior_no_thread call. In remote_unpush_target (remote.c) I have left the switch_to_inferior_no_thread call as it is required for the generic_mourn_inferior call. --- gdb/event-top.c | 3 +-- gdb/inferior.c | 42 +++++++++++++++++++++++++++++++++++++++ gdb/inferior.h | 20 +++++++++++++++++++ gdb/remote.c | 2 +- gdb/scoped-mock-context.h | 2 +- gdb/target.c | 40 +------------------------------------ gdb/target.h | 11 ---------- gdb/top.c | 3 +-- 8 files changed, 67 insertions(+), 56 deletions(-) diff --git a/gdb/event-top.c b/gdb/event-top.c index 29dd151f0b5..bcf80bbd7d0 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -1293,10 +1293,9 @@ async_disconnect (gdb_client_data arg) for (inferior *inf : all_inferiors ()) { - switch_to_inferior_no_thread (inf); try { - pop_all_targets (); + inf->pop_all_targets (); } catch (const gdb_exception &exception) { diff --git a/gdb/inferior.c b/gdb/inferior.c index 23cbfd63bde..683e8952d00 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -103,6 +103,48 @@ inferior::unpush_target (struct target_ops *t) return m_target_stack.unpush (t); } +/* See inferior.h. */ + +void +inferior::unpush_target_and_assert (struct target_ops *target) +{ + gdb_assert (current_inferior () == this); + + if (!unpush_target (target)) + internal_error ("pop_all_targets couldn't find target %s\n", + target->shortname ()); +} + +/* See inferior.h. */ + +void +inferior::pop_all_targets_above (enum strata stratum) +{ + /* Unpushing a target might cause it to close. Some targets currently + rely on the current_inferior being set for their ::close method, so we + temporarily switch inferior now. */ + scoped_restore_current_pspace_and_thread restore_pspace_and_thread; + switch_to_inferior_no_thread (this); + + while (top_target ()->stratum () > stratum) + unpush_target_and_assert (top_target ()); +} + +/* See inferior.h. */ + +void +inferior::pop_all_targets_at_and_above (enum strata stratum) +{ + /* Unpushing a target might cause it to close. Some targets currently + rely on the current_inferior being set for their ::close method, so we + temporarily switch inferior now. */ + scoped_restore_current_pspace_and_thread restore_pspace_and_thread; + switch_to_inferior_no_thread (this); + + while (top_target ()->stratum () >= stratum) + unpush_target_and_assert (top_target ()); +} + void inferior::set_tty (std::string terminal_name) { diff --git a/gdb/inferior.h b/gdb/inferior.h index 6fc0a30b12c..8a5339713a4 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -398,6 +398,22 @@ public: target_ops *top_target () { return m_target_stack.top (); } + /* Unpush all targets except the dummy target from m_target_stack. As + targets are removed from m_target_stack their reference count is + decremented, which may cause a target to close. */ + void pop_all_targets () + { pop_all_targets_above (dummy_stratum); } + + /* Unpush all targets above STRATUM from m_target_stack. As targets are + removed from m_target_stack their reference count is decremented, + which may cause a target to close. */ + void pop_all_targets_above (enum strata stratum); + + /* Unpush all targets at and above STRATUM from m_target_stack. As + targets are removed from m_target_stack their reference count is + decremented, which may cause a target to close. */ + void pop_all_targets_at_and_above (enum strata stratum); + /* Return the target at process_stratum level in this inferior's target stack. */ struct process_stratum_target *process_target () @@ -616,6 +632,10 @@ public: registry registry_fields; private: + + /* Unpush TARGET and assert that it worked. */ + void unpush_target_and_assert (struct target_ops *target); + /* The inferior's target stack. */ target_stack m_target_stack; diff --git a/gdb/remote.c b/gdb/remote.c index 614ee069a31..ea968ee0c59 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -5717,7 +5717,7 @@ remote_unpush_target (remote_target *target) for (inferior *inf : all_inferiors (target)) { switch_to_inferior_no_thread (inf); - pop_all_targets_at_and_above (process_stratum); + inf->pop_all_targets_at_and_above (process_stratum); generic_mourn_inferior (); } diff --git a/gdb/scoped-mock-context.h b/gdb/scoped-mock-context.h index a9895303015..87c1df0d206 100644 --- a/gdb/scoped-mock-context.h +++ b/gdb/scoped-mock-context.h @@ -71,7 +71,7 @@ struct scoped_mock_context ~scoped_mock_context () { inferior_list.erase (inferior_list.iterator_to (mock_inferior)); - pop_all_targets_at_and_above (process_stratum); + mock_inferior.pop_all_targets_at_and_above (process_stratum); } }; diff --git a/gdb/target.c b/gdb/target.c index 417d384a67e..b7cd3b9b454 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -1245,44 +1245,6 @@ target_stack::unpush (target_ops *t) return true; } -/* Unpush TARGET and assert that it worked. */ - -static void -unpush_target_and_assert (struct target_ops *target) -{ - if (!current_inferior ()->unpush_target (target)) - { - gdb_printf (gdb_stderr, - "pop_all_targets couldn't find target %s\n", - target->shortname ()); - internal_error (_("failed internal consistency check")); - } -} - -void -pop_all_targets_above (enum strata above_stratum) -{ - while ((int) (current_inferior ()->top_target ()->stratum ()) - > (int) above_stratum) - unpush_target_and_assert (current_inferior ()->top_target ()); -} - -/* See target.h. */ - -void -pop_all_targets_at_and_above (enum strata stratum) -{ - while ((int) (current_inferior ()->top_target ()->stratum ()) - >= (int) stratum) - unpush_target_and_assert (current_inferior ()->top_target ()); -} - -void -pop_all_targets (void) -{ - pop_all_targets_above (dummy_stratum); -} - void target_unpusher::operator() (struct target_ops *ops) const { @@ -2539,7 +2501,7 @@ target_preopen (int from_tty) it doesn't (which seems like a win for UDI), remove it now. */ /* Leave the exec target, though. The user may be switching from a live process to a core of the same program. */ - pop_all_targets_above (file_stratum); + current_inferior ()->pop_all_targets_above (file_stratum); target_pre_inferior (from_tty); } diff --git a/gdb/target.h b/gdb/target.h index 68446a39c1b..547ee8a3bbd 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -2389,17 +2389,6 @@ extern void target_pre_inferior (int); extern void target_preopen (int); -/* Does whatever cleanup is required to get rid of all pushed targets. */ -extern void pop_all_targets (void); - -/* Like pop_all_targets, but pops only targets whose stratum is at or - above STRATUM. */ -extern void pop_all_targets_at_and_above (enum strata stratum); - -/* Like pop_all_targets, but pops only targets whose stratum is - strictly above ABOVE_STRATUM. */ -extern void pop_all_targets_above (enum strata above_stratum); - extern CORE_ADDR target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset); diff --git a/gdb/top.c b/gdb/top.c index 742997808bd..e0e7e48cf7c 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1855,10 +1855,9 @@ quit_force (int *exit_arg, int from_tty) them all out. */ for (inferior *inf : all_inferiors ()) { - switch_to_inferior_no_thread (inf); try { - pop_all_targets (); + inf->pop_all_targets (); } catch (const gdb_exception &ex) { -- 2.30.2