Restore GNU/Hurd functionality (was: Modernize solaris threads support.)
Thomas Schwinge
On Tue, Feb 24, 2009 at 02:38:58PM +0000, Pedro Alves wrote:
> On Tuesday 24 February 2009 14:33:05, Pierre Muller wrote:
> > Pedro,
> > you also broke windows-nat.c compilation...
>
> Uh! Darn it. [...]
... and another one (which I noticed only now...): when we recently got a
new Debian gdb package, all attempts to use GDB on GNU/Hurd started
failing like this:
$ gdb /bin/true
GNU gdb (GDB) 6.8.50.20090628-cvs-debian
Copyright (C) 2009 Free Software Foundation, Inc.
[...]
(no debugging symbols found)
(gdb) r
Starting program: /bin/true
Segmentation fault
> In any case, as I said before, I had to touch most *native*
> configurations, so it's likelly that I missed several cases. If
> you do spot one, the fix is *dead trivial*, so go ahead and commit a
> fix as obvious ...
You did change the ``extern gnu_store_registers'' and ``extern
gnu_fetch_registers'' declarations in gnu-nat.c, but not their definitons
in i386gnu-nat.c, which I have committed now (as obvious). Should we
perhaps move these two declarations into a file that is #included from
i386gnu-nat.c? gnu-nat.h perhaps?
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.10725
diff -u -p -r1.10725 ChangeLog
--- ChangeLog 18 Jul 2009 23:35:30 -0000 1.10725
+++ ChangeLog 20 Jul 2009 09:50:16 -0000
@@ -1,3 +1,8 @@
+2009-07-20 Thomas Schwinge <tsch...@gnu.org>
+
+ * i386gnu-nat.c (gnu_fetch_registers, gnu_store_registers): Adjust to
+ 2009-02-23 target_ops changes.
+
2009-07-18 Michael Snyder <msn...@vmware.com>
* infrun.c (handle_inferior_event): Remove an execution_direction
Index: i386gnu-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386gnu-nat.c,v
retrieving revision 1.36
diff -u -p -r1.36 i386gnu-nat.c
--- i386gnu-nat.c 3 Jan 2009 05:57:52 -0000 1.36
+++ i386gnu-nat.c 20 Jul 2009 09:50:16 -0000
@@ -111,7 +111,8 @@ supply_fpregset (struct regcache *regcac
/* Fetch register REGNO, or all regs if REGNO is -1. */
void
-gnu_fetch_registers (struct regcache *regcache, int regno)
+gnu_fetch_registers (struct target_ops *ops,
+ struct regcache *regcache, int regno)
{
struct proc *thread;
@@ -202,7 +203,8 @@ store_fpregs (const struct regcache *reg
/* Store at least register REGNO, or all regs if REGNO == -1. */
void
-gnu_store_registers (struct regcache *regcache, int regno)
+gnu_store_registers (struct target_ops *ops,
+ struct regcache *regcache, int regno)
{
struct proc *thread;
struct gdbarch *gdbarch = get_regcache_arch (regcache);
Regards,
Thomas
Pedro Alves
> You did change the ``extern gnu_store_registers'' and ``extern
> gnu_fetch_registers'' declarations in gnu-nat.c, but not their definitons
> in i386gnu-nat.c, which I have committed now (as obvious).
Sorry about that, and thanks for fixing!
> Should we
> perhaps move these two declarations into a file that is #included from
> i386gnu-nat.c? gnu-nat.h perhaps?
I've had the patch below here for months already (I think I wrote it
around the the time of the solaris changes), but,
since I had borked by GNU/Hurd setup and hadn't found the energy
yet to restore it, I completelly forgot about it. :-(
It does some house cleaning on the gnu target (by making it inherit from
the inf-child target), and ends up up making those functions static
(while at the same time, getting the port a bit more into shape for
a future !x86 Hurd port).
What do you think about it? Could you try it if you like it?
--
Pedro Alves
2009-07-20 Pedro Alves <pe...@codesourcery.com>
* gnu-nat.c (gnu_mourn_inferior): Use the passed in target_ops
instead of the gnu_ops global.
(gnu_create_inferior): Inline `attach_to_child', use the passed in
target_ops instead of the gnu_ops global.
(gnu_can_run): Delete.
(gnu_attach): Use the passed in target_ops instead of the gnu_ops
global.
(gnu_detach): Ditto.
(gnu_prepare_to_store, gnu_open): Delete.
(gnu_store_registers, gnu_fetch_registers): Delete declarations.
(gnu_ops): Delete.
(init_gnu_ops): Delete.
(gnu_target): New.
(_initialize_gnu_nat): Don't call init_gnu_ops or add_target here.
* gnu-nat.h (gnu_target): Declare.
* i386gnu-nat.c (gnu_fetch_registers, gnu_store_registers): Make
static.
(_initialize_i386gnu_nat): New.
---
gdb/gnu-nat.c | 148 +++++++++++++++++-------------------------------------
gdb/gnu-nat.h | 4 +
gdb/i386gnu-nat.c | 22 +++++++-
3 files changed, 73 insertions(+), 101 deletions(-)
Index: src/gdb/gnu-nat.c
===================================================================
--- src.orig/gdb/gnu-nat.c 2009-07-20 11:11:06.000000000 +0100
+++ src/gdb/gnu-nat.c 2009-07-20 11:21:46.000000000 +0100
@@ -87,8 +87,6 @@ int gnu_debug_flag = 0;
/* Forward decls */
-extern struct target_ops gnu_ops;
-
struct inf *make_inf ();
void inf_clear_wait (struct inf *inf);
void inf_cleanup (struct inf *inf);
@@ -2049,7 +2047,7 @@ gnu_mourn_inferior (struct target_ops *o
{
inf_debug (gnu_current_inf, "rip");
inf_detach (gnu_current_inf);
- unpush_target (&gnu_ops);
+ unpush_target (ops);
generic_mourn_inferior ();
}
@@ -2082,6 +2080,7 @@ gnu_create_inferior (struct target_ops *
int from_tty)
{
struct inf *inf = cur_inf ();
+ int pid;
void trace_me ()
{
@@ -2090,34 +2089,31 @@ gnu_create_inferior (struct target_ops *
if (ptrace (PTRACE_TRACEME) != 0)
error (_("ptrace (PTRACE_TRACEME) failed!"));
}
- void attach_to_child (int pid)
- {
- /* Attach to the now stopped child, which is actually a shell... */
- inf_debug (inf, "attaching to child: %d", pid);
- inf_attach (inf, pid);
+ inf_debug (inf, "creating inferior");
- push_target (&gnu_ops);
+ pid = fork_inferior (exec_file, allargs, env, trace_me, NULL, NULL, NULL);
- inf->pending_execs = 2;
- inf->nomsg = 1;
- inf->traced = 1;
+ /* Attach to the now stopped child, which is actually a shell... */
+ inf_debug (inf, "attaching to child: %d", pid);
- /* Now let the child run again, knowing that it will stop immediately
- because of the ptrace. */
- inf_resume (inf);
+ inf_attach (inf, pid);
- /* We now have thread info. */
- thread_change_ptid (inferior_ptid,
- ptid_build (inf->pid, 0, inf_pick_first_thread ()));
+ push_target (ops);
- startup_inferior (inf->pending_execs);
- }
+ inf->pending_execs = 2;
+ inf->nomsg = 1;
+ inf->traced = 1;
- inf_debug (inf, "creating inferior");
+ /* Now let the child run again, knowing that it will stop
+ immediately because of the ptrace. */
+ inf_resume (inf);
- fork_inferior (exec_file, allargs, env, trace_me, attach_to_child,
- NULL, NULL);
+ /* We now have thread info. */
+ thread_change_ptid (inferior_ptid,
+ ptid_build (inf->pid, 0, inf_pick_first_thread ()));
+
+ startup_inferior (inf->pending_execs);
inf_validate_procinfo (inf);
inf_update_signal_thread (inf);
@@ -2131,14 +2127,6 @@ gnu_create_inferior (struct target_ops *
inf_restore_exc_ports (inf);
}
-/* Mark our target-struct as eligible for stray "run" and "attach"
- commands. */
-static int
-gnu_can_run (void)
-{
- return 1;
-}
-
/* Attach to process PID, then initialize for debugging it
and wait for the trace-trap that results from attaching. */
@@ -2175,7 +2163,7 @@ gnu_attach (struct target_ops *ops, char
inf_attach (inf, pid);
- push_target (&gnu_ops);
+ push_target (ops);
inferior = add_inferior (pid);
inferior->attach_flag = 1;
@@ -2231,7 +2219,7 @@ gnu_detach (struct target_ops *ops, char
inferior_ptid = null_ptid;
detach_inferior (pid);
- unpush_target (&gnu_ops); /* Pop out of handling an inferior */
+ unpush_target (ops); /* Pop out of handling an inferior */
}
static void
@@ -2241,22 +2229,6 @@ gnu_terminal_init_inferior (void)
terminal_init_inferior_with_pgrp (gnu_current_inf->pid);
}
-/* Get ready to modify the registers array. On machines which store
- individual registers, this doesn't need to do anything. On machines
- which store all the registers in one fell swoop, this makes sure
- that registers contains all the registers from the program being
- debugged. */
-static void
-gnu_prepare_to_store (struct regcache *regcache)
-{
-}
-
-static void
-gnu_open (char *arg, int from_tty)
-{
- error (_("Use the \"run\" command to start a Unix child process."));
-}
-
static void
gnu_stop (ptid_t ptid)
{
@@ -2615,53 +2587,34 @@ gnu_pid_to_str (struct target_ops *ops,
}
-extern void gnu_store_registers (struct target_ops *ops,
- struct regcache *regcache, int regno);
-extern void gnu_fetch_registers (struct target_ops *ops,
- struct regcache *regcache, int regno);
-
-struct target_ops gnu_ops;
-
-static void
-init_gnu_ops (void)
-{
- gnu_ops.to_shortname = "GNU"; /* to_shortname */
- gnu_ops.to_longname = "GNU Hurd process"; /* to_longname */
- gnu_ops.to_doc = "GNU Hurd process"; /* to_doc */
- gnu_ops.to_open = gnu_open; /* to_open */
- gnu_ops.to_attach = gnu_attach; /* to_attach */
- gnu_ops.to_attach_no_wait = 1; /* to_attach_no_wait */
- gnu_ops.to_detach = gnu_detach; /* to_detach */
- gnu_ops.to_resume = gnu_resume; /* to_resume */
- gnu_ops.to_wait = gnu_wait; /* to_wait */
- gnu_ops.to_fetch_registers = gnu_fetch_registers; /* to_fetch_registers */
- gnu_ops.to_store_registers = gnu_store_registers; /* to_store_registers */
- gnu_ops.to_prepare_to_store = gnu_prepare_to_store; /* to_prepare_to_store */
- gnu_ops.deprecated_xfer_memory = gnu_xfer_memory;
- gnu_ops.to_find_memory_regions = gnu_find_memory_regions;
- gnu_ops.to_insert_breakpoint = memory_insert_breakpoint;
- gnu_ops.to_remove_breakpoint = memory_remove_breakpoint;
- gnu_ops.to_terminal_init = gnu_terminal_init_inferior;
- gnu_ops.to_terminal_inferior = terminal_inferior;
- gnu_ops.to_terminal_ours_for_output = terminal_ours_for_output;
- gnu_ops.to_terminal_save_ours = terminal_save_ours;
- gnu_ops.to_terminal_ours = terminal_ours;
- gnu_ops.to_terminal_info = child_terminal_info;
- gnu_ops.to_kill = gnu_kill_inferior; /* to_kill */
- gnu_ops.to_create_inferior = gnu_create_inferior; /* to_create_inferior */
- gnu_ops.to_mourn_inferior = gnu_mourn_inferior; /* to_mourn_inferior */
- gnu_ops.to_can_run = gnu_can_run; /* to_can_run */
- gnu_ops.to_thread_alive = gnu_thread_alive; /* to_thread_alive */
- gnu_ops.to_pid_to_str = gnu_pid_to_str; /* to_pid_to_str */
- gnu_ops.to_stop = gnu_stop; /* to_stop */
- gnu_ops.to_stratum = process_stratum; /* to_stratum */
- gnu_ops.to_has_all_memory = default_child_has_all_memory;
- gnu_ops.to_has_memory = default_child_has_memory;
- gnu_ops.to_has_stack = default_child_has_stack;
- gnu_ops.to_has_registers = default_child_has_registers;
- gnu_ops.to_has_execution = default_child_has_execution;
- gnu_ops.to_magic = OPS_MAGIC; /* to_magic */
-} /* init_gnu_ops */
+/* Create a prototype generic GNU/Hurd target. The client can
+ override it with local methods. */
+
+struct target_ops *
+gnu_target (void)
+{
+ struct target_ops *t = inf_child_target ();
+
+ t->to_shortname = "GNU";
+ t->to_longname = "GNU Hurd process";
+ t->to_doc = "GNU Hurd process";
+
+ t->to_attach = gnu_attach;
+ t->to_attach_no_wait = 1;
+ t->to_detach = gnu_detach;
+ t->to_resume = gnu_resume;
+ t->to_wait = gnu_wait;
+ t->deprecated_xfer_memory = gnu_xfer_memory;
+ t->to_find_memory_regions = gnu_find_memory_regions;
+ t->to_terminal_init = gnu_terminal_init_inferior;
+ t->to_kill = gnu_kill_inferior;
+ t->to_create_inferior = gnu_create_inferior;
+ t->to_mourn_inferior = gnu_mourn_inferior;
+ t->to_can_run = gnu_can_run;
+ t->to_thread_alive = gnu_thread_alive;
+ t->to_pid_to_str = gnu_pid_to_str;
+ t->to_stop = gnu_stop;
+}
/* User task commands. */
@@ -3405,9 +3358,6 @@ void
_initialize_gnu_nat (void)
{
proc_server = getproc ();
-
- init_gnu_ops ();
- add_target (&gnu_ops);
add_task_commands ();
add_thread_commands ();
Index: src/gdb/gnu-nat.h
===================================================================
--- src.orig/gdb/gnu-nat.h 2009-07-20 11:11:06.000000000 +0100
+++ src/gdb/gnu-nat.h 2009-07-20 11:12:40.000000000 +0100
@@ -98,4 +98,8 @@ extern int gnu_debug_flag;
do { if (gnu_debug_flag) \
fprintf_unfiltered (gdb_stdlog, "%s:%d: " msg "\r\n", __FILE__ , __LINE__ , ##args); } while (0)
+/* Create a prototype generic GNU/Hurd target. The client can
+ override it with local methods. */
+struct target_ops *gnu_target (void);
+
#endif /* __GNU_NAT_H__ */
Index: src/gdb/i386gnu-nat.c
===================================================================
--- src.orig/gdb/i386gnu-nat.c 2009-07-20 11:11:23.000000000 +0100
+++ src/gdb/i386gnu-nat.c 2009-07-20 11:13:31.000000000 +0100
@@ -110,7 +110,7 @@ supply_fpregset (struct regcache *regcac
#endif
/* Fetch register REGNO, or all regs if REGNO is -1. */
-void
+static void
gnu_fetch_registers (struct target_ops *ops,
struct regcache *regcache, int regno)
{
@@ -202,7 +202,7 @@ store_fpregs (const struct regcache *reg
}
/* Store at least register REGNO, or all regs if REGNO == -1. */
-void
+static void
gnu_store_registers (struct target_ops *ops,
struct regcache *regcache, int regno)
{
@@ -294,3 +294,21 @@ gnu_store_registers (struct target_ops *
store_fpregs (regcache, thread, regno);
}
}
+
+/* Provide a prototype to silence -Wmissing-prototypes. */
+extern initialize_file_ftype _initialize_i386gnu_nat;
+
+void
+_initialize_i386gnu_nat (void)
+{
+ struct target_ops *t;
+
+ /* Fill in the generic GNU/Hurd methods. */
+ t = gnu_target ();
+
+ t->to_fetch_registers = gnu_fetch_registers;
+ t->to_store_registers = gnu_store_registers;
+
+ /* Register the target. */
+ add_target (t);
+}
Thomas Schwinge
On Mon, Jul 20, 2009 at 11:22:34AM +0100, Pedro Alves wrote:
> I've had the patch below here for months already (I think I wrote it
> around the the time of the solaris changes), but,
> since I had borked by GNU/Hurd setup and hadn't found the energy
> yet to restore it, I completelly forgot about it. :-(
Well, thanks to you for helping with maintining GDB's Hurd port! If you
need help with a new Hurd image, or want shell access to a system
(<http://www.gnu.org/software/hurd/public_hurd_boxen.html>), please just
speak up. (Likewise everyone else who is interested, of course!)
> It does some house cleaning on the gnu target (by making it inherit from
> the inf-child target), and ends up up making those functions static
> (while at the same time, getting the port a bit more into shape for
> a future !x86 Hurd port).
>
> What do you think about it? Could you try it if you like it?
I did review it and did some testing in the areas that could most likely
be touched by it and found no problems. I only had to do three very
minor corrections:
> Index: src/gdb/gnu-nat.c
> @@ -2082,6 +2080,7 @@ gnu_create_inferior (struct target_ops *
> int from_tty)
> {
> struct inf *inf = cur_inf ();
> + int pid;
>
> void trace_me ()
> {
> @@ -2090,34 +2089,31 @@ gnu_create_inferior (struct target_ops *
> if (ptrace (PTRACE_TRACEME) != 0)
> error (_("ptrace (PTRACE_TRACEME) failed!"));
> }
> - void attach_to_child (int pid)
> - {
> - /* Attach to the now stopped child, which is actually a shell... */
> - inf_debug (inf, "attaching to child: %d", pid);
>
> - inf_attach (inf, pid);
> + inf_debug (inf, "creating inferior");
>
> - push_target (&gnu_ops);
> + pid = fork_inferior (exec_file, allargs, env, trace_me, NULL, NULL, NULL);
> [...]
Whether it is better to do it the old way ( have a nested function
attach_to_child that will be passed to and called from within
fork_inferior), or do it like your patch does (inline the former
attach_to_child to be executed after fork_inferior has returned) -- I
have no idea, so I'll leave that to you.
> +/* Create a prototype generic GNU/Hurd target. The client can
> + override it with local methods. */
> +
> +struct target_ops *
> +gnu_target (void)
> +{
> + struct target_ops *t = inf_child_target ();
That one needs ``#include "inf-child.h"''.
> + t->to_can_run = gnu_can_run;
This statement should be removed: the default value (as set by
inf_child_target) is alright and you removed gnu_can_run just above.
> + t->to_thread_alive = gnu_thread_alive;
> + t->to_pid_to_str = gnu_pid_to_str;
> + t->to_stop = gnu_stop;
> +}
``return t;'' is missing.
Regards,
Thomas
Pedro Alves
> Well, thanks to you for helping with maintining GDB's Hurd port! If you
> need help with a new Hurd image, or want shell access to a system
> (<http://www.gnu.org/software/hurd/public_hurd_boxen.html>), please just
> speak up. (Likewise everyone else who is interested, of course!)
Cool! I just might.
> Whether it is better to do it the old way ( have a nested function
> attach_to_child that will be passed to and called from within
> fork_inferior), or do it like your patch does (inline the former
> attach_to_child to be executed after fork_inferior has returned) -- I
> have no idea, so I'll leave that to you.
Yeah. I needed to pass the target_ops argument to
attach_to_child, since gnu_ops is now gone. This way was simpler,
as it avoids having to change the callback's interface. That callback
used to make sense when fork_inferior did some extra
work after calling it, and before returning; but, fork_inferior
doesn't do that anymore, it just calls the callback and returns.
We've done the same change to inf-ptrace.c recently-ish
(inlined the corresponding function when we needed the extra
argument, instead of changing the callback's interface).
>
> > +/* Create a prototype generic GNU/Hurd target. The client can
> > + override it with local methods. */
> > +
> > +struct target_ops *
> > +gnu_target (void)
> > +{
> > + struct target_ops *t = inf_child_target ();
>
> That one needs ``#include "inf-child.h"''.
Fixed.
>
> > + t->to_can_run = gnu_can_run;
>
> This statement should be removed: the default value (as set by
> inf_child_target) is alright and you removed gnu_can_run just above.
Fixed.
>
> > + t->to_thread_alive = gnu_thread_alive;
> > + t->to_pid_to_str = gnu_pid_to_str;
> > + t->to_stop = gnu_stop;
> > +}
>
> ``return t;'' is missing.
Fixed.
Thanks. I'll commit the patch in a bit.
--
Pedro Alves