Re: [PATCH] hurd: Add shared mig declarations
On 5/30/20 7:23 PM, Samuel Thibault wrote: > +++ binutils-gdb/gdb/gnu-nat-mig.h > @@ -0,0 +1,33 @@ > +/* Common things used by the various *gnu-nat.c files > + Copyright (C) 2020 Free Software Foundation, Inc. > + > + Written by Samuel Thibault Please don't take this the wrong way, but a while ago the GDB project (following Glibc's lead) decided that we would not add more "contributed by", "written by", etc. notes. See here <https://sourceware.org/gdb/wiki/ContributionChecklist#Attribution> and follow the link there for rationale. Thanks, Pedro Alves
Re: hurd: update RPC prototypes
On 09/06/2017 11:11 PM, Samuel Thibault wrote: > Pedro Alves, on lun. 04 sept. 2017 13:14:33 +0100, wrote: >> On 08/27/2017 07:41 PM, Samuel Thibault wrote: >>> Since hurd's baf7e5c ('hurd: Use polymorphic port types to return some >>> rights.'), some RPCs prototypes have changed, gdb needs the >>> corresponding update. >>> >>> * gdb/gnu-nat.c (S_proc_getmsgport_reply, S_proc_task2proc_reply, >>> S_proc_pid2proc_reply): Add `mach_msg_type_name_t type' parameter. >> >> Say someone downloads some prebuilt Debian Hurd image or some >> such and wants to build newer gdb on that system. I assume that that would >> be >> broken with this change? > > Yes. > >> What's the policy regarding building ToT gdb on non-ToT Hurd systems? >> Is the intention to only ever support building ToT gdb with ToT Hurd? > > ATM we don't really support backward compatibility for mixtures of > versions. OK, seems to me that raises bar of entry to gdb/hurd hacking, but really up to you guys. A few years back, when I did some across-all-gdb-targets changes, I used the prebuilt Debian Hurd qemu image to do the corresponding Hurd changes. Guess I may have been lucky then to not hit some version skew. Thanks, Pedro Alves
Re: hurd: update RPC prototypes
On 08/27/2017 07:41 PM, Samuel Thibault wrote: > Since hurd's baf7e5c ('hurd: Use polymorphic port types to return some > rights.'), some RPCs prototypes have changed, gdb needs the > corresponding update. > > * gdb/gnu-nat.c (S_proc_getmsgport_reply, S_proc_task2proc_reply, > S_proc_pid2proc_reply): Add `mach_msg_type_name_t type' parameter. Say someone downloads some prebuilt Debian Hurd image or some such and wants to build newer gdb on that system. I assume that that would be broken with this change? What's the policy regarding building ToT gdb on non-ToT Hurd systems? Is the intention to only ever support building ToT gdb with ToT Hurd? Thanks, Pedro Alves > > diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c > index d5e3841e68..402027866b 100644 > --- a/gdb/gnu-nat.c > +++ b/gdb/gnu-nat.c > @@ -1874,17 +1876,19 @@ ILL_RPC (S_proc_setmsgport_reply, >mach_port_t oldmsgport) > ILL_RPC (S_proc_getmsgport_reply, >mach_port_t reply_port, kern_return_t return_code, > - mach_port_t msgports) > + mach_port_t msgports, mach_msg_type_name_t type) > ILL_RPC (S_proc_pid2task_reply, >mach_port_t reply_port, kern_return_t return_code, mach_port_t task) > ILL_RPC (S_proc_task2pid_reply, >mach_port_t reply_port, kern_return_t return_code, pid_t pid) > ILL_RPC (S_proc_task2proc_reply, > - mach_port_t reply_port, kern_return_t return_code, mach_port_t proc) > + mach_port_t reply_port, kern_return_t return_code, > + mach_port_t proc, mach_msg_type_name_t type) > ILL_RPC (S_proc_proc2task_reply, >mach_port_t reply_port, kern_return_t return_code, mach_port_t task) > ILL_RPC (S_proc_pid2proc_reply, > - mach_port_t reply_port, kern_return_t return_code, mach_port_t proc) > + mach_port_t reply_port, kern_return_t return_code, > + mach_port_t proc, mach_msg_type_name_t type) > ILL_RPC (S_proc_getprocinfo_reply, >mach_port_t reply_port, kern_return_t return_code, >int flags, procinfo_t procinfo, mach_msg_type_number_t procinfoCnt, >
Re: [pushed][PATCH v3 1/4] Extended-remote follow exec
Hi Thomas, Only noticed this patch now. > On GNU/Hurd, there is no "#define PATH_MAX", so this fails to build. > (I'm aware that there is other PATH_MAX usage in GDB sources, which we > ought to fix at some point, for example in gdbserver -- which is not yet > enabled for GNU/Hurd.) > > OK to push the following? (Similar to Svante's patch in > <https://bugs.debian.org/834575>.) > > --- gdb/remote.c > +++ gdb/remote.c > @@ -6927,7 +6927,6 @@ Packet: '%s'\n"), > else if (strprefix (p, p1, "exec")) > { > ULONGEST ignored; > - char pathname[PATH_MAX]; > int pathlen; > > /* Determine the length of the execd pathname. */ > @@ -6936,11 +6935,12 @@ Packet: '%s'\n"), > > /* Save the pathname for event reporting and for >the next run command. */ > + char *pathname = (char *) xmalloc (pathlen + 1); > hex2bin (p1, (gdb_byte *) pathname, pathlen); > pathname[pathlen] = '\0'; hex2bin can throw, so wrap with a cleanup: char *pathname = (char *) xmalloc (pathlen + 1); struct cleanup *old_chain = make_cleanup (xfree, pathname); hex2bin (p1, (gdb_byte *) pathname, pathlen); pathname[pathlen] = '\0'; discard_cleanups (old_chain); OK with that change. Thanks, Pedro Alves
Re: [PATCH] Make STARTUP_WITH_SHELL a runtime toggle -- add new set/show startup-with-shell option.
On 01/08/2014 09:20 PM, Thomas Schwinge wrote: Hi! On Thu, 24 Oct 2013 16:17:21 +0100, Pedro Alves pal...@redhat.com wrote: Here's what I pushed ..., and what made the Hurd port pretty unhappy. ;-) Whoops. ;-) Thanks for fixing. In the thread around http://news.gmane.org/find-root.php?message_id=%3C200810110047.39807.pedro%40codesourcery.com%3E, and http://news.gmane.org/find-root.php?message_id=%3C200810131935.35253.pedro%40codesourcery.com%3E, we had already (very) briefly been discussing gnu-nat's local pending_execs handling. Ah, these things do have a tendency to byte back. Is the new approach that I'm posting below (flag instead of counting; saves us from repeating in gnu_create_inferior the increment in the startup_with_shell case) OK until we get a clear understanding if and how we can get rid of it for good? It's fine with me. commit 57c9fb3afadab5813d7463dc2393d5affe78849e Author: Thomas Schwinge tho...@codesourcery.com Date: Wed Jan 8 21:42:07 2014 +0100 Hurd: Adjust to startup-with-shell changes. In commit 98882a26513e25b2161b41dfd4bed97b59b2c01a, STARTUP_WITH_SHELL was made a runtime toggle, startup-with-shell. The Hurd code not adjusted, which had a ... code was not ... value hard-coded instead of using START_INFERIOR_TRAPS_EXPECTED. Fix that, and also simplify gnu-nat's pending_execs handling from counting to just a flag. -- Pedro Alves
Re: [RFC] GDB Hurd Fixes
On 09/20/2013 01:43 AM, David Michael wrote: Hi, (Copying gdb-patches this time.) But, we're missing all the context on the gdb-patches@ side. Here is an updated patch to successfully build GDB after today's Hurd/mig changes. -- Pedro Alves
Re: FAIL: gdb.base/nextoverexit.exp: next over exit (the program exited)
On 09/19/2013 09:30 AM, Thomas Schwinge wrote: Hi! On Thu, 19 Sep 2013 15:40:40 +0800, Yue Lu hacklu.newb...@gmail.com wrote: On Wed, Sep 18, 2013 at 8:37 PM, Pedro Alves pal...@redhat.com wrote: On 09/12/2013 04:05 AM, Yue Lu wrote: On Sat, Sep 7, 2013 at 2:53 AM, Pedro Alves pal...@redhat.com wrote: https://sourceware.org/ml/gdb-patches/2013-09/msg00253.html First thank you to tell me how to apply patch from email. I used webmail of gmail and directly copy patch from the email which often apply failed, then I had to patch line by line. Now I used mutt to save email to mbox file then apply it, life changed! Before you told me this, I never imaged this, so thanks! Well, never assume that we'd use any convoluted procedures, such as manually copying a patch's text. ;-) Never hesitate to ask if you think some process is too complicated to be done manually -- there will always be someone who is happy to tell you about his creative solution. I have test your patch, seems need a tiny fix. This is just a spelling mistaken I think. Right; I had come to the same conclusion, see my message in the other thread. After add this change, the gdb can work. But I have found a little strange from the origin gdb. When I set a breakpoint, then I run the inferior, after hit the breakpoint, I just input next next until the inferior exit, then the gdb will complain this: [Inferior 1 (bogus thread id 0) exited normally] Thread-specific breakpoint -37 deleted - thread 4 is gone. Thread-specific breakpoint -38 deleted - thread 4 is gone. Thread-specific breakpoint -39 deleted - thread 4 is gone. Thread-specific breakpoint 0 deleted - thread 4 is gone. I am not sure why this will output or is reasonable. I got this output like this: $./gdb gdb $b main $r $n $n ... $q (quit the debugged gdb) As of recently, I notice the same behavior for GDB on both x86 GNU/Linux and GNU/Hurd, also resulting in the gdb.base/nextoverexit.exp test failing. So, I don't think this is related to any Hurd patches/behavior, but instead a general issue. Quoting from the x86 GNU/Linux' gdb/testsuite/gdb.base2/gdb.log: Breakpoint 1, main () at ../../../Ferry_Tagscherer/gdb/testsuite/gdb.base/nextoverexit.c:21 21exit (0); (gdb) next [Inferior 1 (process 25208) exited normally] Thread-specific breakpoint -5 deleted - thread 1 is gone. Thread-specific breakpoint -6 deleted - thread 1 is gone. Thread-specific breakpoint -7 deleted - thread 1 is gone. Thread-specific breakpoint 0 deleted - thread 1 is gone. (gdb) FAIL: gdb.base/nextoverexit.exp: next over exit (the program exited) Can others confirm this/is this a known issue? Hmm, that message is new, but we shouldn't be seeing it for internal breakpoints... That'll be my fault. I'll take a look. -- Pedro Alves
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
On 09/09/2013 10:58 AM, Thomas Schwinge wrote: Hi! On Sun, 8 Sep 2013 21:35:05 +0800, Yue Lu hacklu.newb...@gmail.com wrote: On Fri, Sep 6, 2013 at 5:37 AM, Thomas Schwinge tho...@codesourcery.com wrote: (correct me if I'm wrong here), the Hurd's threads are kernel threads Correct. so it'd be better to just make the GDB side use the lwp field too. It's really a simple and mechanic change. Nothing in GDB core actually cares which field is used. So in this case, it'd be In GDB's parlance, a lightweight process (identified by a LWP) is a thread that always has a corresponding kernel thread, and in contrast a generic thread (identified by a TID) is not required to always have a corresponding kernel thread, for example, when managed by a run-time library? Then, yes, conceptually the native Hurd port should be switched to using LWPs instead of TIDs. better if you send a preparatory patch Based on the current upstream master branch. Should I change the gdb use lwp filed instead of tid field? There are too many functions use tid. Like make_proc(),inf_tid_to_thread(),ptid_build(), and there is a field named tid in the structure proc also. As you have found, there is a lot of TID usage in gnu-nat.c. TIDs are assigned based on the next_thread_id variable: /* A variable from which to assign new TIDs. */ static int next_thread_id = 1; [...] /* THREADS[I] is a thread we don't know about yet! */ { ptid_t ptid; thread = make_proc (inf, threads[i], next_thread_id++); Five years ago, we've already concluded this is due for some cleanup, http://www.gnu.org/software/hurd/open_issues/gdb_thread_ids.html. But I don't want to require this cleanup to happen before/in context of the Google Summer of Code project's code submission discussed here. That's not what I'm suggesting at all. I'm just talking about storing the thread id in the lwpid field. It's always the target that stores and extracts the fields of a ptid -- the ptid_t struct is mostly opaque to the core. It should really be a small change. (while looking at this, I noticed a bug, and fixed it: http://sourceware.org/ml/gdb-patches/2013-09/msg00579.html) /me gives it a try. I grepped for ptid_build and ptid_get_tid in *gnu* files, and adjusted all I found. --- Subject: [PATCH] [Hurd/gnu-nat.c] Use ptid_t.lwpid to store thread ids instead of ptid_t.tid. In preparation for reusing gnu-nat.c in gdbserver, switch to storing thread ids in the lwpid field of ptid_t rather than in the tid field. The Hurd's thread model is 1:1, so it doesn't feel wrong anyway. gdb/ 2013-09-18 Pedro Alves pal...@redhat.com * gnu-nat.c (inf_validate_procs, gnu_wait, gnu_resume) (gnu_create_inferior) (gnu_attach, gnu_thread_alive, gnu_pid_to_str, cur_thread) (set_sig_thread_cmd): Use the lwpid field of ptids to store the thread id instead of the tid field. --- gdb/gnu-nat.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c index fa55b10..b4f99f8 100644 --- a/gdb/gnu-nat.c +++ b/gdb/gnu-nat.c @@ -1083,7 +1083,7 @@ inf_validate_procs (struct inf *inf) last = thread; proc_debug (thread, new thread: %d, threads[i]); - ptid = ptid_build (inf-pid, 0, thread-tid); + ptid = ptid_build (inf-pid, thread-tid, 0); /* Tell GDB's generic thread code. */ @@ -1613,17 +1613,17 @@ rewait: thread = inf-wait.thread; if (thread) -ptid = ptid_build (inf-pid, 0, thread-tid); +ptid = ptid_build (inf-pid, thread-tid, 0); else if (ptid_equal (ptid, minus_one_ptid)) thread = inf_tid_to_thread (inf, -1); else -thread = inf_tid_to_thread (inf, ptid_get_tid (ptid)); +thread = inf_tid_to_thread (inf, ptid_get_lwp (ptid)); if (!thread || thread-port == MACH_PORT_NULL) { /* TID is dead; try and find a new thread. */ if (inf_update_procs (inf) inf-threads) - ptid = ptid_build (inf-pid, 0, inf-threads-tid); /* The first + ptid = ptid_build (inf-pid, inf-threads-tid, 0); /* The first available thread. */ else @@ -2022,7 +2022,7 @@ gnu_resume (struct target_ops *ops, else /* Just allow a single thread to run. */ { - struct proc *thread = inf_tid_to_thread (inf, ptid_get_tid (ptid)); + struct proc *thread = inf_tid_to_thread (inf, ptid_get_lwp (ptid)); if (!thread) error (_(Can't run single thread id %s: no such thread!), @@ -2033,7 +2033,7 @@ gnu_resume (struct target_ops *ops, if (step) { - step_thread = inf_tid_to_thread (inf, ptid_get_tid (ptid)); + step_thread = inf_tid_to_thread (inf, ptid_get_lwp (ptid)); if (!step_thread
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
On 09/12/2013 04:05 AM, Yue Lu wrote: Honestly to say, I have copied them from function gnu_xfer_memory. But I think, before reference a pointer, check whether it was a NULL seems not a bad way :-). We don't go around checking for NULL before _every_ pointer dereference. :-) NULL pointer checks are used when the function's semantics give special meaning to NULL, like for example, for optional parameters. If that's not the function's semantics, and you still may see a NULL pointer, then that tells there's actually a bug that triggered already elsewhere before the function in question was called. IOW, a NULL check in these cases is just masking out a bug in some caller. In some cases, we might add a gdb_assert to catch the bug in a nicer way instead of just crashing. I suspect that what may happen is that at some point it was possible for GDB to select a no-longer-existing-thread (though I can't see how in the current code), and then setting breakpoints, or other operations that require reading memory would crash. -- Pedro Alves
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
On 09/12/2013 04:05 AM, Yue Lu wrote: On Sat, Sep 7, 2013 at 2:53 AM, Pedro Alves pal...@redhat.com wrote: This is what I meant: https://sourceware.org/ml/gdb-patches/2013-09/msg00253.html I was thinking you'd wrap gnu_xfer_memory. I have study your patch, Thanks. Did you try building gdb with it, and does the resulting GDB still work? but I found there is no to_xfer_partial field or something similar in gdbserver's structure target_obj, how can I do? Please give me more hints, thanks. Right, there's no such function in gdbserver today. I mean, instead of: + +static int +gnu_read_memory (CORE_ADDR addr, unsigned char *myaddr, int length) +{ + int ret = 0; + task_t task = (gnu_current_inf + ? (gnu_current_inf-task +? gnu_current_inf-task-port : 0) : 0); + if (task == MACH_PORT_NULL) +return 0; + ret = gnu_read_inferior (task, addr, myaddr, length); + if (length != ret) +{ + debug (gnu_read_inferior,length=%d, but return %d\n, length, ret); + return -1; +} + return 0; +} you'll have a simpler: static int gnu_read_memory (CORE_ADDR addr, unsigned char *myaddr, int length) { LONGEST res; res = gnu_xfer_memory (...); if (res != length) return -1; return 0; } gnu_xfer_memory already has the task == MACH_PORT_NULL check inside it, so this means there's no need to duplicate it in gnu_read|write_memory. -- Pedro Alves
[Hurd/gnu-nat.c] Use ptid_t.lwpid to store, thread ids instead of ptid_t.tid. (was: Re: [PATCH 1/2] Port gdbserver to GNU/Hurd)
On 09/18/2013 02:48 PM, Yue Lu wrote: On Wed, Sep 18, 2013 at 8:11 PM, Pedro Alves pal...@redhat.com wrote: /me gives it a try. I grepped for ptid_build and ptid_get_tid in *gnu* files, and adjusted all I found. I have tried this way before, but it doesn't work. After apply your patch, the gdb can't use, it says Can't fetch registers from thread Thread 29826.3: No such thread. ... As before, I thought it is a big problem, so I don't dig into it. Your last email has reminder me, both you and I forgot to patch the i386gnu-nat.c which also used the tid filed. Eh, grep-fail then. Add this everything is ok. Nice, thanks. I've merged that in, and applied it. - Subject: [Hurd/gnu-nat.c] Use ptid_t.lwpid to store thread ids instead of ptid_t.tid. In preparation for reusing gnu-nat.c in gdbserver, switch to storing thread ids in the lwpid field of ptid_t rather than in the tid field. The Hurd's thread model is 1:1, so it doesn't feel wrong anyway. gdb/ 2013-09-18 Pedro Alves pal...@redhat.com Yue Lu hacklu.newb...@gmail.com * gnu-nat.c (inf_validate_procs, gnu_wait, gnu_resume) (gnu_create_inferior) (gnu_attach, gnu_thread_alive, gnu_pid_to_str, cur_thread) (set_sig_thread_cmd): Use the lwpid field of ptids to store/extract thread ids instead of the tid field. * i386gnu-nat.c (gnu_fetch_registers): Adjust. --- gdb/gnu-nat.c | 24 gdb/i386gnu-nat.c | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c index fa55b10..b4f99f8 100644 --- a/gdb/gnu-nat.c +++ b/gdb/gnu-nat.c @@ -1083,7 +1083,7 @@ inf_validate_procs (struct inf *inf) last = thread; proc_debug (thread, new thread: %d, threads[i]); - ptid = ptid_build (inf-pid, 0, thread-tid); + ptid = ptid_build (inf-pid, thread-tid, 0); /* Tell GDB's generic thread code. */ @@ -1613,17 +1613,17 @@ rewait: thread = inf-wait.thread; if (thread) -ptid = ptid_build (inf-pid, 0, thread-tid); +ptid = ptid_build (inf-pid, thread-tid, 0); else if (ptid_equal (ptid, minus_one_ptid)) thread = inf_tid_to_thread (inf, -1); else -thread = inf_tid_to_thread (inf, ptid_get_tid (ptid)); +thread = inf_tid_to_thread (inf, ptid_get_lwp (ptid)); if (!thread || thread-port == MACH_PORT_NULL) { /* TID is dead; try and find a new thread. */ if (inf_update_procs (inf) inf-threads) - ptid = ptid_build (inf-pid, 0, inf-threads-tid); /* The first + ptid = ptid_build (inf-pid, inf-threads-tid, 0); /* The first available thread. */ else @@ -2022,7 +2022,7 @@ gnu_resume (struct target_ops *ops, else /* Just allow a single thread to run. */ { - struct proc *thread = inf_tid_to_thread (inf, ptid_get_tid (ptid)); + struct proc *thread = inf_tid_to_thread (inf, ptid_get_lwp (ptid)); if (!thread) error (_(Can't run single thread id %s: no such thread!), @@ -2033,7 +2033,7 @@ gnu_resume (struct target_ops *ops, if (step) { - step_thread = inf_tid_to_thread (inf, ptid_get_tid (ptid)); + step_thread = inf_tid_to_thread (inf, ptid_get_lwp (ptid)); if (!step_thread) warning (_(Can't step thread id %s: no such thread.), target_pid_to_str (ptid)); @@ -2133,7 +2133,7 @@ gnu_create_inferior (struct target_ops *ops, /* We now have thread info. */ thread_change_ptid (inferior_ptid, - ptid_build (inf-pid, 0, inf_pick_first_thread ())); + ptid_build (inf-pid, inf_pick_first_thread (), 0)); startup_inferior (inf-pending_execs); @@ -2190,7 +2190,7 @@ gnu_attach (struct target_ops *ops, char *args, int from_tty) inf_update_procs (inf); - inferior_ptid = ptid_build (pid, 0, inf_pick_first_thread ()); + inferior_ptid = ptid_build (pid, inf_pick_first_thread (), 0); /* We have to initialize the terminal settings now, since the code below might try to restore them. */ @@ -2261,7 +2261,7 @@ gnu_thread_alive (struct target_ops *ops, ptid_t ptid) { inf_update_procs (gnu_current_inf); return !!inf_tid_to_thread (gnu_current_inf, - ptid_get_tid (ptid)); + ptid_get_lwp (ptid)); } @@ -2596,7 +2596,7 @@ static char * gnu_pid_to_str (struct target_ops *ops, ptid_t ptid) { struct inf *inf = gnu_current_inf; - int tid = ptid_get_tid (ptid); + int tid = ptid_get_lwp (ptid); struct proc *thread = inf_tid_to_thread (inf, tid); if (thread) @@ -2729,7 +2729,7 @@ cur_thread (void) { struct inf *inf = cur_inf (); struct proc *thread = inf_tid_to_thread (inf, - ptid_get_tid (inferior_ptid
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
On 09/18/2013 02:48 PM, Yue Lu wrote: (btw, with unknown reason, I can't patch your patch automatically, I have to modify the gnu-nat.c line by line according to your patch). I'm going to guess you copy/pasted the patch to a new file, and while doing that, something (your editor or mailer?) line-wrapped the patch. I just now tried saving the whole email as an mbox file, and then applied it to a pristine tree with git am, and saw no issues. -- Pedro Alves
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
Hi! On 09/05/2013 08:29 PM, Pedro Alves wrote: +static int +gnu_read_memory (CORE_ADDR addr, unsigned char *myaddr, int length) +{ + int ret = 0; + task_t task = (gnu_current_inf + ? (gnu_current_inf-task +? gnu_current_inf-task-port : 0) : 0); + if (task == MACH_PORT_NULL) +return 0; + ret = gnu_read_inferior (task, addr, myaddr, length); + if (length != ret) +{ + gnu_debug (gnu_read_inferior,length=%d, but return %d\n, length, ret); + return -1; +} + return 0; +} + +static int +gnu_write_memory (CORE_ADDR addr, const unsigned char *myaddr, int length) +{ + int ret = 0; + task_t task = (gnu_current_inf + ? (gnu_current_inf-task +? gnu_current_inf-task-port : 0) : 0); + if (task == MACH_PORT_NULL) +return 0; + ret = gnu_write_inferior (task, addr, myaddr, length); + if (length != ret) +{ + gnu_debug (gnu_write_inferior,length=%d, but return %d\n, length, + ret); + return -1; +} + return 0; +} + These should reall be wrappers around a common shared function. I have a patch that I think helps here. I'll post it in soon. This is what I meant: https://sourceware.org/ml/gdb-patches/2013-09/msg00253.html I was thinking you'd wrap gnu_xfer_memory. But I have to say I don't really understand the real need for all those: task_t task = (gnu_current_inf ? (gnu_current_inf-task ? gnu_current_inf-task-port : 0) : 0); int res; if (task == MACH_PORT_NULL) return 0; checks in the existing code. I mean, why would we reach here with an invalid inferior/task/port selected? It just reads as workaround for some bug to me. Thanks, -- Pedro Alves
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
On 09/05/2013 11:53 AM, Yue Lu wrote: Hi, This is the my new patch. Thanks. Follows a few comments, by no means an in depth review. We'll probably need to iterate a few times. I'm counting on Thomas and others to help as well! I'm actually very excited to see gdb and gdbserver sharing a single target backend, even if we still have many wrinkles in the interfaces to iron out! It does looks like this way results in lots of less work, and makes me believe we can port other targets to gdbserver already without unsurmountable effort. It'd be very useful if you send and maintain along with the patch the rationale for all design divergences between the ports you found necessary. E.g., it seems that gdbserver's task/proc bookkeeping is different from gdb's. Why? I have put some soft link in directory gdbserver like *.defs which point to [gdb]/gdb/*.defs. We'll need to adjust the Makefile etc. instead to use the original files instead of that. No symlinks please. 2013-09-03 Yue Lu hacklu.newb...@gmail.com gdb * configure.tgt: Set build_gdbserver=yes for GNU/Hurd hosts. * configure: Regenerate. * config/i386/i386gnu.mh: Add #ifdef to determine which rule to use in gdbserver. * i386gnu-nat.c: Add macor GDBSERVER to support build gdbserver. * gnu-nat.h: Add macor GDBSERVER to support build gdbserver. * gnu-nat.c: Add macor GDBSERVER to support build gdbserver. (gnu_debug): New function, use for debug printf. (gnu_wait_1): wait for inferior used in gdbserver. (gnu_resume_1): resume inferior used in gdbserver. (gnu_kill): New function, used in gdbserver. (gnu_mourn): New function, clean up after inferior dies, used in gdbserver. (gnu_create_inferior): New function, use for create inferior in gdbserver. (gnu_attach): New function, a placeholder. (gnu_detach): New function, used in gdbserver detach from inferior. (gnu_thread_alive): New function, call find_thread_ptid(). (gnu_ptid_build): New function, build structure ptid_t from pid, and tid. (gnu_get_tid): New function, return lwp from structure ptid_t, this is different with gdb's behavior. (gnu_add_process): New function, add a new process to process list. (gnu_join): New function, a placeholder. (gnu_resume): New function, a wrap function, just call gnu_resume_1(). (gnu_wait): New function, a wrap function, just call gnu_wait_1(). (gnu_fetch_registers_wrap): New function, a wrap function, just call gnu_fetch_registers(). (gnu_store_registers_wrap): New function, a wrap function, just call gnu_store_registers(). (gnu_read_memory): New function, a wrap function, just call gnu_read_inferior(). (gnu_write_memory): New function, a wrap function, just call gnu_write_inferior(). (gnu_request_interrupt): New function, a placeholder. (store_waitstatus): New function, helper function, copy from [gdb]/gdb/inf-child.c. (initialize_low): New function, use for initialize gdbserver's target_ops. (initialize_low_arch): New function, used by initialize_low(). (_initialize_gnu_nat): New function, used by initialize_low(). diff --git a/gdb/config/i386/i386gnu.mh b/gdb/config/i386/i386gnu.mh index a3ea122..224cf0f 100644 --- a/gdb/config/i386/i386gnu.mh +++ b/gdb/config/i386/i386gnu.mh @@ -1,4 +1,5 @@ # Host: Intel 386 running the GNU Hurd +ifndef GDBSERVER NATDEPFILES= i386gnu-nat.o gnu-nat.o core-regset.o fork-child.o \ notify_S.o process_reply_S.o msg_reply_S.o \ msg_U.o exc_request_U.o exc_request_S.o @@ -12,6 +13,10 @@ XM_CLIBS = -lshouldbeinlibc # Use our own user stubs for the msg rpcs, so we can make them time out, in # case the program is fucked, or we guess the wrong signal thread. msg-MIGUFLAGS = -D'MSG_IMPORTS=waittime 1000;' +else +NATDEPFILES= notify_S.o process_reply_S.o msg_reply_S.o \ + exc_request_U.o exc_request_S.o +endif # ick MIGCOM = $(MIG) -cc cat - /dev/null diff --git a/gdb/configure.tgt b/gdb/configure.tgt index b0bee47..3eb2ff7 100644 --- a/gdb/configure.tgt +++ b/gdb/configure.tgt @@ -231,6 +231,7 @@ i[34567]86-*-linux*) i[34567]86-*-gnu*) # Target: Intel 386 running the GNU Hurd gdb_target_obs=i386-tdep.o i387-tdep.o i386gnu-tdep.o solib-svr4.o + build_gdbserver=yes ;; i[34567]86-*-cygwin*) # Target: Intel 386 running win32 diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c index 59d2f23..1cdcd1d 100644 --- a/gdb/gnu-nat.c +++ b/gdb/gnu-nat.c @@ -1,6 +1,7 @@ /* Interface GDB to the GNU Hurd. Copyright (C) 1992-2013 Free Software Foundation, Inc. + This file is part of GDB. Please go through the patch and remove spurious whitespace changes line these. Written by Miles Bader mi...@gnu.ai.mit.edu @@ -20,6 +21,7 @@ You should have received a copy of the GNU General
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
converge further, and the #ifdefs disappear, but for now that's a necessary evil. It's fine to leave bits of functionality disabled on gdbserver, wrapped in #ifndef GDBSERVER. After that initial work is committed, we can then easily progress the gdbserver port by just looking for those #ifdefs. It's fine with me to leave the existing native files under gdb/ while doing this; it's probably easier that way. Moving them under gdb/nat/ can be left for a cleanup step further down the road. Could we try that approach? -- Pedro Alves
Re: [patch] for mig check in GDB's configure
On 05/03/2013 09:28 AM, Thomas Schwinge wrote: Hmm, I think that instead of only examining the host system, $host, this also needs to examine the target system, $target. (Please tell if the difference between build, host, and target system is not clear to you.) The MIG tool is used to generate files (from RPC definition files) that are used by the native GDB port for GNU Hurd (which, of couse, is the only GNU Hurd port that currently exists.) But if someone, for example, builds GDB targeting mips-linux-gnu on a GNU Hurd system, they would not need the MIG tool. GDB folks, would it make sense to use something like: case $gdb_native:$host in [...] yes:i[[3456]]86-*-gnu*) [error if MIG not found] ..., to check that both host and target are GNU Hurd? Sure. Looking at config/i386/i386gnu.mh, NAT_GENERATED_FILES in particular, I think gdb will want to run mig even on cross gdbs hosted on GNU Hurd, though... -- Pedro Alves
Re: [PATCH,HURD] hurd: compliance fixes for getgroups
On 04/28/2012 12:21 PM, Pino Toscano wrote: Alle venerdì 27 aprile 2012, Roland McGrath ha scritto: 2012-04-27 Pino Toscano toscano.p...@tiscali.it * sysdeps/mach/hurd/getgroups.c (__getgroups): Return -1 and set EINVAL for negative `n' or less than `ngids'. The norm is to use all caps and no quotes to mention local variable names. You can also drop the (function) when it's the only function in the file of the same name. Ah ok, fixed patch attached. Where can I read about these two change log style settings (i.e. variables as all caps, and no (function) when it's the only one in a file)? I don't remember them in the official pages[1], so maybe I missed some documentation... [1] http://www.gnu.org/prep/standards/html_node/Change-Logs.html Pedantically, the norm is to use all caps when talking about the _value_ of a variable, as opposed to the variable itself, as in: less than NGIDS. instead of less than the value of the `ngids' variable. See http://www.gnu.org/prep/standards/standards.html#Comments : The comment on a function is much clearer if you use the argument names to speak about the argument values. The variable name itself should be lower case, but write it in upper case when you are speaking about the value rather than the variable itself. Thus, “the inode number NODE_NUM” rather than “an inode”. -- Pedro Alves
Re: Restore GNU/Hurd functionality (was: Modernize solaris threads support.)
On Monday 20 July 2009 10:55:21, Thomas Schwinge wrote: 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.0 +0100 +++ src/gdb/gnu-nat.c 2009-07-20 11:21:46.0 +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
Re: Restore GNU/Hurd functionality
On Monday 20 July 2009 15:53:34, Thomas Schwinge wrote: 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
Re: [PATCH?] GDB HEAD (partly) broken for GNU/Hurd
Hi, and sorry I didn't reply back sooner. On Monday 13 October 2008 17:40:25, Ulrich Weigand wrote: Thomas Schwinge wrote: Ha, I, myself, am the GDB guru here ;-)! I had a look at the log again, experimented some more, and finally got it going with the following patch. However, I have absolutely no idea whether that is correct in all cases, etc. Should perhaps target_wait (a.k.a. gnu-nat.c's gnu_wait) be doing that? Thanks Thomas :-) One thing I asked myself was, if gnu-nat.c couldn't be using the port's id as thread ids instead of a locally auto-generated number. Maybe the thread id of the main thread would be preserved across execs this way, but, this is off-scope for now. Adding switch_to_thread at this location seems correct to me (even though it should be a no-op on most targets). You shouldn't call it on a few cases, namely: - TARGET_WAITKIND_IGNORE, TARGET_WAITKIND_SIGNALLED and TARGET_WAITKIND_EXITED. The ptid returned isn't a thread, so you could hit an internal error inside e.g., switch_to_thread-is_exited,is_running. The _IGNORE case is actually broken today, as we shouldn't issue either a target_resume or set_executing (..., 0) in that case. Sorry I missed it before. Here's the similar handling in handle_inferior_event for reference: if (ecs-ws.kind != TARGET_WAITKIND_IGNORE) { /* Mark the non-executing threads accordingly. */ if (!non_stop || ecs-ws.kind == TARGET_WAITKIND_EXITED || ecs-ws.kind == TARGET_WAITKIND_SIGNALLED) set_executing (pid_to_ptid (-1), 0); else set_executing (ecs-ptid, 0); } - TARGET_WAITKIND_SPURIOUS. This one's a bit tricky, as some targets may return ptid(-1,0,0) with it. It looked like procfs.c can in some cases last time I tried looking at cleaning that up. handle_inferior_event doesn't switch context on it, so we could do the same. But, we'll most probably not see this event here, I hope. - If target_wait returns ptid(-1). It shouldn't be returning that in any case where we should call switch_to_thread, though. We can see this happening in the the TARGET_WAITKIND_IGNORE case --- don't switch threads in this case anyway. - calling switch_to_thread before calling set_executing (..., 0), has the effect of not reading stop_pc. I guess that's what we really want here? Thus we avoid touching the shell's registers until we return from startup_inferior, which I undertand was one of the goals of this new loop. Notice that we're assuming that handle_inferior_event()'s new_thread_event handling isn't needed here in any platform. Could you test your patch on a non-Hurd target to make sure, anyway? + /* TODO. How to keep this synchronized with gnu-nat.c's own counting? */ Hmm. It would appear that set exec-wrapper is currently broken with the gnu-nat.c target, right? Yeah, it appears so. Don't know if it's possible to get rid of the local pending execs handling in gnu-nat.c. An alternative would be to make pending_execs a property of inferior.h:`struct inferior' instead of of gnu-nat.c:`struct inf'. (I also notice that when going through the shell in non-stop mode, it would be more correct to resume all threads --- we don't want non-stop and its scheduler-locking to apply to the shell. Basically, non-stop should be off if there are pending execs. This was an existing issue, and doesn't affect linux today, so I'll just ignore that for now, as it needs more tweaking to fix.) ( hope not many issues like this appear, as we're doing more duplication of handle_inferior_event logic :-( ) What do you guys think? Thomas, could you try the attached patch on the Hurd, please? I just gave it a spin on x86-64-unknown-linux-gnu without regressions. -- Pedro Alves 2008-10-13 Pedro Alves [EMAIL PROTECTED] * fork-child.c (startup_inferior): Only set threads not-executing after getting all the pending execs. On TARGET_WAITKIND_IGNORE, keep waiting, don't resume. On all other cases but TARGET_WAITKIND_SIGNALLED and TARGET_WAITKIND_EXITED, switch to the event ptid. --- gdb/fork-child.c | 30 ++ 1 file changed, 22 insertions(+), 8 deletions(-) Index: src/gdb/fork-child.c === --- src.orig/gdb/fork-child.c 2008-10-13 18:34:10.0 +0100 +++ src/gdb/fork-child.c 2008-10-13 19:12:03.0 +0100 @@ -434,21 +434,18 @@ startup_inferior (int ntraps) { int resume_signal = TARGET_SIGNAL_0; ptid_t resume_ptid; + ptid_t event_ptid; struct target_waitstatus ws; memset (ws, 0, sizeof (ws)); - resume_ptid = target_wait (pid_to_ptid (-1), ws); + event_ptid = target_wait (pid_to_ptid (-1), ws); - /* Mark all threads non-executing. */ - set_executing (pid_to_ptid (-1), 0); - - /* In all-stop mode, resume all threads. */ - if (!non_stop) - resume_ptid = pid_to_ptid (-1
Re: GDB HEAD (partly) broken for GNU/Hurd
On Saturday 11 October 2008 18:26:11, Thomas Schwinge wrote: Doesn't resume the whole shell? But as I see things we always have ``non_stop == 0'' and thus ``resume_ptid = pid_to_ptid (-1)'', so that should be fine, isn't it? Duh! Yes, of course. Here is a debugging-enabled run of a thusly patched GDB HEAD: Thanks. STOPPED_BY_WATCHPOINT () = 0 infrun: context switch infrun: Switching context from bogus thread id 1 to Thread 25830.3 ^^ This is missing in the new implementation. In Hurd when going through the shell doing fork/execs, we see new threads at every exec, and the old threads disappear. Could you try adding a `switch_to_thread (resume_thread)' somewhere after target_wait returns and before any other target method? -- Pedro Alves
Re: GDB HEAD (partly) broken for GNU/Hurd
On Saturday 11 October 2008 00:27:06, Thomas Schwinge wrote: On HEAD, when undoing this change (and additionally commenting out the two ``stop_soon = X'' lines in that file), things are fine again. As most of GDB's internals are a big black box to me, I need help here. :-) Eh, I did point out at the time of that change that gnu-nat.c does things a bit different. :-) Off-hand advice: One thing that the hurd has a bit different, is that we have multi-threading when going through the shell. Could it be that target_wait is returning a specific ptid here: fork_child.c:startup_inferior: while (1) { int resume_signal = TARGET_SIGNAL_0; ptid_t resume_ptid; struct target_waitstatus ws; memset (ws, 0, sizeof (ws)); resume_ptid = target_wait (pid_to_ptid (-1), ws); ^^^ Hence this a bit below: if (--pending_execs == 0) break; /* Just make it go on. */ target_resume (resume_ptid, 0, TARGET_SIGNAL_0); ^^^ } } Doesn't resume the whole shell? If you make this change: - target_resume (resume_ptid, 0, TARGET_SIGNAL_0); + target_resume (minus_one_ptid, 0, TARGET_SIGNAL_0); The other thing I suggest to look at, is to make sure the local `pending_execs' and the `gnu-nat.c:struct inf'::pending_execs aren't in conflict, but it doesn't look like it. Hope this helps. -- Pedro Alves
Re: GDB HEAD (partly) broken for GNU/Hurd
A Thursday 09 October 2008 10:34:24, Thomas Schwinge escreveu: Hello! Some of the changes that have been installed between gdb_6_8-branch and HEAD cause GDB to no longer function properly on GNU/Hurd under certain circumstances. [EMAIL PROTECTED]:~/tmp/gdb/HEAD.build $ gdb/gdb ~/tmp/n1/hurd/ext2fs.static GNU gdb 6.8.0.20081008-cvs [...] This GDB was configured as i386-unknown-gnu0.3... (gdb) r Starting program: /media/data/home/tschwinge/tmp/n1/hurd/ext2fs.static [New thread 8112.1] [New thread 8112.2] [New thread 8112.3] [New thread 8112.4] [New thread 8112.5] Program received signal SIGSEGV, Segmentation fault. convert_options (argp=0x813f0bc, parent=0x0, parent_index=0, group=0x81712e8, cvt=0x101fad0) at argp.h:579 579 argp.h: No such file or directory. in argp.h [EMAIL PROTECTED]:~/tmp/gdb/HEAD.build $ gdb/gdb ~/tmp/n1/hurd/ext2fs.static GNU gdb (GDB) 6.8.50.20081009-cvs [...] (gdb) r Starting program: /media/data/home/tschwinge/tmp/n1/hurd/ext2fs.static Can't fetch registers from thread bogus thread id 1: No such thread Both have been built (natively) on the same up-to-date Debian GNU/Hurd system. For easy reproduction, I can publish the faulting binary. Could you check what caused the breakage? It *may* have been the ptid changes I made, or not. 2008-09-08 Pedro Alves [EMAIL PROTECTED] Use ptid_t.tid to store thread ids instead of ptid_t.pid. * gnu-nat.c (inf_validate_procs): If this is the first time we're seeing a thread id, extend the main thread's ptid. If we still have pending execs, don't be verbose about new threads. (gnu_wait, gnu_resume, gnu_attach, gnu_thread_alive) (gnu_pid_to_str, cur_thread, sig_thread_cmd): Adjust. * i386gnu-nat.c (gnu_fetch_registers, gnu_store_registers): Adjust. If nothing obvious turns up, could you open up a bug report with a testcase and instructions, please? -- Pedro Alves