On 11/30/2018 03:39 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
>>> What if I added a protected constructor to target_ops that took the
>>> stratum as an argument? Would there be a problem with that?
>
> Pedro> I don't see a problem with that, but, given that a target's stratum is
> Pedro> a property of the type, and not of an instance of the type, wouldn't it
> Pedro> be better to get rid of to_stratum and replace it with a virtual method?
> Pedro> I.e., when we have 10 target remote instances active, there's no need
> Pedro> for each of the instances to have their own to_stratum copy.
>
> Pedro> Like this, on top of the series.
>
> Looks good to me, thanks.
>
> Of course now perhaps the constant should just be a template argument :)
> But that seemed like a pain and there's nothing wrong with this patch.
Yeah.
I pushed the virtual method version in, as below.
From 261654bd0773bcb8d09423b37114848f38b57a7a Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 30 Nov 2018 13:41:45 +0000
Subject: [PATCH] target_ops::to_stratum -> target_ops::stratum() virtual
method
Given that a target's stratum is a property of the type, and not of an
instance of the type, get rid of to_stratum data field and replace it
with a virtual method.
I.e., when we have e.g., 10 target remote instances active, there's no
need for each of the instances to have their own to_stratum copy.
gdb/ChangeLog:
2018-11-30 Pedro Alves <palves@redhat.com>
* aix-thread.c (aix_thread_target) <aix_thread_target>: Delete.
<stratum>: New override.
* bfd-target.c (aix_thread_target) <aix_thread_target>: Delete.
<stratum>: New override.
* bsd-uthread.c (bsd_uthread_target) <bsd_uthread_target>: Delete.
<stratum>: New override.
* exec.c (exec_target) <exec_target>: Delete.
<stratum>: New override.
* gdbarch-selftests.c (register_to_value_test): Adjust to use the
stratum method instead of the to_stratum field.
* linux-thread-db.c (thread_db_target) <thread_db_target>: Delete.
<stratum>: New override.
(thread_db_target::thread_db_target): Delete.
* make-target-delegates (print_class): Don't print a ctor
declaration. Print a stratum method override declaration.
* process-stratum-target.h (process_stratum_target)
<process_stratum_target>: Delete.
<stratum>: New override.
* ravenscar-thread.c (ravenscar_thread_target)
<ravenscar_thread_target>: Delete.
<stratum>: New override.
* record-btrace.c (record_btrace_target)
<record_btrace_target>: Delete.
<stratum>: New override.
* record-full.c (record_full_base_target)
<record_full_base_target>: Delete.
<stratum>: New override.
* record.c (record_disconnect, record_detach)
(record_mourn_inferior, record_kill): Adjust to use the stratum
method instead of the to_stratum field.
* regcache.c (cooked_read_test, cooked_write_test): Likewise.
* sol-thread.c (sol_thread_target)
<sol_thread_target>: Delete.
<stratum>: New override.
* spu-multiarch.c (spu_multiarch_target)
<spu_multiarch_target>: Delete.
<stratum>: New override.
* target-delegates.c: Regenerate.
* target.c (target_stack::push, target_stack::unpush)
(pop_all_targets_above, pop_all_targets_at_and_above)
(info_target_command, target_require_runnable)
(target_stack::find_beneath): Adjust to use the stratum method
instead of the to_stratum field.
(dummy_target::dummy_target): Delete.
(dummy_target::stratum): New.
(debug_target::debug_target): Delete.
(debug_target::stratum): New.
(maintenance_print_target_stack): Adjust to use the stratum method
instead of the to_stratum field.
* target.h (struct target_ops) <stratum>: New method.
<to_stratum>: Delete.
<is_pushed>: Adjust to use the stratum method
instead of the to_stratum field.
---
gdb/aix-thread.c | 5 ++---
gdb/bfd-target.c | 3 ++-
gdb/bsd-uthread.c | 5 ++---
gdb/exec.c | 5 ++---
gdb/gdbarch-selftests.c | 2 +-
gdb/linux-thread-db.c | 9 ++-------
gdb/make-target-delegates | 4 ++--
gdb/process-stratum-target.h | 7 ++-----
gdb/ravenscar-thread.c | 5 ++---
gdb/record-btrace.c | 5 ++---
gdb/record-full.c | 5 ++---
gdb/record.c | 8 ++++----
gdb/regcache.c | 4 ++--
gdb/sol-thread.c | 5 ++---
gdb/spu-multiarch.c | 5 ++---
gdb/target-delegates.c | 8 ++++----
gdb/target.c | 48 +++++++++++++++++++++++++-------------------
gdb/target.h | 6 ++++--
18 files changed, 66 insertions(+), 73 deletions(-)
@@ -117,12 +117,11 @@ static const target_info aix_thread_target_info = {
class aix_thread_target final : public target_ops
{
public:
- aix_thread_target ()
- { to_stratum = thread_stratum; }
-
const target_info &info () const override
{ return aix_thread_target_info; }
+ strata stratum () const override { return thread_stratum; }
+
void detach (inferior *, int) override;
void resume (ptid_t, int, enum gdb_signal) override;
ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
@@ -40,6 +40,8 @@ public:
const target_info &info () const override
{ return target_bfd_target_info; }
+ strata stratum () const override { return file_stratum; }
+
void close () override;
target_xfer_status
@@ -92,7 +94,6 @@ target_bfd::get_section_table ()
target_bfd::target_bfd (struct bfd *abfd)
: m_bfd (gdb_bfd_ref_ptr::new_reference (abfd))
{
- this->to_stratum = file_stratum;
m_table.sections = NULL;
m_table.sections_end = NULL;
build_section_table (abfd, &m_table.sections, &m_table.sections_end);
@@ -41,12 +41,11 @@ static const target_info bsd_uthread_target_info = {
struct bsd_uthread_target final : public target_ops
{
- bsd_uthread_target ()
- { to_stratum = thread_stratum; }
-
const target_info &info () const override
{ return bsd_uthread_target_info; }
+ strata stratum () const override { return thread_stratum; }
+
void close () override;
void mourn_inferior () override;
@@ -60,12 +60,11 @@ Specify the filename of the executable file.")
struct exec_target final : public target_ops
{
- exec_target ()
- { to_stratum = file_stratum; }
-
const target_info &info () const override
{ return exec_target_info; }
+ strata stratum () const override { return file_stratum; }
+
void close () override;
enum target_xfer_status xfer_partial (enum target_object object,
const char *annex,
@@ -72,7 +72,7 @@ register_to_value_test (struct gdbarch *gdbarch)
/* Error out if debugging something, because we're going to push the
test target, which would pop any existing target. */
- if (current_top_target ()->to_stratum >= process_stratum)
+ if (current_top_target ()->stratum () >= process_stratum)
error (_("target already pushed"));
/* Create a mock environment. An inferior with a thread, with a
@@ -85,11 +85,11 @@ static const target_info thread_db_target_info = {
class thread_db_target final : public target_ops
{
public:
- thread_db_target ();
-
const target_info &info () const override
{ return thread_db_target_info; }
+ strata stratum () const override { return thread_stratum; }
+
void detach (inferior *, int) override;
ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
void resume (ptid_t, int, enum gdb_signal) override;
@@ -107,11 +107,6 @@ public:
inferior *inf) override;
};
-thread_db_target::thread_db_target ()
-{
- this->to_stratum = thread_stratum;
-}
-
static char *libthread_db_search_path;
/* Set to non-zero if thread_db auto-loading is enabled
@@ -390,10 +390,10 @@ sub print_class($) {
print "struct " . $name . " : public target_ops\n";
print "{\n";
- print " $name ();\n";
- print "\n";
print " const target_info &info () const override;\n";
print "\n";
+ print " strata stratum () const override;\n";
+ print "\n";
for $name (@delegators) {
my $return_type = $return_types{$name};
@@ -27,13 +27,10 @@
class process_stratum_target : public target_ops
{
public:
- process_stratum_target ()
- {
- to_stratum = process_stratum;
- }
-
~process_stratum_target () override = 0;
+ strata stratum () const override { return process_stratum; }
+
/* We must default these because they must be implemented by any
target that can run. */
bool can_async_p () override { return false; }
@@ -81,12 +81,11 @@ static const target_info ravenscar_target_info = {
struct ravenscar_thread_target final : public target_ops
{
- ravenscar_thread_target ()
- { to_stratum = thread_stratum; }
-
const target_info &info () const override
{ return ravenscar_target_info; }
+ strata stratum () const override { return thread_stratum; }
+
ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
void resume (ptid_t, int, enum gdb_signal) override;
@@ -53,12 +53,11 @@ static const target_info record_btrace_target_info = {
class record_btrace_target final : public target_ops
{
public:
- record_btrace_target ()
- { to_stratum = record_stratum; }
-
const target_info &info () const override
{ return record_btrace_target_info; }
+ strata stratum () const override { return record_stratum; }
+
void close () override;
void async (int) override;
@@ -218,11 +218,10 @@ static const char record_doc[]
class record_full_base_target : public target_ops
{
public:
- record_full_base_target ()
- { to_stratum = record_stratum; }
-
const target_info &info () const override = 0;
+ strata stratum () const override { return record_stratum; }
+
void close () override;
void async (int) override;
ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
@@ -174,7 +174,7 @@ record_unpush (struct target_ops *t)
void
record_disconnect (struct target_ops *t, const char *args, int from_tty)
{
- gdb_assert (t->to_stratum == record_stratum);
+ gdb_assert (t->stratum () == record_stratum);
DEBUG ("disconnect %s", t->shortname ());
@@ -189,7 +189,7 @@ record_disconnect (struct target_ops *t, const char *args, int from_tty)
void
record_detach (struct target_ops *t, inferior *inf, int from_tty)
{
- gdb_assert (t->to_stratum == record_stratum);
+ gdb_assert (t->stratum () == record_stratum);
DEBUG ("detach %s", t->shortname ());
@@ -204,7 +204,7 @@ record_detach (struct target_ops *t, inferior *inf, int from_tty)
void
record_mourn_inferior (struct target_ops *t)
{
- gdb_assert (t->to_stratum == record_stratum);
+ gdb_assert (t->stratum () == record_stratum);
DEBUG ("mourn inferior %s", t->shortname ());
@@ -220,7 +220,7 @@ record_mourn_inferior (struct target_ops *t)
void
record_kill (struct target_ops *t)
{
- gdb_assert (t->to_stratum == record_stratum);
+ gdb_assert (t->stratum () == record_stratum);
DEBUG ("kill %s", t->shortname ());
@@ -1560,7 +1560,7 @@ cooked_read_test (struct gdbarch *gdbarch)
{
/* Error out if debugging something, because we're going to push the
test target, which would pop any existing target. */
- if (current_top_target ()->to_stratum >= process_stratum)
+ if (current_top_target ()->stratum () >= process_stratum)
error (_("target already pushed"));
/* Create a mock environment. An inferior with a thread, with a
@@ -1730,7 +1730,7 @@ cooked_write_test (struct gdbarch *gdbarch)
{
/* Error out if debugging something, because we're going to push the
test target, which would pop any existing target. */
- if (current_top_target ()->to_stratum >= process_stratum)
+ if (current_top_target ()->stratum () >= process_stratum)
error (_("target already pushed"));
/* Create a mock environment. A process_stratum target pushed. */
@@ -78,12 +78,11 @@ static const target_info thread_db_target_info = {
class sol_thread_target final : public target_ops
{
public:
- sol_thread_target ()
- { this->to_stratum = thread_stratum; }
-
const target_info &info () const override
{ return thread_db_target_info; }
+ strata stratum () const override { return thread_stratum; }
+
void detach (inferior *, int) override;
ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
void resume (ptid_t, int, enum gdb_signal) override;
@@ -44,12 +44,11 @@ static const target_info spu_multiarch_target_info = {
struct spu_multiarch_target final : public target_ops
{
- spu_multiarch_target ()
- { to_stratum = arch_stratum; };
-
const target_info &info () const override
{ return spu_multiarch_target_info; }
+ strata stratum () const override { return arch_stratum; }
+
void mourn_inferior () override;
void fetch_registers (struct regcache *, int) override;
@@ -6,10 +6,10 @@
struct dummy_target : public target_ops
{
- dummy_target ();
-
const target_info &info () const override;
+ strata stratum () const override;
+
void post_attach (int arg0) override;
void detach (inferior *arg0, int arg1) override;
void disconnect (const char *arg0, int arg1) override;
@@ -173,10 +173,10 @@ struct dummy_target : public target_ops
struct debug_target : public target_ops
{
- debug_target ();
-
const target_info &info () const override;
+ strata stratum () const override;
+
void post_attach (int arg0) override;
void detach (inferior *arg0, int arg1) override;
void disconnect (const char *arg0, int arg1) override;
@@ -562,18 +562,20 @@ void
target_stack::push (target_ops *t)
{
/* If there's already a target at this stratum, remove it. */
- if (m_stack[t->to_stratum] != NULL)
+ strata stratum = t->stratum ();
+
+ if (m_stack[stratum] != NULL)
{
- target_ops *prev = m_stack[t->to_stratum];
- m_stack[t->to_stratum] = NULL;
+ target_ops *prev = m_stack[stratum];
+ m_stack[stratum] = NULL;
target_close (prev);
}
/* Now add the new one. */
- m_stack[t->to_stratum] = t;
+ m_stack[stratum] = t;
- if (m_top < t->to_stratum)
- m_top = t->to_stratum;
+ if (m_top < stratum)
+ m_top = stratum;
}
/* See target.h. */
@@ -597,7 +599,9 @@ unpush_target (struct target_ops *t)
bool
target_stack::unpush (target_ops *t)
{
- if (t->to_stratum == dummy_stratum)
+ strata stratum = t->stratum ();
+
+ if (stratum == dummy_stratum)
internal_error (__FILE__, __LINE__,
_("Attempt to unpush the dummy target"));
@@ -606,7 +610,7 @@ target_stack::unpush (target_ops *t)
/* Look for the specified target. Note that a target can only occur
once in the target stack. */
- if (m_stack[t->to_stratum] != t)
+ if (m_stack[stratum] != t)
{
/* If T wasn't pushed, quit. Only open targets should be
closed. */
@@ -614,10 +618,10 @@ target_stack::unpush (target_ops *t)
}
/* Unchain the target. */
- m_stack[t->to_stratum] = NULL;
+ m_stack[stratum] = NULL;
- if (m_top == t->to_stratum)
- m_top = t->beneath ()->to_stratum;
+ if (m_top == stratum)
+ m_top = t->beneath ()->stratum ();
/* Finally close the target. Note we do this after unchaining, so
any target method calls from within the target_close
@@ -645,7 +649,7 @@ unpush_target_and_assert (struct target_ops *target)
void
pop_all_targets_above (enum strata above_stratum)
{
- while ((int) (current_top_target ()->to_stratum) > (int) above_stratum)
+ while ((int) (current_top_target ()->stratum ()) > (int) above_stratum)
unpush_target_and_assert (current_top_target ());
}
@@ -654,7 +658,7 @@ pop_all_targets_above (enum strata above_stratum)
void
pop_all_targets_at_and_above (enum strata stratum)
{
- while ((int) (current_top_target ()->to_stratum) >= (int) stratum)
+ while ((int) (current_top_target ()->stratum ()) >= (int) stratum)
unpush_target_and_assert (current_top_target ());
}
@@ -1881,7 +1885,7 @@ info_target_command (const char *args, int from_tty)
if (!t->has_memory ())
continue;
- if ((int) (t->to_stratum) <= (int) dummy_stratum)
+ if ((int) (t->stratum ()) <= (int) dummy_stratum)
continue;
if (has_all_mem)
printf_unfiltered (_("\tWhile running this, "
@@ -2323,7 +2327,7 @@ target_require_runnable (void)
/* Do not worry about targets at certain strata that can not
create inferiors. Assume they will be pushed again if
necessary, and continue to the process_stratum. */
- if (t->to_stratum > process_stratum)
+ if (t->stratum () > process_stratum)
continue;
error (_("The \"%s\" target does not support \"run\". "
@@ -3110,7 +3114,7 @@ target_ops *
target_stack::find_beneath (const target_ops *t) const
{
/* Look for a non-empty slot at stratum levels beneath T's. */
- for (int stratum = t->to_stratum - 1; stratum >= 0; --stratum)
+ for (int stratum = t->stratum () - 1; stratum >= 0; --stratum)
if (m_stack[stratum] != NULL)
return m_stack[stratum];
@@ -3224,14 +3228,16 @@ static const target_info dummy_target_info = {
""
};
-dummy_target::dummy_target ()
+strata
+dummy_target::stratum () const
{
- to_stratum = dummy_stratum;
+ return dummy_stratum;
}
-debug_target::debug_target ()
+strata
+debug_target::stratum () const
{
- to_stratum = debug_stratum;
+ return debug_stratum;
}
const target_info &
@@ -3779,7 +3785,7 @@ maintenance_print_target_stack (const char *cmd, int from_tty)
for (target_ops *t = current_top_target (); t != NULL; t = t->beneath ())
{
- if (t->to_stratum == debug_stratum)
+ if (t->stratum () == debug_stratum)
continue;
printf_filtered (" - %s (%s)\n", t->shortname (), t->longname ());
}
@@ -431,6 +431,9 @@ struct target_info
struct target_ops
{
+ /* Return this target's stratum. */
+ virtual strata stratum () const = 0;
+
/* To the target under this one. */
target_ops *beneath () const;
@@ -672,7 +675,6 @@ struct target_ops
TARGET_DEFAULT_IGNORE ();
virtual struct target_section_table *get_section_table ()
TARGET_DEFAULT_RETURN (NULL);
- enum strata to_stratum;
/* Provide default values for all "must have" methods. */
virtual bool has_all_memory () { return false; }
@@ -1286,7 +1288,7 @@ public:
/* Returns true if T is pushed on the target stack. */
bool is_pushed (target_ops *t) const
- { return at (t->to_stratum) == t; }
+ { return at (t->stratum ()) == t; }
/* Return the target at STRATUM. */
target_ops *at (strata stratum) const { return m_stack[stratum]; }