Re: Using mach_convert_device_to_port() in the device pager
On Wed, Sep 18, 2013 at 07:38:54AM +0200, Marin Ramesa wrote: > This is more a question than a patch. > > Why don't the device pager hash functions use the device server routines to > track > the device associated ports? > > If the the devices and associated ports are not in one-to-one correspodence, > why do > hash functions take ports as arguments, why not simply devices? > > Wouldn't something like this be better (please don't apply this, it probably > breaks > something): If I'm right, the "HACK" mentioned in those source lines you replace refer to an optimization used by the Mach kernel which consists of using object addresses as kernel port names to skip looking up the object port. So actually, these hashing functions take both a port name or a device as argument. -- Richard Braun
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 wrote:
>> On Fri, Sep 6, 2013 at 5:37 AM, Thomas Schwinge
>> 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,
> . 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
* 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, pt
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 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
Re: [PATCH 1/2] Port gdbserver to GNU/Hurd
Hi,
On Wed, Sep 18, 2013 at 8:11 PM, Pedro Alves wrote:
> On 09/09/2013 10:58 AM, Thomas Schwinge wrote:
>> Hi!
>>
>> On Sun, 8 Sep 2013 21:35:05 +0800, Yue Lu wrote:
>>> On Fri, Sep 6, 2013 at 5:37 AM, Thomas Schwinge
>>> 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,
>> . 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.
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". (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).
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.
Add this everything is ok.
diff --git a/gdb/i386gnu-nat.c b/gdb/i386gnu-nat.c
index 0fd8d91..2b93fee 100644
--- a/gdb/i386gnu-nat.c
+++ b/gdb/i386gnu-nat.c
@@ -132,7 +132,7 @@ gnu_fetch_registers (struct target_ops *ops,
inf_update_procs (gnu_current_inf);
thread = inf_tid_to_thread (gnu_current_inf,
- ptid_get_tid (inferior_ptid));
+ ptid_get_lwp (inferior_ptid));
if (!thread)
error (_("Can't fetch registers from thread %s: No such thread"),
target_pid_to_str (inferior_ptid));
@@ -225,7 +225,7 @@ gnu_store_registers (struct target_ops *ops,
inf_update_procs (gnu_current_inf);
thread = inf_tid_to_thread (gnu_current_inf,
- ptid_get_tid (inferior_ptid));
+ ptid_get_lwp (inferior_ptid));
if (!thread)
error (_("Couldn't store registers into thread %s: No such thread"),
target_pid_to_str (inferior_ptid));
--
Yue Lu (陆岳)
[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 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
Yue Lu
* 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
