This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/3] Introduce process_stratum_target


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]