Patchwork [RFC] Change the_dummy_target to be a global

login
register
mail settings
Submitter Tom Tromey
Date March 4, 2019, 8:41 p.m.
Message ID <20190304204100.27702-1-tromey@adacore.com>
Download mbox | patch
Permalink /patch/31717/
State New
Headers show

Comments

Tom Tromey - March 4, 2019, 8:41 p.m.
While debugging gdb, I printed the target stack and got:

    (top-gdb) p g_target_stack
    $10 = {
      m_top = thread_stratum,
      m_stack = {0x142b0b0, 0x13da600 <exec_ops>, 0x1c70690, 0x13d63b0 <ravenscar_ops>, 0x0, 0x0, 0x0}
    }

(This is clearly from before the change to make ravenscar
multi-target-capable.)

Here, 0x142b0b0 is the singleton dummy target.  It seems to me that
since this is always a singleton, it would be a bit nicer if it were a
global, so that it would be noted in the above.

This patch implements this idea, and now I get:

    (top-gdb) p g_target_stack
    $2 = {
      m_top = dummy_stratum,
      m_stack = {0x1f1b040 <the_dummy_target>, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
    }

I did not do the same for the debug target.  It didn't seem as useful
to me.

gdb/ChangeLog
2019-03-04  Tom Tromey  <tromey@adacore.com>

	* target.c (the_dummy_target): Move later.  Change type to
	"dummy_target".
	(initialize_targets): Don't initialize the_dummy_target.
---
 gdb/ChangeLog |  6 ++++++
 gdb/target.c  | 11 ++++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)
Pedro Alves - March 5, 2019, 8:15 p.m.
On 03/04/2019 08:41 PM, Tom Tromey wrote:
> While debugging gdb, I printed the target stack and got:
> 
>     (top-gdb) p g_target_stack
>     $10 = {
>       m_top = thread_stratum,
>       m_stack = {0x142b0b0, 0x13da600 <exec_ops>, 0x1c70690, 0x13d63b0 <ravenscar_ops>, 0x0, 0x0, 0x0}
>     }
> 
> (This is clearly from before the change to make ravenscar
> multi-target-capable.)
> 
> Here, 0x142b0b0 is the singleton dummy target.  It seems to me that
> since this is always a singleton, it would be a bit nicer if it were a
> global, so that it would be noted in the above.
> 
> This patch implements this idea, and now I get:
> 
>     (top-gdb) p g_target_stack
>     $2 = {
>       m_top = dummy_stratum,
>       m_stack = {0x1f1b040 <the_dummy_target>, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
>     }
> 
> I did not do the same for the debug target.  It didn't seem as useful
> to me.
> 
> gdb/ChangeLog
> 2019-03-04  Tom Tromey  <tromey@adacore.com>
> 
> 	* target.c (the_dummy_target): Move later.  Change type to
> 	"dummy_target".
> 	(initialize_targets): Don't initialize the_dummy_target.

Yes, I think that's fine.

At some point in the multi-target branch I made a change like that,
but the current state of the branch it's back to being heap-allocated.
I'm not seeing any reason that requires that, though.  I replicated
the change on top of the branch and ran the new multi-target tests,
and they all still passed.

Thanks,
Pedro Alves
Tom Tromey - March 5, 2019, 8:46 p.m.
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Yes, I think that's fine.

Pedro> At some point in the multi-target branch I made a change like that,
Pedro> but the current state of the branch it's back to being heap-allocated.
Pedro> I'm not seeing any reason that requires that, though.  I replicated
Pedro> the change on top of the branch and ran the new multi-target tests,
Pedro> and they all still passed.

Thanks, I'll check it in then.

Tom

Patch

diff --git a/gdb/target.c b/gdb/target.c
index d5ff932c748..5c0375a59a3 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -106,10 +106,8 @@  static enum exec_direction_kind default_execution_direction
 static std::unordered_map<const target_info *, target_open_ftype *>
   target_factories;
 
-/* The initial current target, so that there is always a semi-valid
-   current target.  */
+/* The singleton debug target.  */
 
-static struct target_ops *the_dummy_target;
 static struct target_ops *the_debug_target;
 
 /* The target stack.  */
@@ -3240,6 +3238,10 @@  dummy_make_corefile_notes (struct target_ops *self,
 
 #include "target-delegates.c"
 
+/* The initial current target, so that there is always a semi-valid
+   current target.  */
+
+static dummy_target the_dummy_target;
 
 static const target_info dummy_target_info = {
   "None",
@@ -3977,8 +3979,7 @@  set_write_memory_permission (const char *args, int from_tty,
 void
 initialize_targets (void)
 {
-  the_dummy_target = new dummy_target ();
-  push_target (the_dummy_target);
+  push_target (&the_dummy_target);
 
   the_debug_target = new debug_target ();