Re: [PATCH 3/3] Fix "Remote 'g' packet reply is too long" problems with multiple inferiors

["Maciej W. Rozycki" <macro@mips.com>]

On Tue, 12 Dec 2017, Pedro Alves wrote:

> > [...]
> > Process .../gdb/testsuite/outputs/gdb.base/advance/advance created; pid = 2670
> > Listening on port 2346
> > target remote [...]:2346
> > Remote debugging using [...]:2346
> > Reading symbols from .../lib64/ld.so.1...done.
> > [Switching to Thread <main>]
> > (gdb) continue
> > Cannot execute this command without a live selected thread.
> > (gdb) 
> > 
> > This is as from commit 5cd63fda035d.  Up to 5cd63fda035d^ instead I get:
> > 
> > [...]
> > Process .../gdb/testsuite/outputs/gdb.base/advance/advance created; pid = 30044
> > Listening on port 2346
> > target remote [...]:2346
> > Remote debugging using [...]:2346
> > Reading symbols from .../lib64/ld.so.1...done.
> > 0x000000fff79534f0 in __start () from .../lib64/ld.so.1
> > (gdb) continue
> > Continuing.
> > warning: Could not load shared library symbols for linux-vdso.so.1.
> > Do you need "set solib-search-path" or "set sysroot"?
> > 
> > Breakpoint 1, main () at .../gdb/testsuite/gdb.base/advance.c:41
> > 41	  c = 5;
> > (gdb) 
> > 
> > At the protocol level the difference starts here (bad):
> > 
> > Sending packet: $qfThreadInfo#bb...Ack
> > Packet received: m1814
> > Sending packet: $qsThreadInfo#c8...Ack
> > Packet received: l
> > [Switching to Thread <main>]
> > Sending packet: $qSymbol::#5b...Ack
> > Packet received: qSymbol:6e70746c5f76657273696f6e
> > Sending packet: $qSymbol::6e70746c5f76657273696f6e#4d...Ack
> > Packet received: OK
> > Sending packet: $Hg1814#7d...Ack
> > Packet received: OK
> > Sending packet: $Hg0#df...Ack
> > Packet received: E01
> > (gdb) continue
> > Cannot execute this command without a live selected thread.
> > (gdb)
> 
> So IIUC, the difference is that after this patch, we now start sending those
> two Hg / select-thread packets, and I assume that the second one throws,
> an error (given the E01) which leaves the thread state out of sync.

 Right, I somehow missed this error reply.  My first observation was that 
we don't issue these memory examination packets after the `l' response 
anymore (those are clearly used to report where the debuggee has stopped 
at connection).

> Strange, off hand I'm not seeing how this patch can affect/cause this, since
> it's mostly touching gdbarch / register bits.
> 
> OK, in the bugzilla report you mentioned 
> "Empty `qsThreadInfo' reply handling regression causing inability
> to execute".

 I didn't read the description of `qsThreadInfo' carefully enough in the 
manual and didn't notice it is related to `qfThreadInfo'.  Sorry about 
that.  The title of the bug will have to be changed according to further 
findings.

> That sequence:
> 
> > Sending packet: $qfThreadInfo#bb...Ack
> > Packet received: m1814
> > Sending packet: $qsThreadInfo#c8...Ack
> > Packet received: l
> 
> looks OK to me off hand, since "l" means there are no more
> threads (other than 1814).
> 
> Just to double-check; you're positive that this is the right
> commit to blame?  I'm wondering that because the remote.c thread
> listing bits were touched by other patches recently, and
> I'd suspect those changes first.

 I am positive, I tried several times and the failure is always the same 
from 5cd63fda035d onwards, whereas earlier revisions work just fine.

> > vs (good):
> > 
> > Sending packet: $qfThreadInfo#bb...Ack
> > Packet received: m154d
> > Sending packet: $qsThreadInfo#c8...Ack
> > Packet received: l
> > Sending packet: $mfff726c4f0,4#cb...Ack
> > Packet received: 03e0c825
> > Sending packet: $mfff726c4ec,4#fd...Ack
> > Packet received: 00000000
> > Sending packet: $mfff726c4f0,4#cb...Ack
> > Packet received: 03e0c825
> > 0x000000fff726c4f0 in __start () from .../lib64/ld.so.1
> > Sending packet: $qSymbol::#5b...Ack
> > Packet received: qSymbol:6e70746c5f76657273696f6e
> > Sending packet: $qSymbol::6e70746c5f76657273696f6e#4d...Ack
> > Packet received: OK
> > (gdb) continue
> > Continuing.
> > Sending packet: $mfff726c4f0,4#cb...Ack
> > Packet received: 03e0c825
> > Sending packet: $Z0,120000d6c,4#36...Ack
> > Packet received:
> > Packet Z0 (software-breakpoint) is NOT supported
> > [...]
> 
> So here we didn't used to see any Hg.

 Correct indeed.  Now that you mention it I wonder if there actually was a 
`gdbserver' issue with `Hg0' handling in f8b73d13b7ca^, hmm...  Let's 
check:

	    case 'H':
	      if (own_buf[1] == 'c' || own_buf[1] == 'g' || own_buf[1] == 's')
		{
		  unsigned long gdb_id, thread_id;

		  gdb_id = strtoul (&own_buf[2], NULL, 16);
		  thread_id = gdb_id_to_thread_id (gdb_id);
		  if (thread_id == 0)
		    {
		      write_enn (own_buf);
		      break;
		    }

-- so `Hg0' actually wasn't yet supported back then.  AFAICT it was only 
your commit bd99dc858385 ("Non-stop mode support.") that added this 
special exception.

 So I think we need to handle such a situation somehow, and given that 
`Hg1814' immediately precedes I think that `Hg0' packet can be safely 
discarded from the sequence of requests issued (unless it is meant to 
verify `0' thread ID support, that is).

> > The version of `gdbserver' causing this regression is as at commit 
> > f8b73d13b7ca^, the last MIPS backend version with no XML support.  It's 
> > likely that later versions hit this regression too, as the mode of failure 
> > is clearly not XML-related.
> 
> Note: with current master, you can disable XML support with 
> "set remote target-features-packet off" before connecting.
> Up until recently (7cc244debb58) that command didn't really work.
> So maybe it's possible to use current master gdbserver
> as proxy for emulating the old gdbserver.

 Thanks for the hint, I'll try that as well.

 So far I started regression-testing with 5cd63fda035d^ as the change in 
question is contained with the MIPS backend and there have been no 
significant recent updates there.  This works reasonably well with the 
f8b73d13b7ca^ `gdbserver', except I had to patch `--once' out in the 
harness as not supported back then and execution is slow due to the lack 
of `monitor exit' command support.

 Anyway I think we should strive to continue supporting old stubs as 
people may be stuck with them for some reason (and need current GDB for 
another).

  Maciej