* [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 3/3] Convert default_child_has_foo functions to process_stratum_target methods 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 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 ` Pedro Alves 2018-11-29 18:31 ` Tom Tromey 2018-11-27 20:23 ` [PATCH 2/3] Introduce process_stratum_target Pedro Alves 2018-11-29 18:31 ` [PATCH 0/3] " 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 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 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
* [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 ` [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods Pedro Alves @ 2018-11-27 20:23 ` Pedro Alves 2018-11-29 18:26 ` Tom Tromey 2018-11-29 18:31 ` [PATCH 0/3] " 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
* 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 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 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
* 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 2/3] Introduce process_stratum_target 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
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 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-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-29 18:31 ` [PATCH 0/3] " 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).