From patchwork Fri Nov 30 16:31:33 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 30464 Received: (qmail 57852 invoked by alias); 30 Nov 2018 16:31:53 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 57701 invoked by uid 89); 30 Nov 2018 16:31:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=debugger, sk:PROCESS X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 30 Nov 2018 16:31:35 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C09EA3081D89; Fri, 30 Nov 2018 16:31:34 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id DA31B60BE7; Fri, 30 Nov 2018 16:31:33 +0000 (UTC) Subject: Re: [PATCH 3/3] Convert default_child_has_foo functions to process_stratum_target methods To: Tom Tromey References: <20181127202247.7646-1-palves@redhat.com> <20181127202247.7646-4-palves@redhat.com> <877egvburv.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <0ed52acf-0035-b80c-da21-c3870c46d6dc@redhat.com> Date: Fri, 30 Nov 2018 16:31:33 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <877egvburv.fsf@tromey.com> On 11/29/2018 06:31 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves 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 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 * corelow.c (core_target) : 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) : 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) : New method overrides. * ravenscar-thread.c (ravenscar_thread_target) : Delete. * remote-sim.c (gdbsim_target) : Delete. * remote.c (remote_target) : 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) : 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 + + * corelow.c (core_target) : 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) : 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) + : New method overrides. + * ravenscar-thread.c (ravenscar_thread_target) : Delete. + * remote-sim.c (gdbsim_target) : Delete. + * remote.c (remote_target) : 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) : New override. + 2018-11-30 Pedro Alves * 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 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; };