Re: [PATCH 1/2] Port gdbserver to GNU/Hurd

[Yue Lu <hacklu.newborn@gmail.com>]

Hi,

On Wed, Sep 18, 2013 at 8:11 PM, Pedro Alves <palves@redhat.com> wrote:
> On 09/09/2013 10:58 AM, Thomas Schwinge wrote:
>> Hi!
>>
>> On Sun, 8 Sep 2013 21:35:05 +0800, Yue Lu <hacklu.newborn@gmail.com> wrote:
>>> On Fri, Sep 6, 2013 at 5:37 AM, Thomas Schwinge <thomas@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.

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 (陆岳)