* [PATCH 0/3] Introduce process_stratum_target
@ 2018-11-27 20:22 Pedro Alves
2018-11-27 20:22 ` [PATCH 1/3] Move test_target_ops to a separate file Pedro Alves
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Pedro Alves @ 2018-11-27 20:22 UTC (permalink / raw)
To: gdb-patches
Some more easily-splittable bits from my multi-target branch.
In the branch, I did a lot of back and forth around changing the
interface of the target_ops::has_foo methods, and it annoyed me that I
had to adjust the interface in several places. At some point I came
up with these patches to try to centralize things a little more.
Pedro Alves (3):
Move test_target_ops to a separate file
Introduce process_stratum_target
Convert default_child_has_foo functions to process_stratum_target
methods
gdb/Makefile.in | 2 +
gdb/bsd-kvm.c | 6 +--
gdb/corelow.c | 8 ++--
gdb/gdbarch-selftests.c | 1 +
gdb/inf-child.c | 35 ---------------
gdb/inf-child.h | 17 ++-----
gdb/process-stratum-target.c | 100 +++++++++++++++++++++++++++++++++++++++++
gdb/process-stratum-target.h | 60 +++++++++++++++++++++++++
gdb/ravenscar-thread.c | 7 ---
gdb/regcache.c | 1 +
gdb/remote-sim.c | 15 ++-----
gdb/remote.c | 14 ++----
gdb/target-delegates.c | 4 +-
gdb/target.c | 105 -------------------------------------------
gdb/target.h | 67 ++-------------------------
gdb/test-target.c | 39 ++++++++++++++++
gdb/test-target.h | 65 +++++++++++++++++++++++++++
gdb/tracefile.c | 5 ---
gdb/tracefile.h | 6 ++-
19 files changed, 295 insertions(+), 262 deletions(-)
create mode 100644 gdb/process-stratum-target.c
create mode 100644 gdb/process-stratum-target.h
create mode 100644 gdb/test-target.c
create mode 100644 gdb/test-target.h
--
2.14.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] Move test_target_ops to a separate file
2018-11-27 20:22 [PATCH 0/3] Introduce process_stratum_target Pedro Alves
@ 2018-11-27 20:22 ` Pedro Alves
2018-11-27 20:23 ` [PATCH 2/3] Introduce process_stratum_target Pedro Alves
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2018-11-27 20:22 UTC (permalink / raw)
To: gdb-patches
There's no need to have all target.h users seeing this type.
Also helps with a follow up patch.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_SFILES): Add test-target.c.
* gdbarch-selftests.c: Include "test-target.h".
* regcache.c: Include "test-target.h".
* target.c (test_target_info, test_target_ops::info): Move to ...
* test-target.c: ... this new file.
* target.h (test_target_ops): Move to ...
* test-target.h: ... this new file.
---
gdb/Makefile.in | 1 +
gdb/gdbarch-selftests.c | 1 +
gdb/regcache.c | 1 +
gdb/target.c | 21 ---------------
gdb/target.h | 45 --------------------------------
gdb/test-target.c | 39 ++++++++++++++++++++++++++++
gdb/test-target.h | 69 +++++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 111 insertions(+), 66 deletions(-)
create mode 100644 gdb/test-target.c
create mode 100644 gdb/test-target.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3be058f052..7ec3c0a019 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1110,6 +1110,7 @@ COMMON_SFILES = \
target-dcache.c \
target-descriptions.c \
target-memory.c \
+ test-target.c \
thread.c \
thread-iter.c \
thread-fsm.c \
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index 663146f655..af97c7de2f 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -24,6 +24,7 @@
#include "inferior.h"
#include "gdbthread.h"
#include "target.h"
+#include "test-target.h"
#include "target-float.h"
#include "common/def-vector.h"
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 6e0e8c3e7e..69e42a2722 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -21,6 +21,7 @@
#include "inferior.h"
#include "gdbthread.h"
#include "target.h"
+#include "test-target.h"
#include "gdbarch.h"
#include "gdbcmd.h"
#include "regcache.h"
diff --git a/gdb/target.c b/gdb/target.c
index 29ce5eb414..7fad3a9602 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -192,27 +192,6 @@ target_command (const char *arg, int from_tty)
gdb_stdout);
}
-#if GDB_SELF_TEST
-namespace selftests {
-
-/* A mock process_stratum target_ops that doesn't read/write registers
- anywhere. */
-
-static const target_info test_target_info = {
- "test",
- N_("unit tests target"),
- N_("You should never see this"),
-};
-
-const target_info &
-test_target_ops::info () const
-{
- return test_target_info;
-}
-
-} /* namespace selftests */
-#endif /* GDB_SELF_TEST */
-
/* Default target_has_* methods for process_stratum targets. */
int
diff --git a/gdb/target.h b/gdb/target.h
index 4731e3bf79..e170bbce3c 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2574,49 +2574,4 @@ extern void target_prepare_to_generate_core (void);
/* See to_done_generating_core. */
extern void target_done_generating_core (void);
-#if GDB_SELF_TEST
-namespace selftests {
-
-/* A mock process_stratum target_ops that doesn't read/write registers
- anywhere. */
-
-class test_target_ops : public target_ops
-{
-public:
- test_target_ops ()
- : target_ops {}
- {
- to_stratum = process_stratum;
- }
-
- const target_info &info () const override;
-
- bool has_registers () override
- {
- return true;
- }
-
- bool has_stack () override
- {
- return true;
- }
-
- bool has_memory () override
- {
- return true;
- }
-
- void prepare_to_store (regcache *regs) override
- {
- }
-
- void store_registers (regcache *regs, int regno) override
- {
- }
-};
-
-
-} // namespace selftests
-#endif /* GDB_SELF_TEST */
-
#endif /* !defined (TARGET_H) */
diff --git a/gdb/test-target.c b/gdb/test-target.c
new file mode 100644
index 0000000000..3d3c950325
--- /dev/null
+++ b/gdb/test-target.c
@@ -0,0 +1,39 @@
+/* A mock process_stratum target_ops
+
+ Copyright (C) 2017-2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "test-target.h"
+
+#if GDB_SELF_TEST
+namespace selftests {
+
+static const target_info test_target_info = {
+ "test",
+ N_("unit tests target"),
+ N_("You should never see this"),
+};
+
+const target_info &
+test_target_ops::info () const
+{
+ return test_target_info;
+}
+
+} /* namespace selftests */
+#endif /* GDB_SELF_TEST */
diff --git a/gdb/test-target.h b/gdb/test-target.h
new file mode 100644
index 0000000000..b3170f8265
--- /dev/null
+++ b/gdb/test-target.h
@@ -0,0 +1,69 @@
+/* A mock process_stratum target_ops
+
+ Copyright (C) 2017-2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef TEST_TARGET_H
+#define TEST_TARGET_H
+
+#include "target.h"
+
+#if GDB_SELF_TEST
+namespace selftests {
+
+/* A mock process_stratum target_ops that doesn't read/write registers
+ anywhere. */
+
+class test_target_ops : public target_ops
+{
+public:
+ test_target_ops ()
+ : target_ops {}
+ {
+ to_stratum = process_stratum;
+ }
+
+ const target_info &info () const override;
+
+ bool has_registers () override
+ {
+ return true;
+ }
+
+ bool has_stack () override
+ {
+ return true;
+ }
+
+ bool has_memory () override
+ {
+ return true;
+ }
+
+ void prepare_to_store (regcache *regs) override
+ {
+ }
+
+ void store_registers (regcache *regs, int regno) override
+ {
+ }
+};
+
+} // namespace selftests
+#endif /* GDB_SELF_TEST */
+
+#endif /* !defined (TEST_TARGET_H) */
--
2.14.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] Introduce process_stratum_target
2018-11-27 20:22 [PATCH 0/3] Introduce process_stratum_target Pedro Alves
2018-11-27 20:22 ` [PATCH 1/3] Move test_target_ops to a separate file Pedro Alves
@ 2018-11-27 20:23 ` Pedro Alves
2018-11-29 18:26 ` Tom Tromey
2018-11-27 20:23 ` [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods Pedro Alves
2018-11-29 18:31 ` [PATCH 0/3] Introduce process_stratum_target Tom Tromey
3 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2018-11-27 20:23 UTC (permalink / raw)
To: gdb-patches
This adds a base class that all process_stratum targets inherit from.
default_thread_address_space/default_thread_architecture only make
sense for process_stratum targets, so they are transformed to
process_stratum_target methods/overrides.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_SFILES): Add process-stratum-target.c.
* bsd-kvm.c: Include "process-stratum-target.h".
(bsd_kvm_target): Now inherits from process_stratum_target.
(bsd_kvm_target::bsd_kvm_target): Default it.
* corelow.c: Include "process-stratum-target.h".
(core_target): Now inherits from process_stratum_target.
(core_target::core_target): Don't set to_stratum here.
* inf-child.c (inf_child_target::inf_child_target): Delete.
* inf-child.h: Include "process-stratum-target.h".
(inf_child_target): Inherit from process_stratum_target.
(inf_child_target) <inf_child_target>: Default it.
<can_async_p, supports_non_stop, supports_disable_randomization>:
Delete overrides.
* process-stratum-target.c: New file.
* process-stratum-target.h: New file.
* remote-sim.c: Include "process-stratum-target.h".
(gdbsim_target): Inherit from process_stratum_target.
<gdbsim_target>: Default it.
* remote.c: Include "process-stratum-target.h".
(remote_target): Inherit from process_stratum_target.
<remote_target>: Default it.
* target.c (default_thread_address_space)
(default_thread_architecture): Delete.
* target.h (target_ops) <thread_architecture>: Now returns NULL by
default.
<thread_address_space>: Ditto.
* test-target.h: Include "process-stratum-target.h" instead of
"target.h".
(test_target_ops): Inherit from process_stratum_target.
<test_target_ops>: Default it.
* tracefile.c (tracefile_target::tracefile_target): Delete.
* tracefile.h: Include "process-stratum-target.h".
(tracefile_target): Inherit from process_stratum_target.
<tracefile_target>: Default it.
* target-delegates.c: Regenerate.
---
gdb/Makefile.in | 1 +
gdb/bsd-kvm.c | 6 ++---
gdb/corelow.c | 5 ++---
gdb/inf-child.c | 5 -----
gdb/inf-child.h | 11 +++-------
gdb/process-stratum-target.c | 49 +++++++++++++++++++++++++++++++++++++++++
gdb/process-stratum-target.h | 52 ++++++++++++++++++++++++++++++++++++++++++++
gdb/remote-sim.c | 6 ++---
gdb/remote.c | 8 +++----
gdb/target-delegates.c | 4 ++--
gdb/target.c | 30 -------------------------
gdb/target.h | 13 ++++-------
gdb/test-target.h | 10 +++------
gdb/tracefile.c | 5 -----
gdb/tracefile.h | 5 +++--
15 files changed, 128 insertions(+), 82 deletions(-)
create mode 100644 gdb/process-stratum-target.c
create mode 100644 gdb/process-stratum-target.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 7ec3c0a019..1099d710c9 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1075,6 +1075,7 @@ COMMON_SFILES = \
parse.c \
printcmd.c \
probe.c \
+ process-stratum-target.c \
producer.c \
progspace.c \
progspace-and-thread.c \
diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index af8305f386..977baf9fe9 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -24,6 +24,7 @@
#include "frame.h"
#include "regcache.h"
#include "target.h"
+#include "process-stratum-target.h"
#include "value.h"
#include "gdbcore.h"
#include "inferior.h" /* for get_exec_file */
@@ -71,11 +72,10 @@ static const target_info bsd_kvm_target_info = {
Optionally specify the filename of a core dump.")
};
-class bsd_kvm_target final : public target_ops
+class bsd_kvm_target final : public process_stratum_target
{
public:
- bsd_kvm_target ()
- { this->to_stratum = process_stratum; }
+ bsd_kvm_target () = default;
const target_info &info () const override
{ return bsd_kvm_target_info; }
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 72f2807640..deabf84def 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -31,6 +31,7 @@
#include "command.h"
#include "bfd.h"
#include "target.h"
+#include "process-stratum-target.h"
#include "gdbcore.h"
#include "gdbthread.h"
#include "regcache.h"
@@ -61,7 +62,7 @@ static const target_info core_target_info = {
N_("Use a core file as a target. Specify the filename of the core file.")
};
-class core_target final : public target_ops
+class core_target final : public process_stratum_target
{
public:
core_target ();
@@ -132,8 +133,6 @@ private: /* per-core data */
core_target::core_target ()
{
- to_stratum = process_stratum;
-
m_core_gdbarch = gdbarch_from_bfd (core_bfd);
/* Find a suitable core file handler to munch on core_bfd */
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 44aa2f66fb..8cdfa05146 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -439,11 +439,6 @@ inf_child_target::can_use_agent ()
return agent_loaded_p ();
}
-inf_child_target::inf_child_target ()
-{
- this->to_stratum = process_stratum;
-}
-
/* See inf-child.h. */
void
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index 98969bc5fa..d301d398eb 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -21,15 +21,16 @@
#define INF_CHILD_H
#include "target.h"
+#include "process-stratum-target.h"
/* A prototype child target. The client can override it with local
methods. */
class inf_child_target
- : public memory_breakpoint_target<target_ops>
+ : public memory_breakpoint_target<process_stratum_target>
{
public:
- inf_child_target ();
+ inf_child_target () = default;
~inf_child_target () override = 0;
const target_info &info () const override;
@@ -69,12 +70,6 @@ public:
void post_attach (int) override;
- /* We must default these because they must be implemented by any
- target that can run. */
- bool can_async_p () override { return false; }
- bool supports_non_stop () override { return false; }
- bool supports_disable_randomization () override { return false; }
-
char *pid_to_exec_file (int pid) override;
bool has_all_memory () override;
diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
new file mode 100644
index 0000000000..9ce8d3dd47
--- /dev/null
+++ b/gdb/process-stratum-target.c
@@ -0,0 +1,49 @@
+/* Abstract base class inherited by all process_stratum targets
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "process-stratum-target.h"
+#include "inferior.h"
+
+process_stratum_target::~process_stratum_target ()
+{
+}
+
+struct address_space *
+process_stratum_target::thread_address_space (ptid_t ptid)
+{
+ /* Fall-back to the "main" address space of the inferior. */
+ inferior *inf = find_inferior_ptid (ptid);
+
+ if (inf == NULL || inf->aspace == NULL)
+ internal_error (__FILE__, __LINE__,
+ _("Can't determine the current "
+ "address space of thread %s\n"),
+ target_pid_to_str (ptid));
+
+ return inf->aspace;
+}
+
+struct gdbarch *
+process_stratum_target::thread_architecture (ptid_t ptid)
+{
+ inferior *inf = find_inferior_ptid (ptid);
+ gdb_assert (inf != NULL);
+ return inf->gdbarch;
+}
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
new file mode 100644
index 0000000000..68f9684378
--- /dev/null
+++ b/gdb/process-stratum-target.h
@@ -0,0 +1,52 @@
+/* Abstract base class inherited by all process_stratum targets
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef PROCESS_STRATUM_TARGET_H
+#define PROCESS_STRATUM_TARGET_H
+
+#include "target.h"
+
+/* Abstract base class inherited by all process_stratum targets. */
+
+class process_stratum_target : public target_ops
+{
+public:
+ process_stratum_target ()
+ : target_ops {}
+ {
+ to_stratum = process_stratum;
+ }
+
+ ~process_stratum_target () override = 0;
+
+ /* We must default these because they must be implemented by any
+ target that can run. */
+ bool can_async_p () override { return false; }
+ bool supports_non_stop () override { return false; }
+ bool supports_disable_randomization () override { return false; }
+
+ /* This default implementation returns the inferior's address
+ space. */
+ struct address_space *thread_address_space (ptid_t ptid) override;
+
+ /* This default implementation always returns target_gdbarch (). */
+ struct gdbarch *thread_architecture (ptid_t ptid) override;
+};
+
+#endif /* !defined (PROCESS_STRATUM_TARGET_H) */
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 63e41458d7..1ceecaae2c 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -31,6 +31,7 @@
#include <setjmp.h>
#include "terminal.h"
#include "target.h"
+#include "process-stratum-target.h"
#include "gdbcore.h"
#include "gdb/callback.h"
#include "gdb/remote-sim.h"
@@ -82,10 +83,9 @@ static const target_info gdbsim_target_info = {
};
struct gdbsim_target final
- : public memory_breakpoint_target<target_ops>
+ : public memory_breakpoint_target<process_stratum_target>
{
- gdbsim_target ()
- { to_stratum = process_stratum; }
+ gdbsim_target () = default;
const target_info &info () const override
{ return gdbsim_target_info; }
diff --git a/gdb/remote.c b/gdb/remote.c
index 90b5dabc8a..ae35bec451 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -27,6 +27,7 @@
#include "bfd.h"
#include "symfile.h"
#include "target.h"
+#include "process-stratum-target.h"
/*#include "terminal.h" */
#include "gdbcmd.h"
#include "objfiles.h"
@@ -404,13 +405,10 @@ static const target_info remote_target_info = {
remote_doc
};
-class remote_target : public target_ops
+class remote_target : public process_stratum_target
{
public:
- remote_target ()
- {
- to_stratum = process_stratum;
- }
+ remote_target () = default;
~remote_target () override;
const target_info &info () const override
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index eeb5057ce7..1e70823fce 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -2808,7 +2808,7 @@ target_ops::thread_architecture (ptid_t arg0)
struct gdbarch *
dummy_target::thread_architecture (ptid_t arg0)
{
- return default_thread_architecture (this, arg0);
+ return NULL;
}
struct gdbarch *
@@ -2834,7 +2834,7 @@ target_ops::thread_address_space (ptid_t arg0)
struct address_space *
dummy_target::thread_address_space (ptid_t arg0)
{
- return default_thread_address_space (this, arg0);
+ return NULL;
}
struct address_space *
diff --git a/gdb/target.c b/gdb/target.c
index 7fad3a9602..8905ce3630 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -82,16 +82,10 @@ static int default_verify_memory (struct target_ops *self,
const gdb_byte *data,
CORE_ADDR memaddr, ULONGEST size);
-static struct address_space *default_thread_address_space
- (struct target_ops *self, ptid_t ptid);
-
static void tcomplain (void) ATTRIBUTE_NORETURN;
static struct target_ops *find_default_run_target (const char *);
-static struct gdbarch *default_thread_architecture (struct target_ops *ops,
- ptid_t ptid);
-
static int dummy_find_memory_regions (struct target_ops *self,
find_memory_region_ftype ignore1,
void *ignore2);
@@ -2567,22 +2561,6 @@ target_get_osdata (const char *type)
return target_read_stralloc (t, TARGET_OBJECT_OSDATA, type);
}
-static struct address_space *
-default_thread_address_space (struct target_ops *self, ptid_t ptid)
-{
- struct inferior *inf;
-
- /* Fall-back to the "main" address space of the inferior. */
- inf = find_inferior_ptid (ptid);
-
- if (inf == NULL || inf->aspace == NULL)
- internal_error (__FILE__, __LINE__,
- _("Can't determine the current "
- "address space of thread %s\n"),
- target_pid_to_str (ptid));
-
- return inf->aspace;
-}
/* Determine the current address space of thread PTID. */
@@ -3180,14 +3158,6 @@ default_watchpoint_addr_within_range (struct target_ops *target,
return addr >= start && addr < start + length;
}
-static struct gdbarch *
-default_thread_architecture (struct target_ops *ops, ptid_t ptid)
-{
- inferior *inf = find_inferior_ptid (ptid);
- gdb_assert (inf != NULL);
- return inf->gdbarch;
-}
-
/* See target.h. */
target_ops *
diff --git a/gdb/target.h b/gdb/target.h
index e170bbce3c..36f8d5e5a2 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -882,18 +882,13 @@ struct target_ops
to_thread_architecture would return SPU, otherwise PPC32 or PPC64).
This is architecture used to perform decr_pc_after_break adjustment,
and also determines the frame architecture of the innermost frame.
- ptrace operations need to operate according to target_gdbarch ().
-
- The default implementation always returns target_gdbarch (). */
+ ptrace operations need to operate according to target_gdbarch (). */
virtual struct gdbarch *thread_architecture (ptid_t)
- TARGET_DEFAULT_FUNC (default_thread_architecture);
-
- /* Determine current address space of thread PTID.
+ TARGET_DEFAULT_RETURN (NULL);
- The default implementation always returns the inferior's
- address space. */
+ /* Determine current address space of thread PTID. */
virtual struct address_space *thread_address_space (ptid_t)
- TARGET_DEFAULT_FUNC (default_thread_address_space);
+ TARGET_DEFAULT_RETURN (NULL);
/* Target file operations. */
diff --git a/gdb/test-target.h b/gdb/test-target.h
index b3170f8265..1286fc1a6e 100644
--- a/gdb/test-target.h
+++ b/gdb/test-target.h
@@ -20,7 +20,7 @@
#ifndef TEST_TARGET_H
#define TEST_TARGET_H
-#include "target.h"
+#include "process-stratum-target.h"
#if GDB_SELF_TEST
namespace selftests {
@@ -28,14 +28,10 @@ namespace selftests {
/* A mock process_stratum target_ops that doesn't read/write registers
anywhere. */
-class test_target_ops : public target_ops
+class test_target_ops : public process_stratum_target
{
public:
- test_target_ops ()
- : target_ops {}
- {
- to_stratum = process_stratum;
- }
+ test_target_ops () = default;
const target_info &info () const override;
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index b367f6e403..88e79f7958 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -470,11 +470,6 @@ tracefile_target::get_trace_status (struct trace_status *ts)
return -1;
}
-tracefile_target::tracefile_target ()
-{
- this->to_stratum = process_stratum;
-}
-
void
_initialize_tracefile (void)
{
diff --git a/gdb/tracefile.h b/gdb/tracefile.h
index 47f8bee8f2..3ae3e7d0e5 100644
--- a/gdb/tracefile.h
+++ b/gdb/tracefile.h
@@ -3,6 +3,7 @@
#include "tracepoint.h"
#include "target.h"
+#include "process-stratum-target.h"
struct trace_file_writer;
@@ -116,10 +117,10 @@ extern struct trace_file_writer *tfile_trace_file_writer_new (void);
/* Base class for tracefile related targets. */
-class tracefile_target : public target_ops
+class tracefile_target : public process_stratum_target
{
public:
- tracefile_target ();
+ tracefile_target () = default;
int get_trace_status (trace_status *ts) override;
bool has_all_memory () override;
--
2.14.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods
2018-11-27 20:22 [PATCH 0/3] Introduce process_stratum_target Pedro Alves
2018-11-27 20:22 ` [PATCH 1/3] Move test_target_ops to a separate file Pedro Alves
2018-11-27 20:23 ` [PATCH 2/3] Introduce process_stratum_target Pedro Alves
@ 2018-11-27 20:23 ` Pedro Alves
2018-11-29 18:31 ` Tom Tromey
2018-11-29 18:31 ` [PATCH 0/3] Introduce process_stratum_target Tom Tromey
3 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2018-11-27 20:23 UTC (permalink / raw)
To: gdb-patches
This patch converts the default_child_has_foo functions to
process_stratum_target methods. This simplifies "regular"
non-inf_child process_stratum targets, since they no longer have to
override the target_ops::has_foo methods to call the default_child_foo
functions. A couple targets need to override the new defaults
(corelow and tracefiles), but it still seems like a good tradeoff,
since those are expected to be little different (target doesn't run).
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* corelow.c (core_target) <has_all_memory, has_execution>: New
overrides.
* inf-child.c (inf_child_target::has_all_memory)
(inf_child_target::has_memory, inf_child_target::has_stack)
(inf_child_target::has_registers)
(inf_child_target::has_execution): Delete.
* inf-child.h (inf_child_target) <has_all_memory, has_memory,
has_stack, has_registers, has_execution>: Delete.
* process-stratum-target.c
(process_stratum_target::has_all_memory)
(process_stratum_target::has_memory)
(process_stratum_target::has_stack)
(process_stratum_target::has_registers)
(process_stratum_target::has_execution): New.
* process-stratum-target.h (process_stratum_target)
<has_all_memory, has_memory, has_stack, has_registers,
has_execution>: New method overrides.
* ravenscar-thread.c (ravenscar_thread_target) <has_all_memory,
has_memory, has_stack, has_registers, has_execution>: Delete.
* remote-sim.c (gdbsim_target) <has_stack, has_registers,
has_execution>: Delete.
* remote.c (remote_target) <has_all_memory, has_memory, has_stack,
has_registers, has_execution>: Delete.
* target.c (default_child_has_all_memory)
(default_child_has_memory, default_child_has_stack)
(default_child_has_registers, default_child_has_execution):
Delete.
* target.h (default_child_has_all_memory)
(default_child_has_memory, default_child_has_stack)
(default_child_has_registers, default_child_has_execution):
Delete.
* tracefile.h (tracefile_target) <has_execution>: New override.
---
gdb/corelow.c | 3 +++
gdb/inf-child.c | 30 ------------------------
gdb/inf-child.h | 6 -----
gdb/process-stratum-target.c | 51 +++++++++++++++++++++++++++++++++++++++++
gdb/process-stratum-target.h | 8 +++++++
gdb/ravenscar-thread.c | 7 ------
gdb/remote-sim.c | 9 --------
gdb/remote.c | 6 -----
gdb/target.c | 54 --------------------------------------------
gdb/target.h | 9 --------
gdb/tracefile.h | 1 +
11 files changed, 63 insertions(+), 121 deletions(-)
diff --git a/gdb/corelow.c b/gdb/corelow.c
index deabf84def..7cc177c9d6 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -90,9 +90,12 @@ public:
const char *thread_name (struct thread_info *) override;
+ bool has_all_memory () override { return false; }
bool has_memory () override;
bool has_stack () override;
bool has_registers () override;
+ bool has_execution (ptid_t) override { return false; }
+
bool info_proc (const char *, enum info_proc_what) override;
/* A few helpers. */
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 8cdfa05146..fc704455b3 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -243,36 +243,6 @@ inf_child_target::pid_to_exec_file (int pid)
return NULL;
}
-bool
-inf_child_target::has_all_memory ()
-{
- return default_child_has_all_memory ();
-}
-
-bool
-inf_child_target::has_memory ()
-{
- return default_child_has_memory ();
-}
-
-bool
-inf_child_target::has_stack ()
-{
- return default_child_has_stack ();
-}
-
-bool
-inf_child_target::has_registers ()
-{
- return default_child_has_registers ();
-}
-
-bool
-inf_child_target::has_execution (ptid_t ptid)
-{
- return default_child_has_execution (ptid);
-}
-
/* Implementation of to_fileio_open. */
int
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index d301d398eb..a0bb40b958 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -72,12 +72,6 @@ public:
char *pid_to_exec_file (int pid) override;
- bool has_all_memory () override;
- bool has_memory () override;
- bool has_stack () override;
- bool has_registers () override;
- bool has_execution (ptid_t) override;
-
int fileio_open (struct inferior *inf, const char *filename,
int flags, int mode, int warn_if_slow,
int *target_errno) override;
diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index 9ce8d3dd47..a9d02f2e4f 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -47,3 +47,54 @@ process_stratum_target::thread_architecture (ptid_t ptid)
gdb_assert (inf != NULL);
return inf->gdbarch;
}
+
+bool
+process_stratum_target::has_all_memory ()
+{
+ /* If no inferior selected, then we can't read memory here. */
+ if (inferior_ptid == null_ptid)
+ return false;
+
+ return true;
+}
+
+bool
+process_stratum_target::has_memory ()
+{
+ /* If no inferior selected, then we can't read memory here. */
+ if (inferior_ptid == null_ptid)
+ return false;
+
+ return true;
+}
+
+bool
+process_stratum_target::has_stack ()
+{
+ /* If no inferior selected, there's no stack. */
+ if (inferior_ptid == null_ptid)
+ return false;
+
+ return true;
+}
+
+bool
+process_stratum_target::has_registers ()
+{
+ /* Can't read registers from no inferior. */
+ if (inferior_ptid == null_ptid)
+ return false;
+
+ return true;
+}
+
+bool
+process_stratum_target::has_execution (ptid_t the_ptid)
+{
+ /* If there's no thread selected, then we can't make it run through
+ hoops. */
+ if (the_ptid == null_ptid)
+ return 0;
+
+ return 1;
+}
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index 68f9684378..3a40a563fb 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -47,6 +47,14 @@ public:
/* This default implementation always returns target_gdbarch (). */
struct gdbarch *thread_architecture (ptid_t ptid) override;
+
+ /* Default implementations for process_stratum targets. Return true
+ if there's a selected inferior, false otherwise. */
+ bool has_all_memory () override;
+ bool has_memory () override;
+ bool has_stack () override;
+ bool has_registers () override;
+ bool has_execution (ptid_t the_ptid) override;
};
#endif /* !defined (PROCESS_STRATUM_TARGET_H) */
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index e60fad8746..0f2889cf0e 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -116,13 +116,6 @@ struct ravenscar_thread_target final : public target_ops
ptid_t get_ada_task_ptid (long lwp, long thread) override;
void mourn_inferior () override;
-
- bool has_all_memory () override { return default_child_has_all_memory (); }
- bool has_memory () override { return default_child_has_memory (); }
- bool has_stack () override { return default_child_has_stack (); }
- bool has_registers () override { return default_child_has_registers (); }
- bool has_execution (ptid_t ptid) override
- { return default_child_has_execution (ptid); }
};
/* This module's target-specific operations. */
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 1ceecaae2c..b3fb95a77f 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -128,15 +128,6 @@ struct gdbsim_target final
bool has_all_memory () override;
bool has_memory () override;
-
- bool has_stack () override
- { return default_child_has_stack (); }
-
- bool has_registers () override
- { return default_child_has_registers (); }
-
- bool has_execution (ptid_t ptid) override
- { return default_child_has_execution (ptid); }
};
static struct gdbsim_target gdbsim_ops;
diff --git a/gdb/remote.c b/gdb/remote.c
index ae35bec451..4324452aba 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -525,12 +525,6 @@ public:
CORE_ADDR load_module_addr,
CORE_ADDR offset) override;
- bool has_all_memory () override { return default_child_has_all_memory (); }
- bool has_memory () override { return default_child_has_memory (); }
- bool has_stack () override { return default_child_has_stack (); }
- bool has_registers () override { return default_child_has_registers (); }
- bool has_execution (ptid_t ptid) override { return default_child_has_execution (ptid); }
-
bool can_execute_reverse () override;
std::vector<mem_region> memory_map () override;
diff --git a/gdb/target.c b/gdb/target.c
index 8905ce3630..ecfdde93a1 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -186,60 +186,6 @@ target_command (const char *arg, int from_tty)
gdb_stdout);
}
-/* Default target_has_* methods for process_stratum targets. */
-
-int
-default_child_has_all_memory ()
-{
- /* If no inferior selected, then we can't read memory here. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_memory ()
-{
- /* If no inferior selected, then we can't read memory here. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_stack ()
-{
- /* If no inferior selected, there's no stack. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_registers ()
-{
- /* Can't read registers from no inferior. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_execution (ptid_t the_ptid)
-{
- /* If there's no thread selected, then we can't make it run through
- hoops. */
- if (the_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-
int
target_has_all_memory_1 (void)
{
diff --git a/gdb/target.h b/gdb/target.h
index 36f8d5e5a2..b0469bba14 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1790,15 +1790,6 @@ extern int target_has_execution_current (void);
#define target_has_execution target_has_execution_current ()
-/* Default implementations for process_stratum targets. Return true
- if there's a selected inferior, false otherwise. */
-
-extern int default_child_has_all_memory ();
-extern int default_child_has_memory ();
-extern int default_child_has_stack ();
-extern int default_child_has_registers ();
-extern int default_child_has_execution (ptid_t the_ptid);
-
/* Can the target support the debugger control of thread execution?
Can it lock the thread scheduler? */
diff --git a/gdb/tracefile.h b/gdb/tracefile.h
index 3ae3e7d0e5..8f9dc0e06d 100644
--- a/gdb/tracefile.h
+++ b/gdb/tracefile.h
@@ -127,6 +127,7 @@ public:
bool has_memory () override;
bool has_stack () override;
bool has_registers () override;
+ bool has_execution (ptid_t) override { return false; }
bool thread_alive (ptid_t ptid) override;
};
--
2.14.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] Introduce process_stratum_target
2018-11-27 20:23 ` [PATCH 2/3] Introduce process_stratum_target Pedro Alves
@ 2018-11-29 18:26 ` Tom Tromey
2018-11-29 23:16 ` Tom Tromey
2018-11-30 14:22 ` Pedro Alves
0 siblings, 2 replies; 12+ messages in thread
From: Tom Tromey @ 2018-11-29 18:26 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> This adds a base class that all process_stratum targets inherit from.
Pedro> default_thread_address_space/default_thread_architecture only make
Pedro> sense for process_stratum targets, so they are transformed to
Pedro> process_stratum_target methods/overrides.
Pedro> + process_stratum_target ()
Pedro> + : target_ops {}
This struck me as slightly weird -- though not weird enough to block
anything.
What if I added a protected constructor to target_ops that took the
stratum as an argument? Would there be a problem with that?
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] Introduce process_stratum_target
2018-11-27 20:22 [PATCH 0/3] Introduce process_stratum_target Pedro Alves
` (2 preceding siblings ...)
2018-11-27 20:23 ` [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods Pedro Alves
@ 2018-11-29 18:31 ` Tom Tromey
3 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-11-29 18:31 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Some more easily-splittable bits from my multi-target branch.
Pedro> In the branch, I did a lot of back and forth around changing the
Pedro> interface of the target_ops::has_foo methods, and it annoyed me that I
Pedro> had to adjust the interface in several places. At some point I came
Pedro> up with these patches to try to centralize things a little more.
I sent a couple of nits but otherwise this all seems great to me.
Thanks.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods
2018-11-27 20:23 ` [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods Pedro Alves
@ 2018-11-29 18:31 ` Tom Tromey
2018-11-30 16:31 ` Pedro Alves
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2018-11-29 18:31 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> +bool
Pedro> +process_stratum_target::has_all_memory ()
Pedro> +{
Pedro> + /* If no inferior selected, then we can't read memory here. */
Pedro> + if (inferior_ptid == null_ptid)
Pedro> + return false;
Pedro> +
Pedro> + return true;
I don't really care too much but I was wondering why not just
return inferior_ptid != null_ptid;
Pedro> +bool
Pedro> +process_stratum_target::has_execution (ptid_t the_ptid)
Pedro> +{
Pedro> + /* If there's no thread selected, then we can't make it run through
Pedro> + hoops. */
Pedro> + if (the_ptid == null_ptid)
Pedro> + return 0;
Pedro> +
Pedro> + return 1;
This one should use true/false.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] Introduce process_stratum_target
2018-11-29 18:26 ` Tom Tromey
@ 2018-11-29 23:16 ` Tom Tromey
2018-11-30 14:22 ` Pedro Alves
1 sibling, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2018-11-29 23:16 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
Tom> What if I added a protected constructor to target_ops that took the
Tom> stratum as an argument? Would there be a problem with that?
Like this. If it's reasonable I'll rebase it on top of yours after it
goes in.
Tom
commit 207fb6d5368f6406c536e8dee356ee1564e4d494
Author: Tom Tromey <tom@tromey.com>
Date: Thu Nov 29 16:04:37 2018 -0700
Add target_ops constructor
This adds a protected target_ops constructor that sets the to_stratum
member, and updates subclasses to invoke it.
gdb/ChangeLog
2018-11-29 Tom Tromey <tom@tromey.com>
* spu-multiarch.c (spu_multiarch_target::spu_multiarch_target):
Update.
* ravenscar-thread.c
(ravenscar_thread_target::ravenscar_thread_target): Update.
* bsd-uthread.c (bsd_uthread_target::bsd_uthread_target): Update.
* sol-thread.c (sol_thread_target::sol_thread_target): Update.
* aix-thread.c (aix_thread_target::aix_thread_target): Update.
* bsd-kvm.c (bsd_kvm_target::bsd_kvm_target): Update.
* tracefile.c (tracefile_target::tracefile_target): Update.
* target.h (struct target_ops) <target_ops>: New constructor.
(struct memory_breakpoint_target) <memory_breakpoint_target>:
Likewise.
(test_target_ops::test_target_ops): Update.
* target.c (dummy_target::dummy_target): Update.
(debug_target::debug_target): Update.
* remote.c (remote_target::remote_target): Update.
* remote-sim.c (gdbsim_target::gdbsim_target): Update.
* record-full.c
(record_full_base_target::record_full_base_target): Update.
* record-btrace.c (record_btrace_target::record_btrace_target):
Update.
* linux-thread-db.c (thread_db_target::thread_db_target): Update.
* inf-child.c (inf_child_target::inf_child_target): Update.
* exec.c (exec_target::exec_target): Update.
* corelow.c (core_target::core_target): Update.
* bfd-target.c (target_bfd::target_bfd): Update.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 70d4c06742..d1cd119484 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,32 @@
+2018-11-29 Tom Tromey <tom@tromey.com>
+
+ * spu-multiarch.c (spu_multiarch_target::spu_multiarch_target):
+ Update.
+ * ravenscar-thread.c
+ (ravenscar_thread_target::ravenscar_thread_target): Update.
+ * bsd-uthread.c (bsd_uthread_target::bsd_uthread_target): Update.
+ * sol-thread.c (sol_thread_target::sol_thread_target): Update.
+ * aix-thread.c (aix_thread_target::aix_thread_target): Update.
+ * bsd-kvm.c (bsd_kvm_target::bsd_kvm_target): Update.
+ * tracefile.c (tracefile_target::tracefile_target): Update.
+ * target.h (struct target_ops) <target_ops>: New constructor.
+ (struct memory_breakpoint_target) <memory_breakpoint_target>:
+ Likewise.
+ (test_target_ops::test_target_ops): Update.
+ * target.c (dummy_target::dummy_target): Update.
+ (debug_target::debug_target): Update.
+ * remote.c (remote_target::remote_target): Update.
+ * remote-sim.c (gdbsim_target::gdbsim_target): Update.
+ * record-full.c
+ (record_full_base_target::record_full_base_target): Update.
+ * record-btrace.c (record_btrace_target::record_btrace_target):
+ Update.
+ * linux-thread-db.c (thread_db_target::thread_db_target): Update.
+ * inf-child.c (inf_child_target::inf_child_target): Update.
+ * exec.c (exec_target::exec_target): Update.
+ * corelow.c (core_target::core_target): Update.
+ * bfd-target.c (target_bfd::target_bfd): Update.
+
2018-11-29 Tom Tromey <tom@tromey.com>
* valarith.c (value_x_unop): Don't set argvec[3].
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 97592e5b1f..a5460dc61f 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -118,7 +118,8 @@ class aix_thread_target final : public target_ops
{
public:
aix_thread_target ()
- { to_stratum = thread_stratum; }
+ : target_ops (thread_stratum)
+ { }
const target_info &info () const override
{ return aix_thread_target_info; }
diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
index 6138d450d9..837f0f32f3 100644
--- a/gdb/bfd-target.c
+++ b/gdb/bfd-target.c
@@ -90,9 +90,9 @@ target_bfd::get_section_table ()
}
target_bfd::target_bfd (struct bfd *abfd)
- : m_bfd (gdb_bfd_ref_ptr::new_reference (abfd))
+ : target_ops (file_stratum),
+ 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);
diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index af8305f386..342645e3ad 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -75,7 +75,8 @@ class bsd_kvm_target final : public target_ops
{
public:
bsd_kvm_target ()
- { this->to_stratum = process_stratum; }
+ : target_ops (process_stratum)
+ { }
const target_info &info () const override
{ return bsd_kvm_target_info; }
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 3229a18055..7e456ad101 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -42,7 +42,8 @@ static const target_info bsd_uthread_target_info = {
struct bsd_uthread_target final : public target_ops
{
bsd_uthread_target ()
- { to_stratum = thread_stratum; }
+ : target_ops (thread_stratum)
+ { }
const target_info &info () const override
{ return bsd_uthread_target_info; }
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 72f2807640..69e9d6e922 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -131,9 +131,8 @@ private: /* per-core data */
};
core_target::core_target ()
+ : target_ops (process_stratum)
{
- to_stratum = process_stratum;
-
m_core_gdbarch = gdbarch_from_bfd (core_bfd);
/* Find a suitable core file handler to munch on core_bfd */
diff --git a/gdb/exec.c b/gdb/exec.c
index 615fb2b5db..f95a0fbda4 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -61,7 +61,8 @@ Specify the filename of the executable file.")
struct exec_target final : public target_ops
{
exec_target ()
- { to_stratum = file_stratum; }
+ : target_ops (file_stratum)
+ { }
const target_info &info () const override
{ return exec_target_info; }
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 44aa2f66fb..ab02f97f68 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -441,7 +441,6 @@ inf_child_target::can_use_agent ()
inf_child_target::inf_child_target ()
{
- this->to_stratum = process_stratum;
}
/* See inf-child.h. */
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 74acec2629..433cb5800c 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -108,8 +108,8 @@ public:
};
thread_db_target::thread_db_target ()
+ : target_ops (thread_stratum)
{
- this->to_stratum = thread_stratum;
}
static char *libthread_db_search_path;
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index e60fad8746..6bbb4d13a2 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -82,7 +82,8 @@ static const target_info ravenscar_target_info = {
struct ravenscar_thread_target final : public target_ops
{
ravenscar_thread_target ()
- { to_stratum = thread_stratum; }
+ : target_ops (thread_stratum)
+ { }
const target_info &info () const override
{ return ravenscar_target_info; }
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 814f080941..d33de7ee15 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -54,7 +54,8 @@ class record_btrace_target final : public target_ops
{
public:
record_btrace_target ()
- { to_stratum = record_stratum; }
+ : target_ops (record_stratum)
+ { }
const target_info &info () const override
{ return record_btrace_target_info; }
diff --git a/gdb/record-full.c b/gdb/record-full.c
index dbaa8c3d40..ce799308a0 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -219,7 +219,8 @@ class record_full_base_target : public target_ops
{
public:
record_full_base_target ()
- { to_stratum = record_stratum; }
+ : target_ops (record_stratum)
+ { }
const target_info &info () const override = 0;
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 63e41458d7..186503f34f 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -85,7 +85,7 @@ struct gdbsim_target final
: public memory_breakpoint_target<target_ops>
{
gdbsim_target ()
- { to_stratum = process_stratum; }
+ { }
const target_info &info () const override
{ return gdbsim_target_info; }
diff --git a/gdb/remote.c b/gdb/remote.c
index 90b5dabc8a..d389468919 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -408,8 +408,8 @@ class remote_target : public target_ops
{
public:
remote_target ()
+ : target_ops (process_stratum)
{
- to_stratum = process_stratum;
}
~remote_target () override;
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index c6a5aca501..212a8fcbca 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -79,7 +79,8 @@ class sol_thread_target final : public target_ops
{
public:
sol_thread_target ()
- { this->to_stratum = thread_stratum; }
+ : target_ops (thread_stratum)
+ { }
const target_info &info () const override
{ return thread_db_target_info; }
diff --git a/gdb/spu-multiarch.c b/gdb/spu-multiarch.c
index 7e642d663a..30c4586c5b 100644
--- a/gdb/spu-multiarch.c
+++ b/gdb/spu-multiarch.c
@@ -45,7 +45,8 @@ static const target_info spu_multiarch_target_info = {
struct spu_multiarch_target final : public target_ops
{
spu_multiarch_target ()
- { to_stratum = arch_stratum; };
+ : spu_multiarch_target (arch_stratum)
+ { }
const target_info &info () const override
{ return spu_multiarch_target_info; }
diff --git a/gdb/target.c b/gdb/target.c
index 29ce5eb414..3d82e7ce8b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3330,13 +3330,13 @@ static const target_info dummy_target_info = {
};
dummy_target::dummy_target ()
+ : target_ops (dummy_stratum)
{
- to_stratum = dummy_stratum;
}
debug_target::debug_target ()
+ : target_ops (debug_stratum)
{
- to_stratum = debug_stratum;
}
const target_info &
diff --git a/gdb/target.h b/gdb/target.h
index 4731e3bf79..a0a5da25b6 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -431,6 +431,15 @@ struct target_info
struct target_ops
{
+ protected:
+
+ explicit target_ops (enum strata stratum)
+ : to_stratum (stratum)
+ {
+ }
+
+ public:
+
/* To the target under this one. */
target_ops *beneath () const;
@@ -2419,6 +2428,15 @@ extern int memory_insert_breakpoint (struct target_ops *,
template <typename BaseTarget>
struct memory_breakpoint_target : public BaseTarget
{
+protected:
+
+ memory_breakpoint_target ()
+ : BaseTarget (process_stratum)
+ {
+ }
+
+public:
+
int insert_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt) override
{ return memory_insert_breakpoint (this, gdbarch, bp_tgt); }
@@ -2584,9 +2602,8 @@ class test_target_ops : public target_ops
{
public:
test_target_ops ()
- : target_ops {}
+ : target_ops (process_stratum)
{
- to_stratum = process_stratum;
}
const target_info &info () const override;
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index b367f6e403..5db8426e33 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -471,8 +471,8 @@ tracefile_target::get_trace_status (struct trace_status *ts)
}
tracefile_target::tracefile_target ()
+ : target_ops (process_stratum)
{
- this->to_stratum = process_stratum;
}
void
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] Introduce process_stratum_target
2018-11-29 18:26 ` Tom Tromey
2018-11-29 23:16 ` Tom Tromey
@ 2018-11-30 14:22 ` Pedro Alves
2018-11-30 15:40 ` Tom Tromey
1 sibling, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2018-11-30 14:22 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 11/29/2018 06:26 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> This adds a base class that all process_stratum targets inherit from.
> Pedro> default_thread_address_space/default_thread_architecture only make
> Pedro> sense for process_stratum targets, so they are transformed to
> Pedro> process_stratum_target methods/overrides.
>
> Pedro> + process_stratum_target ()
> Pedro> + : target_ops {}
>
> This struck me as slightly weird -- though not weird enough to block
> anything.
Yeah. Hmm. I think I added that because at some point in the branch
because I had added some data field to target_ops and forgot to
initialize it. I don't recall exactly. Would be best to just properly
initialize the field in the first place, of course. I just forgot
this explicit initialization here, it seems.
> What if I added a protected constructor to target_ops that took the
> stratum as an argument? Would there be a problem with that?
I don't see a problem with that, but, given that a target's stratum is
a property of the type, and not of an instance of the type, wouldn't it
be better to get rid of to_stratum and replace it with a virtual method?
I.e., when we have 10 target remote instances active, there's no need
for each of the instances to have their own to_stratum copy.
Like this, on top of the series.
From d06419cf3a59983d410ed903e7d5ab36541d1de8 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
---
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 | 8 ++------
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(+), 74 deletions(-)
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 97592e5b1f..d61fd4aac3 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -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;
diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
index 6138d450d9..d6804740d3 100644
--- a/gdb/bfd-target.c
+++ b/gdb/bfd-target.c
@@ -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);
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 3229a18055..3fcb7547a6 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -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;
diff --git a/gdb/exec.c b/gdb/exec.c
index 615fb2b5db..44b212a4d4 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -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,
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index af97c7de2f..9859e4fddd 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -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
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 74acec2629..3c0998e02f 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -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
diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
index 28b49a4e8f..bc16c886cd 100755
--- a/gdb/make-target-delegates
+++ b/gdb/make-target-delegates
@@ -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};
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index 3a40a563fb..ce731ebf83 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -27,14 +27,10 @@
class process_stratum_target : public target_ops
{
public:
- process_stratum_target ()
- : target_ops {}
- {
- 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; }
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 0f2889cf0e..856f40bd16 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -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;
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 814f080941..1ca0176ec8 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -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;
diff --git a/gdb/record-full.c b/gdb/record-full.c
index dbaa8c3d40..66234dbc43 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -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;
diff --git a/gdb/record.c b/gdb/record.c
index fdc76f80cc..05a9960f77 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -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 ());
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 69e42a2722..50a905695c 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -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. */
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index c6a5aca501..a35137fab9 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -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;
diff --git a/gdb/spu-multiarch.c b/gdb/spu-multiarch.c
index 7e642d663a..95c56f08db 100644
--- a/gdb/spu-multiarch.c
+++ b/gdb/spu-multiarch.c
@@ -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;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 1e70823fce..2ff800cca3 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -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;
diff --git a/gdb/target.c b/gdb/target.c
index ecfdde93a1..80b8453176 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -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 ());
}
diff --git a/gdb/target.h b/gdb/target.h
index b0469bba14..abe0ee4a93 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -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]; }
--
2.14.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] Introduce process_stratum_target
2018-11-30 14:22 ` Pedro Alves
@ 2018-11-30 15:40 ` Tom Tromey
2018-11-30 17:52 ` Pedro Alves
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2018-11-30 15:40 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "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.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods
2018-11-29 18:31 ` Tom Tromey
@ 2018-11-30 16:31 ` Pedro Alves
0 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2018-11-30 16:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 11/29/2018 06:31 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> +bool
> Pedro> +process_stratum_target::has_all_memory ()
> Pedro> +{
> Pedro> + /* If no inferior selected, then we can't read memory here. */
> Pedro> + if (inferior_ptid == null_ptid)
> Pedro> + return false;
> Pedro> +
> Pedro> + return true;
>
> I don't really care too much but I was wondering why not just
>
> return inferior_ptid != null_ptid;
No good reason, I guess. No bad reason either, I guess.
It was already like that in default_child_has_all_memory etc.
I've changed it.
>
> Pedro> +bool
> Pedro> +process_stratum_target::has_execution (ptid_t the_ptid)
> Pedro> +{
> Pedro> + /* If there's no thread selected, then we can't make it run through
> Pedro> + hoops. */
> Pedro> + if (the_ptid == null_ptid)
> Pedro> + return 0;
> Pedro> +
> Pedro> + return 1;
>
> This one should use true/false.
Thanks. Fixed to use the != style.
Here's what I merged.
From f3d11a9a96465432c01678445fc2fe84f2ef94f7 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 30 Nov 2018 14:53:40 +0000
Subject: [PATCH] Convert default_child_has_foo functions to
process_stratum_target methods
This patch converts the default_child_has_foo functions to
process_stratum_target methods. This simplifies "regular"
non-inf_child process_stratum targets, since they no longer have to
override the target_ops::has_foo methods to call the default_child_foo
functions. A couple targets need to override the new defaults
(corelow and tracefiles), but it still seems like a good tradeoff,
since those are expected to be little different (target doesn't run).
gdb/ChangeLog:
2018-11-30 Pedro Alves <palves@redhat.com>
* corelow.c (core_target) <has_all_memory, has_execution>: New
overrides.
* inf-child.c (inf_child_target::has_all_memory)
(inf_child_target::has_memory, inf_child_target::has_stack)
(inf_child_target::has_registers)
(inf_child_target::has_execution): Delete.
* inf-child.h (inf_child_target) <has_all_memory, has_memory,
has_stack, has_registers, has_execution>: Delete.
* process-stratum-target.c
(process_stratum_target::has_all_memory)
(process_stratum_target::has_memory)
(process_stratum_target::has_stack)
(process_stratum_target::has_registers)
(process_stratum_target::has_execution): New.
* process-stratum-target.h (process_stratum_target)
<has_all_memory, has_memory, has_stack, has_registers,
has_execution>: New method overrides.
* ravenscar-thread.c (ravenscar_thread_target) <has_all_memory,
has_memory, has_stack, has_registers, has_execution>: Delete.
* remote-sim.c (gdbsim_target) <has_stack, has_registers,
has_execution>: Delete.
* remote.c (remote_target) <has_all_memory, has_memory, has_stack,
has_registers, has_execution>: Delete.
* target.c (default_child_has_all_memory)
(default_child_has_memory, default_child_has_stack)
(default_child_has_registers, default_child_has_execution):
Delete.
* target.h (default_child_has_all_memory)
(default_child_has_memory, default_child_has_stack)
(default_child_has_registers, default_child_has_execution):
Delete.
* tracefile.h (tracefile_target) <has_execution>: New override.
---
gdb/ChangeLog | 35 ++++++++++++++++++++++++++++
gdb/corelow.c | 3 +++
gdb/inf-child.c | 30 ------------------------
gdb/inf-child.h | 6 -----
gdb/process-stratum-target.c | 36 +++++++++++++++++++++++++++++
gdb/process-stratum-target.h | 8 +++++++
gdb/ravenscar-thread.c | 7 ------
gdb/remote-sim.c | 9 --------
gdb/remote.c | 6 -----
gdb/target.c | 54 --------------------------------------------
gdb/target.h | 9 --------
gdb/tracefile.h | 1 +
12 files changed, 83 insertions(+), 121 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index faed74bb83..8fdf04a9d2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,38 @@
+2018-11-30 Pedro Alves <palves@redhat.com>
+
+ * corelow.c (core_target) <has_all_memory, has_execution>: New
+ overrides.
+ * inf-child.c (inf_child_target::has_all_memory)
+ (inf_child_target::has_memory, inf_child_target::has_stack)
+ (inf_child_target::has_registers)
+ (inf_child_target::has_execution): Delete.
+ * inf-child.h (inf_child_target) <has_all_memory, has_memory,
+ has_stack, has_registers, has_execution>: Delete.
+ * process-stratum-target.c
+ (process_stratum_target::has_all_memory)
+ (process_stratum_target::has_memory)
+ (process_stratum_target::has_stack)
+ (process_stratum_target::has_registers)
+ (process_stratum_target::has_execution): New.
+ * process-stratum-target.h (process_stratum_target)
+ <has_all_memory, has_memory, has_stack, has_registers,
+ has_execution>: New method overrides.
+ * ravenscar-thread.c (ravenscar_thread_target) <has_all_memory,
+ has_memory, has_stack, has_registers, has_execution>: Delete.
+ * remote-sim.c (gdbsim_target) <has_stack, has_registers,
+ has_execution>: Delete.
+ * remote.c (remote_target) <has_all_memory, has_memory, has_stack,
+ has_registers, has_execution>: Delete.
+ * target.c (default_child_has_all_memory)
+ (default_child_has_memory, default_child_has_stack)
+ (default_child_has_registers, default_child_has_execution):
+ Delete.
+ * target.h (default_child_has_all_memory)
+ (default_child_has_memory, default_child_has_stack)
+ (default_child_has_registers, default_child_has_execution):
+ Delete.
+ * tracefile.h (tracefile_target) <has_execution>: New override.
+
2018-11-30 Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_SFILES): Add process-stratum-target.c.
diff --git a/gdb/corelow.c b/gdb/corelow.c
index deabf84def..7cc177c9d6 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -90,9 +90,12 @@ public:
const char *thread_name (struct thread_info *) override;
+ bool has_all_memory () override { return false; }
bool has_memory () override;
bool has_stack () override;
bool has_registers () override;
+ bool has_execution (ptid_t) override { return false; }
+
bool info_proc (const char *, enum info_proc_what) override;
/* A few helpers. */
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 8cdfa05146..fc704455b3 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -243,36 +243,6 @@ inf_child_target::pid_to_exec_file (int pid)
return NULL;
}
-bool
-inf_child_target::has_all_memory ()
-{
- return default_child_has_all_memory ();
-}
-
-bool
-inf_child_target::has_memory ()
-{
- return default_child_has_memory ();
-}
-
-bool
-inf_child_target::has_stack ()
-{
- return default_child_has_stack ();
-}
-
-bool
-inf_child_target::has_registers ()
-{
- return default_child_has_registers ();
-}
-
-bool
-inf_child_target::has_execution (ptid_t ptid)
-{
- return default_child_has_execution (ptid);
-}
-
/* Implementation of to_fileio_open. */
int
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index d301d398eb..a0bb40b958 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -72,12 +72,6 @@ public:
char *pid_to_exec_file (int pid) override;
- bool has_all_memory () override;
- bool has_memory () override;
- bool has_stack () override;
- bool has_registers () override;
- bool has_execution (ptid_t) override;
-
int fileio_open (struct inferior *inf, const char *filename,
int flags, int mode, int warn_if_slow,
int *target_errno) override;
diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index 9ce8d3dd47..ca9d13a7b3 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -47,3 +47,39 @@ process_stratum_target::thread_architecture (ptid_t ptid)
gdb_assert (inf != NULL);
return inf->gdbarch;
}
+
+bool
+process_stratum_target::has_all_memory ()
+{
+ /* If no inferior selected, then we can't read memory here. */
+ return inferior_ptid != null_ptid;
+}
+
+bool
+process_stratum_target::has_memory ()
+{
+ /* If no inferior selected, then we can't read memory here. */
+ return inferior_ptid != null_ptid;
+}
+
+bool
+process_stratum_target::has_stack ()
+{
+ /* If no inferior selected, there's no stack. */
+ return inferior_ptid != null_ptid;
+}
+
+bool
+process_stratum_target::has_registers ()
+{
+ /* Can't read registers from no inferior. */
+ return inferior_ptid != null_ptid;
+}
+
+bool
+process_stratum_target::has_execution (ptid_t the_ptid)
+{
+ /* If there's no thread selected, then we can't make it run through
+ hoops. */
+ return the_ptid != null_ptid;
+}
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index 0a799f7a28..74640aa1ed 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -46,6 +46,14 @@ public:
/* This default implementation always returns target_gdbarch (). */
struct gdbarch *thread_architecture (ptid_t ptid) override;
+
+ /* Default implementations for process_stratum targets. Return true
+ if there's a selected inferior, false otherwise. */
+ bool has_all_memory () override;
+ bool has_memory () override;
+ bool has_stack () override;
+ bool has_registers () override;
+ bool has_execution (ptid_t the_ptid) override;
};
#endif /* !defined (PROCESS_STRATUM_TARGET_H) */
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index e60fad8746..0f2889cf0e 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -116,13 +116,6 @@ struct ravenscar_thread_target final : public target_ops
ptid_t get_ada_task_ptid (long lwp, long thread) override;
void mourn_inferior () override;
-
- bool has_all_memory () override { return default_child_has_all_memory (); }
- bool has_memory () override { return default_child_has_memory (); }
- bool has_stack () override { return default_child_has_stack (); }
- bool has_registers () override { return default_child_has_registers (); }
- bool has_execution (ptid_t ptid) override
- { return default_child_has_execution (ptid); }
};
/* This module's target-specific operations. */
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 1ceecaae2c..b3fb95a77f 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -128,15 +128,6 @@ struct gdbsim_target final
bool has_all_memory () override;
bool has_memory () override;
-
- bool has_stack () override
- { return default_child_has_stack (); }
-
- bool has_registers () override
- { return default_child_has_registers (); }
-
- bool has_execution (ptid_t ptid) override
- { return default_child_has_execution (ptid); }
};
static struct gdbsim_target gdbsim_ops;
diff --git a/gdb/remote.c b/gdb/remote.c
index ae35bec451..4324452aba 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -525,12 +525,6 @@ public:
CORE_ADDR load_module_addr,
CORE_ADDR offset) override;
- bool has_all_memory () override { return default_child_has_all_memory (); }
- bool has_memory () override { return default_child_has_memory (); }
- bool has_stack () override { return default_child_has_stack (); }
- bool has_registers () override { return default_child_has_registers (); }
- bool has_execution (ptid_t ptid) override { return default_child_has_execution (ptid); }
-
bool can_execute_reverse () override;
std::vector<mem_region> memory_map () override;
diff --git a/gdb/target.c b/gdb/target.c
index 8905ce3630..ecfdde93a1 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -186,60 +186,6 @@ target_command (const char *arg, int from_tty)
gdb_stdout);
}
-/* Default target_has_* methods for process_stratum targets. */
-
-int
-default_child_has_all_memory ()
-{
- /* If no inferior selected, then we can't read memory here. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_memory ()
-{
- /* If no inferior selected, then we can't read memory here. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_stack ()
-{
- /* If no inferior selected, there's no stack. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_registers ()
-{
- /* Can't read registers from no inferior. */
- if (inferior_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-int
-default_child_has_execution (ptid_t the_ptid)
-{
- /* If there's no thread selected, then we can't make it run through
- hoops. */
- if (the_ptid == null_ptid)
- return 0;
-
- return 1;
-}
-
-
int
target_has_all_memory_1 (void)
{
diff --git a/gdb/target.h b/gdb/target.h
index 36f8d5e5a2..b0469bba14 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1790,15 +1790,6 @@ extern int target_has_execution_current (void);
#define target_has_execution target_has_execution_current ()
-/* Default implementations for process_stratum targets. Return true
- if there's a selected inferior, false otherwise. */
-
-extern int default_child_has_all_memory ();
-extern int default_child_has_memory ();
-extern int default_child_has_stack ();
-extern int default_child_has_registers ();
-extern int default_child_has_execution (ptid_t the_ptid);
-
/* Can the target support the debugger control of thread execution?
Can it lock the thread scheduler? */
diff --git a/gdb/tracefile.h b/gdb/tracefile.h
index 3ae3e7d0e5..8f9dc0e06d 100644
--- a/gdb/tracefile.h
+++ b/gdb/tracefile.h
@@ -127,6 +127,7 @@ public:
bool has_memory () override;
bool has_stack () override;
bool has_registers () override;
+ bool has_execution (ptid_t) override { return false; }
bool thread_alive (ptid_t ptid) override;
};
--
2.14.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] Introduce process_stratum_target
2018-11-30 15:40 ` Tom Tromey
@ 2018-11-30 17:52 ` Pedro Alves
0 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2018-11-30 17:52 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
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(-)
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 97592e5b1f..d61fd4aac3 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -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;
diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
index 6138d450d9..d6804740d3 100644
--- a/gdb/bfd-target.c
+++ b/gdb/bfd-target.c
@@ -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);
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 3229a18055..3fcb7547a6 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -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;
diff --git a/gdb/exec.c b/gdb/exec.c
index 615fb2b5db..44b212a4d4 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -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,
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index af97c7de2f..9859e4fddd 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -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
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 74acec2629..3c0998e02f 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -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
diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
index 28b49a4e8f..bc16c886cd 100755
--- a/gdb/make-target-delegates
+++ b/gdb/make-target-delegates
@@ -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};
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index 74640aa1ed..ce731ebf83 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -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; }
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 0f2889cf0e..856f40bd16 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -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;
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 814f080941..1ca0176ec8 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -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;
diff --git a/gdb/record-full.c b/gdb/record-full.c
index dbaa8c3d40..66234dbc43 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -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;
diff --git a/gdb/record.c b/gdb/record.c
index fdc76f80cc..05a9960f77 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -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 ());
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 69e42a2722..50a905695c 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -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. */
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index c6a5aca501..a35137fab9 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -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;
diff --git a/gdb/spu-multiarch.c b/gdb/spu-multiarch.c
index 7e642d663a..95c56f08db 100644
--- a/gdb/spu-multiarch.c
+++ b/gdb/spu-multiarch.c
@@ -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;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 1e70823fce..2ff800cca3 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -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;
diff --git a/gdb/target.c b/gdb/target.c
index ecfdde93a1..80b8453176 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -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 ());
}
diff --git a/gdb/target.h b/gdb/target.h
index b0469bba14..abe0ee4a93 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -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]; }
--
2.14.4
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-11-30 17:52 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 20:22 [PATCH 0/3] Introduce process_stratum_target Pedro Alves
2018-11-27 20:22 ` [PATCH 1/3] Move test_target_ops to a separate file Pedro Alves
2018-11-27 20:23 ` [PATCH 2/3] Introduce process_stratum_target Pedro Alves
2018-11-29 18:26 ` Tom Tromey
2018-11-29 23:16 ` Tom Tromey
2018-11-30 14:22 ` Pedro Alves
2018-11-30 15:40 ` Tom Tromey
2018-11-30 17:52 ` Pedro Alves
2018-11-27 20:23 ` [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods Pedro Alves
2018-11-29 18:31 ` Tom Tromey
2018-11-30 16:31 ` Pedro Alves
2018-11-29 18:31 ` [PATCH 0/3] Introduce process_stratum_target Tom Tromey
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).