[PATCH 1/2] gdb: introduce 'all_non_exited_process_targets' and 'switch_to_target_no_thread'

[PATCH 1/2] gdb: introduce 'all_non_exited_process_targets' and 'switch_to_target_no_thread'

[Tankut Baris Aktemur]

Introduce two new convenience functions:

1. all_non_exited_process_targets: returns a collection of all process
stratum targets that have non-exited inferiors on them.  Useful for
iterating targets.

2. switch_to_target_no_thread: switch the context to the first
inferior of the given target, and to no selected thread.

gdb/ChangeLog:
2020-04-30  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* process-stratum-target.h (all_non_exited_process_targets): New
	function declaration.
	(switch_to_target_no_thread): New function declaration.
	* process-stratum-target.c (all_non_exited_process_targets): New
	function implementation.
	(switch_to_target_no_thread): New function implementation.
---
 gdb/process-stratum-target.c | 25 +++++++++++++++++++++++++
 gdb/process-stratum-target.h |  9 +++++++++
 2 files changed, 34 insertions(+)

diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index f3fd9ee905d..b9ea5007273 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -20,6 +20,7 @@
 #include "defs.h"
 #include "process-stratum-target.h"
 #include "inferior.h"
+#include <set>
 
 process_stratum_target::~process_stratum_target ()
 {
@@ -83,3 +84,27 @@ process_stratum_target::has_execution (inferior *inf)
      through hoops.  */
   return inf->pid != 0;
 }
+
+/* See process-stratum-target.h.  */
+
+std::set<process_stratum_target *>
+all_non_exited_process_targets ()
+{
+  std::set<process_stratum_target *> targets;
+  for (inferior *inf : all_non_exited_inferiors ())
+    targets.insert (inf->process_target ());
+
+  return targets;
+}
+
+/* See process-stratum-target.h.  */
+
+void
+switch_to_target_no_thread (process_stratum_target *target)
+{
+  for (inferior *inf : all_inferiors (target))
+    {
+      switch_to_inferior_no_thread (inf);
+      break;
+    }
+}
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index 1be02100dcf..6cf99099a24 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -82,4 +82,13 @@ as_process_stratum_target (target_ops *target)
   return static_cast<process_stratum_target *> (target);
 }
 
+/* Return a collection of targets that have non-exited inferiors.  */
+
+extern std::set<process_stratum_target *> all_non_exited_process_targets ();
+
+/* Switch to the first inferior (and program space) of TARGET, and
+   switch to no thread selected.  */
+
+extern void switch_to_target_no_thread (process_stratum_target *target);
+
 #endif /* !defined (PROCESS_STRATUM_TARGET_H) */
-- 
2.17.1

Re: [PATCH 1/2] gdb: introduce 'all_non_exited_process_targets' and 'switch_to_target_no_thread'

[Christian Biesinger]

On Sun, May 3, 2020 at 10:37 AM Tankut Baris Aktemur via Gdb-patches
<gdb-patches@sourceware.org> wrote:
> @@ -83,3 +84,27 @@ process_stratum_target::has_execution (inferior *inf)
>       through hoops.  */
>    return inf->pid != 0;
>  }
> +
> +/* See process-stratum-target.h.  */
> +
> +std::set<process_stratum_target *>
> +all_non_exited_process_targets ()
> +{
> +  std::set<process_stratum_target *> targets;
> +  for (inferior *inf : all_non_exited_inferiors ())
> +    targets.insert (inf->process_target ());
> +
> +  return targets;

Instead of creating a new set, it seems like it would be a lot more
efficient to just wrap the all_non_exited_inferiors() iterator and
return it->process_target(). What do you think?

Christian

RE: [PATCH 1/2] gdb: introduce 'all_non_exited_process_targets' and 'switch_to_target_no_thread'

[Aktemur, Tankut Baris]

On Wednesday, May 13, 2020 12:29 AM, Christian Biesinger wrote:
> On Sun, May 3, 2020 at 10:37 AM Tankut Baris Aktemur via Gdb-patches
> <gdb-patches@sourceware.org> wrote:
> > @@ -83,3 +84,27 @@ process_stratum_target::has_execution (inferior *inf)
> >       through hoops.  */
> >    return inf->pid != 0;
> >  }
> > +
> > +/* See process-stratum-target.h.  */
> > +
> > +std::set<process_stratum_target *>
> > +all_non_exited_process_targets ()
> > +{
> > +  std::set<process_stratum_target *> targets;
> > +  for (inferior *inf : all_non_exited_inferiors ())
> > +    targets.insert (inf->process_target ());
> > +
> > +  return targets;
> 
> Instead of creating a new set, it seems like it would be a lot more
> efficient to just wrap the all_non_exited_inferiors() iterator and
> return it->process_target(). What do you think?

But the inferiors may share targets and this would not eliminate duplicates from
appearing, would it?  We would still need a mechanism to remember the previously
returned process targets and skip duplicates, I think.

A remark about the code above is that the order of elements when iterating the set
is not necessarily the same order we would get when iterating inferiors (i.e. the
order of insertion into the set).  I don't think this matters, though.  And as long
as a new target is not added to or removed from the debug session, the order obtained
from each call to all_non_exited_process_targets is consistent.

-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/2] gdb: introduce 'all_non_exited_process_targets' and 'switch_to_target_no_thread'

[Christian Biesinger]

On Wed, May 13, 2020 at 3:26 AM Aktemur, Tankut Baris
<tankut.baris.aktemur@intel.com> wrote:
>
> On Wednesday, May 13, 2020 12:29 AM, Christian Biesinger wrote:
> > On Sun, May 3, 2020 at 10:37 AM Tankut Baris Aktemur via Gdb-patches
> > <gdb-patches@sourceware.org> wrote:
> > > @@ -83,3 +84,27 @@ process_stratum_target::has_execution (inferior *inf)
> > >       through hoops.  */
> > >    return inf->pid != 0;
> > >  }
> > > +
> > > +/* See process-stratum-target.h.  */
> > > +
> > > +std::set<process_stratum_target *>
> > > +all_non_exited_process_targets ()
> > > +{
> > > +  std::set<process_stratum_target *> targets;
> > > +  for (inferior *inf : all_non_exited_inferiors ())
> > > +    targets.insert (inf->process_target ());
> > > +
> > > +  return targets;
> >
> > Instead of creating a new set, it seems like it would be a lot more
> > efficient to just wrap the all_non_exited_inferiors() iterator and
> > return it->process_target(). What do you think?
>
> But the inferiors may share targets and this would not eliminate duplicates from
> appearing, would it?  We would still need a mechanism to remember the previously
> returned process targets and skip duplicates, I think.
>
> A remark about the code above is that the order of elements when iterating the set
> is not necessarily the same order we would get when iterating inferiors (i.e. the
> order of insertion into the set).  I don't think this matters, though.  And as long
> as a new target is not added to or removed from the debug session, the order obtained
> from each call to all_non_exited_process_targets is consistent.

Ah yes, that makes sense. Maybe add a comment that you're using set<>
to remove duplicates?

Christian