On 01/08/2018 04:34 PM, Pedro Alves wrote:
> On 01/08/2018 09:54 AM, Maciej W. Rozycki wrote:
>>
>>  GDB uses the special thread ID 0, standing for `any', which older 
>> `gdbserver' versions do not recognise.  It does not verify beforehand 
>> whether `gdbserver' supports this request and does not handle an error 
>> reply gracefully.  Consequently an error reply to a `Hg0' packet issued 
>> causes GDB to lose track of what is going on, making it impossible to 
>> continue with the debug session.  This happens with all sessions in the 
>> initial connection handshake, making the combination of new GDB and old 
>> `gdbserver' unusable.
> 
> I'm looking at this.  I can reproduce it on x86-64 using a gdbserver
> from 2007 (git hash "f8b73d13b7ca^", the same revision Maciej's
> gdbserver is built from).  I confirm that 5cd63fda035d somehow
> introduces the regression.  No idea why yet.
> 
> So not specific to MIPS.

So the "E01"s in response to Hg0 aren't really what causes things
to get out of sync.  GDB doesn't actually throw an error in response
to those "E01"'s.

The real problem is:

 (gdb) c
 Cannot execute this command without a live selected thread.
 (gdb) info threads 
   Id   Target Id         Frame 
   1    Thread 14917      0x00007f341cd98ed0 in _start () from /lib64/ld-linux-x86-64.so.2

 The current thread <Thread ID 2> has terminated.  See `help thread'.
                     ^^^^^^^^^^^
 (gdb) 

Note, thread _2_.  There's really only one thread in
the inferior (it's still at the entry point!), but somehow
GDB added a second thread.  It's because GDB thinks there's
more than one thread that we start seeing the Hg (select
general thread) packets.

The reason GDB started adding a second thread after 5cd63fda035d
is this hunk:

+                 if (event->ptid == null_ptid)
+                   {
+                     const char *thr = strstr (p1 + 1, ";thread:");
+                     if (thr != NULL)
+                       event->ptid = read_ptid (thr + strlen (";thread:"),
+                                                NULL);
+                     else
+                       event->ptid = magic_null_ptid;
+                   }

Note the else branch that falls back to magic_null_ptid.  We reach
that when we process the initial stop reply sent back in response
to the the "?" (status) packet early in the connection setup:

 Sending packet: $?#3f...Ack
 Packet received: T0506:0000000000000000;07:40a510f4fd7f0000;10:d0fe1201577f0000;

And note that that response does not include a ";thread:XXX" part.

This stop reply is processed after listing threads with
qfThreadInfo / qsThreadInfo :

 Sending packet: $qfThreadInfo#bb...Ack
 Packet received: m3915
 Sending packet: $qsThreadInfo#c8...Ack
 Packet received: l

meaning, when we process that stop reply, we treat the event as coming
from a thread with ptid == magic_null_ptid, which is not yet in the
thread list, so we add it then:

(top-gdb) p ptid
$1 = {m_pid = 42000, m_lwp = -1, m_tid = 1}
(top-gdb) bt
#0  0x0000000000840a8c in add_thread_silent(ptid_t) (ptid=...) at src/gdb/thread.c:269
#1  0x00000000007ad61d in remote_add_thread(ptid_t, int, int) (ptid=..., running=0, executing=0)
    at src/gdb/remote.c:1838
#2  0x00000000007ad8de in remote_notice_new_inferior(ptid_t, int) (currthread=..., executing=0)
    at src/gdb/remote.c:1921
#3  0x00000000007b758b in process_stop_reply(stop_reply*, target_waitstatus*) (stop_reply=0x1158860, status=0x7fffffffcc00)
    at src/gdb/remote.c:7217
#4  0x00000000007b7a38 in remote_wait_as(ptid_t, target_waitstatus*, int) (ptid=..., status=0x7fffffffcc00, options=0)
    at src/gdb/remote.c:7380
#5  0x00000000007b7cd1 in remote_wait(target_ops*, ptid_t, target_waitstatus*, int) (ops=0x102fac0 <remote_ops>, ptid=..., status=0x7fffffffcc00, options=0) at src/gdb/remote.c:7446
#6  0x000000000081587b in delegate_wait(target_ops*, ptid_t, target_waitstatus*, int) (self=0x102fac0 <remote_ops>, arg1=..., arg2=0x7fffffffcc00, arg3=0) at src/gdb/target-delegates.c:138
#7  0x0000000000827d77 in target_wait(ptid_t, target_waitstatus*, int) (ptid=..., status=0x7fffffffcc00, options=0)
    at src/gdb/target.c:2179
#8  0x0000000000715fda in do_target_wait(ptid_t, target_waitstatus*, int) (ptid=..., status=0x7fffffffcc00, options=0)
    at src/gdb/infrun.c:3589
#9  0x0000000000716351 in wait_for_inferior() () at src/gdb/infrun.c:3707
#10 0x0000000000715435 in start_remote(int) (from_tty=1) at src/gdb/infrun.c:3212

things go downhill from this.

We don't see the problem with current master gdbserver, because that version
always sends the ";thread:" part in the initial stop reply:

 Sending packet: $?#3f...Packet received: T0506:0000000000000000;07:a0d4ffffff7f0000;10:d05eddf7ff7f0000;thread:p3cea.3cea;core:3;

Years ago I had added a "--disable-packet=" command line
option to gdbserver which comes in handy for testing this,
since the existing "--disable-packet=Tthread" precisely makes gdbserver not
send that ";thread:" part in stop replies.  The testcase in the attached
patch emulates old gdbserver making use of that.

I've compared a testrun at 5cd63fda035d^ (before regression) with
'current master+patch', against old gdbserver at f8b73d13b7ca^.  I hacked
out --once, and "monitor exit" to be able to test.  The results are a bit
too unstable to tell accurately, but it looked like there were no
regressions.

No regressions on master (against master gdbserver).

Maciej, does this work for you / MIPS?