From patchwork Fri Nov 30 14:22:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 30416 Received: (qmail 92811 invoked by alias); 30 Nov 2018 14:22:22 -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 92789 invoked by uid 89); 30 Nov 2018 14:22:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=17307 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 14:22:15 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5A38A308FB87; Fri, 30 Nov 2018 14:22:14 +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 58D4418C5D; Fri, 30 Nov 2018 14:22:13 +0000 (UTC) Subject: Re: [PATCH 2/3] Introduce process_stratum_target To: Tom Tromey References: <20181127202247.7646-1-palves@redhat.com> <20181127202247.7646-3-palves@redhat.com> <87bm67bv0h.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <69876a1c-d4fa-deeb-b124-53fce26f4cc0@redhat.com> Date: Fri, 30 Nov 2018 14:22:12 +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: <87bm67bv0h.fsf@tromey.com> On 11/29/2018 06:26 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves 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 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]; }