From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv3 4/7] gdb: remove the pop_all_targets (and friends) global functions
Date: Fri, 18 Nov 2022 16:42:55 +0000 [thread overview]
Message-ID: <4b85a8b4e635dafa1ca18e3602ba04669876b6b4.1668789658.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1668789658.git.aburgess@redhat.com>
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 | 39 ++++++++++++++++++++++++++++++++++++++
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, 64 insertions(+), 56 deletions(-)
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 0e371194ee3..d858104c1e8 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 7eb2bd97907..4bad2198af7 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -103,6 +103,45 @@ 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 69525a2e053..2917f42e624 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -377,6 +377,22 @@ class inferior : public refcounted_object,
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 ()
@@ -595,6 +611,10 @@ class inferior : public refcounted_object,
registry<inferior> 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 24da3d544d5..4db5fd18b14 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 75bc6c680d8..b2af51551ba 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1236,44 +1236,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
{
@@ -2530,7 +2492,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 e9794184f07..f5fc7276ac7 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1845,10 +1845,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.25.4
next prev parent reply other threads:[~2022-11-18 16:43 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-21 13:12 [PATCH] gdb: fix target_ops reference count for some cases Andrew Burgess
2022-09-21 15:30 ` Simon Marchi
2022-09-22 14:21 ` Andrew Burgess
2022-09-22 14:52 ` Simon Marchi
2022-09-22 15:00 ` Simon Marchi
2022-09-22 17:24 ` Andrew Burgess
2022-09-26 14:16 ` Simon Marchi
2022-10-01 20:58 ` Andrew Burgess
2022-10-02 17:04 ` [PATCHv2 0/7] " Andrew Burgess
2022-10-02 17:04 ` [PATCHv2 1/7] gdb/remote: remove some manual reference count handling Andrew Burgess
2022-10-02 17:04 ` [PATCHv2 2/7] gdb: remove decref_target Andrew Burgess
2022-10-02 17:04 ` [PATCHv2 3/7] gdb: have target_stack automate reference count handling Andrew Burgess
2022-10-02 17:04 ` [PATCHv2 4/7] gdb: remove the pop_all_targets (and friends) global functions Andrew Burgess
2022-10-05 20:49 ` Lancelot SIX
2022-10-06 11:14 ` Andrew Burgess
2022-10-02 17:04 ` [PATCHv2 5/7] gdb: ensure all targets are popped before an inferior is destructed Andrew Burgess
2022-10-02 17:04 ` [PATCHv2 6/7] gdb/maint: add core file name to 'maint info program-spaces' output Andrew Burgess
2022-10-02 17:21 ` Eli Zaretskii
2022-10-02 17:04 ` [PATCHv2 7/7] gdb: some process_stratum_target should not be shared Andrew Burgess
2022-10-02 17:21 ` Eli Zaretskii
2022-10-05 21:15 ` Lancelot SIX
2022-10-06 11:44 ` Andrew Burgess
2022-11-18 16:42 ` [PATCHv3 0/7] gdb: fix target_ops reference count for some cases Andrew Burgess
2022-11-18 16:42 ` [PATCHv3 1/7] gdb/remote: remove some manual reference count handling Andrew Burgess
2022-11-18 16:42 ` [PATCHv3 2/7] gdb: remove decref_target Andrew Burgess
2022-11-18 17:22 ` Tom Tromey
2022-11-18 16:42 ` [PATCHv3 3/7] gdb: have target_stack automate reference count handling Andrew Burgess
2022-11-18 17:25 ` Tom Tromey
2022-11-18 16:42 ` Andrew Burgess [this message]
2022-11-18 17:29 ` [PATCHv3 4/7] gdb: remove the pop_all_targets (and friends) global functions Tom Tromey
2022-11-18 16:42 ` [PATCHv3 5/7] gdb: ensure all targets are popped before an inferior is destructed Andrew Burgess
2022-11-18 16:42 ` [PATCHv3 6/7] gdb/maint: add core file name to 'maint info program-spaces' output Andrew Burgess
2022-11-18 17:03 ` Eli Zaretskii
2022-11-18 16:42 ` [PATCHv3 7/7] gdb: some process_stratum_target should not be shared Andrew Burgess
2022-11-18 17:02 ` Eli Zaretskii
2022-11-18 18:04 ` Tom Tromey
2022-12-14 13:57 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4b85a8b4e635dafa1ca18e3602ba04669876b6b4.1668789658.git.aburgess@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).