[PATCH 4/4] Introduce class target_stack

Pedro Alves
2018-05-28T16:19:20Z

Currently, the target stack is represented by a singly linked list,
with target_ops having a pointer to the target beneath. This poses a
problem for multi-process / multi-target debugging. In that case, we
will naturally want multiple instances of target stacks. E.g., one
stack for inferior 1 which is debugging a core file, and another
target stack for inferior 2 which is debugging a remote process. The
problem then is in finding a target's "beneath" target, if we consider
that for some target_ops types, we'll be sharing a single target_ops
instance between several inferiors. For example, so far, I found no
need to have multiple instances of the spu_multiarch_target /
exec_target / dummy_target targets.

Thus this patch, which changes the target stack representation to an
array of pointers. For now, there's still a single global instance of
this new target_stack class, though further down in the multi-target
work, each inferior will have its own instance.

gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>

* target.h (target_ops) <beneath>: Now a method. All references
updated.
(class target_stack): New.
* target.c (g_target_stack): New.
(g_current_top_target): Delete.
(current_top_target): Get the top target out of g_target_stack.
(target_stack::push, target_stack::unpush): New.
(push_target, unpush_target): Reimplement.
(target_is_pushed): Reimplement in terms of g_target_stack.
(target_ops::beneath, target_stack::find_beneath): New.
---
gdb/target.c | 116 +++++++++++++++++++++++++++++------------------------------
gdb/target.h | 47 ++++++++++++++++++++++--
2 files changed, 103 insertions(+), 60 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index cc4f81ec98..17e7e3821a 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -119,16 +119,17 @@ static std::unordered_map<const target_info *, target_open_ftype *>
static struct target_ops *the_dummy_target;
static struct target_ops *the_debug_target;

+/* The target stack. */
+static target_stack g_target_stack;
+
/* Top of target stack. */
/* The target structure we are currently using to talk to a process
or file or whatever "inferior" we have. */

-static target_ops *g_current_top_target;
-
target_ops *
current_top_target ()
{
- return g_current_top_target;
+ return g_target_stack.top ();
}

/* Command list for target. */
@@ -631,49 +632,46 @@ default_execution_direction (struct target_ops *self)
to_execution_direction must be implemented for reverse async");
}

-/* Push a new target type into the stack of the existing target accessors,
- possibly superseding some of the existing accessors.
-
- Rather than allow an empty stack, we always have the dummy target at
- the bottom stratum, so we can call the function vectors without
- checking them. */
+/* See target.h. */

void
-push_target (struct target_ops *t)
+target_stack::push (target_ops *t)
{
- struct target_ops **cur;
-
- /* Find the proper stratum to install this target in. */
- for (cur = &g_current_top_target; (*cur) != NULL; cur = &(*cur)->m_beneath)
+ /* If there's already a target at this stratum, remove it. */
+ if (m_stack[t->to_stratum] != NULL)
{
- if ((int) (t->to_stratum) >= (int) (*cur)->to_stratum)
- break;
+ target_ops *prev = m_stack[t->to_stratum];
+ m_stack[t->to_stratum] = NULL;
+ target_close (prev);
}

- /* If there's already targets at this stratum, remove them. */
- /* FIXME: cagney/2003-10-15: I think this should be popping all
- targets to CUR, and not just those at this stratum level. */
- while ((*cur) != NULL && t->to_stratum == (*cur)->to_stratum)
- {
- /* There's already something at this stratum level. Close it,
- and un-hook it from the stack. */
- struct target_ops *tmp = (*cur);
+ /* Now add the new one. */
+ m_stack[t->to_stratum] = t;

- (*cur) = (*cur)->m_beneath;
- tmp->m_beneath = NULL;
- target_close (tmp);
- }
+ if (m_top < t->to_stratum)
+ m_top = t->to_stratum;
+}

- /* We have removed all targets in our stratum, now add the new one. */
- t->m_beneath = (*cur);
- (*cur) = t;
+/* See target.h. */
+
+void
+push_target (struct target_ops *t)
+{
+ g_target_stack.push (t);
}

-/* Remove a target_ops vector from the stack, wherever it may be.
- Return how many times it was removed (0 or 1). */
+/* See target.h. */

int
unpush_target (struct target_ops *t)
+{
+ return g_target_stack.unpush (t);
+}
+
+/* See target.h. */
+
+bool
+target_stack::unpush (target_ops *t)
{
struct target_ops **cur;
struct target_ops *tmp;
@@ -682,31 +680,30 @@ unpush_target (struct target_ops *t)
internal_error (__FILE__, __LINE__,
_("Attempt to unpush the dummy target"));

+ gdb_assert (t != NULL);
+
/* Look for the specified target. Note that we assume that a target
can only occur once in the target stack. */

- for (cur = &g_current_top_target; (*cur) != NULL; cur = &(*cur)->m_beneath)
+ if (m_stack[t->to_stratum] != t)
{
- if ((*cur) == t)
- break;
+ /* If we don't find target_ops, quit. Only open targets should be
+ closed. */
+ return false;
}

- /* If we don't find target_ops, quit. Only open targets should be
- closed. */
- if ((*cur) == NULL)
- return 0;
-
/* Unchain the target. */
- tmp = (*cur);
- (*cur) = (*cur)->m_beneath;
- tmp->m_beneath = NULL;
+ m_stack[t->to_stratum] = NULL;
+
+ if (m_top == t->to_stratum)
+ m_top = t->beneath ()->to_stratum;

/* Finally close the target. Note we do this after unchaining, so
any target method calls from within the target_close
implementation don't end up in T anymore. */
target_close (t);

- return 1;
+ return true;
}

/* Unpush TARGET and assert that it worked. */
@@ -751,13 +748,7 @@ pop_all_targets (void)
int
target_is_pushed (struct target_ops *t)
{
- for (target_ops *cur = current_top_target ();
- cur != NULL;
- cur = cur->beneath ())
- if (cur == t)
- return 1;
-
- return 0;
+ return g_target_stack.is_pushed (t);
}

/* Default implementation of to_get_thread_local_address. */
@@ -2625,7 +2616,7 @@ target_thread_address_space (ptid_t ptid)
target_ops *
target_ops::beneath () const
{
- return m_beneath;
+ return g_target_stack.find_beneath (this);
}

void
@@ -3213,16 +3204,25 @@ default_thread_architecture (struct target_ops *ops, ptid_t ptid)

/* See target.h. */

-struct target_ops *
-find_target_at (enum strata stratum)
+target_ops *
+target_stack::find_beneath (const target_ops *t) const
{
- for (target_ops *t = current_top_target (); t != NULL; t = t->beneath ())
- if (t->to_stratum == stratum)
- return t;
+ /* Look for a non-empty slot at stratum levels beneath T's. */
+ for (int stratum = t->to_stratum - 1; stratum >= 0; --stratum)
+ if (m_stack[stratum] != NULL)
+ return m_stack[stratum];

return NULL;
}

+/* See target.h. */
+
+struct target_ops *
+find_target_at (enum strata stratum)
+{
+ return g_target_stack.at (stratum);
+}
+
\f

/* See target.h */
diff --git a/gdb/target.h b/gdb/target.h
index 9dd29a641d..d1dd98912a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -61,7 +61,11 @@ struct inferior;
of variables any more (the file target is handling them and they
never get to the process target). So when you push a file target,
it goes into the file stratum, which is always below the process
- stratum. */
+ stratum.
+
+ Note that rather than allow an empty stack, we always have the
+ dummy target at the bottom stratum, so we can call the function
+ vectors without checking them. */

#include "target/target.h"
#include "target/resume.h"
@@ -425,7 +429,6 @@ struct target_info
struct target_ops
{
/* To the target under this one. */
- target_ops *m_beneath;
target_ops *beneath () const;

/* Free resources associated with the target. Note that singleton
@@ -1268,6 +1271,46 @@ extern void set_native_target (target_ops *target);
NULL. */
extern target_ops *get_native_target ();

+/* Type that manages a target stack. See description of target stacks
+ and strata at the top of the file. */
+
+class target_stack
+{
+public:
+ target_stack () = default;
+ DISABLE_COPY_AND_ASSIGN (target_stack);
+
+ /* Push a new target into the stack of the existing target
+ accessors, possibly superseding some existing accessor. */
+ void push (target_ops *t);
+
+ /* Remove a target from the stack, wherever it may be. Return true
+ if it was removed, false otherwise. */
+ bool unpush (target_ops *t);
+
+ /* Returns true if T is pushed on the target stack. */
+ bool is_pushed (target_ops *t) const
+ { return at (t->to_stratum) == t; }
+
+ /* Return the target at STRATUM. */
+ target_ops *at (strata stratum) const { return m_stack[stratum]; }
+
+ /* Return the target at the top of the stack. */
+ target_ops *top () const { return at (m_top); }
+
+ /* Find the next target down the stack from the specified target. */
+ target_ops *find_beneath (const target_ops *t) const;
+
+private:
+ /* The stratum of the top target target. */
+ enum strata m_top {};
+
+ /* The stack, represented as an array, with one slot per stratum.
+ If no target is pushed at some stratum, the corresponding slot is
+ null. */
+ target_ops *m_stack[(int) debug_stratum + 1] {};
+};
+
/* The ops structure for our "current" target process. This should
never be NULL. If there is no target, it points to the dummy_target. */

--
2.14.3