Fix backwards compatibility with old GDBservers (PR remote/22597) (Re: GDB 8.1 release -- 2018-01-08 update)
Commit Message
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?
Comments
On Mon, 8 Jan 2018, Pedro Alves wrote:
> Maciej, does this work for you / MIPS?
It does, with the same `gdbserver' binary I reported the failure against.
Thank you for your fix and sorry to mislead you several times about the
nature of this regression.
Maciej
From 120d9979a082b349590f64faa40dba0c04716646 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 9 Jan 2018 00:37:36 +0000
Subject: [PATCH] Fix backwards compatibility with old GDBservers (PR
remote/22597)
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
PR remote/22597
* gdb.server/stop-reply-no-thread.c: New file.
* gdb.server/stop-reply-no-thread.exp: New file.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
PR remote/22597
* remote.c (remote_parse_stop_reply): Default to the last-set
general thread instead of to 'magic_null_ptid'.
---
gdb/remote.c | 8 ++-
gdb/testsuite/gdb.server/stop-reply-no-thread.c | 22 +++++++
gdb/testsuite/gdb.server/stop-reply-no-thread.exp | 74 +++++++++++++++++++++++
3 files changed, 103 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.server/stop-reply-no-thread.c
create mode 100644 gdb/testsuite/gdb.server/stop-reply-no-thread.exp
@@ -6940,7 +6940,13 @@ Packet: '%s'\n"),
event->ptid = read_ptid (thr + strlen (";thread:"),
NULL);
else
- event->ptid = magic_null_ptid;
+ {
+ /* Either the current thread hasn't changed,
+ or the inferior is not multi-threaded.
+ The event must be for the thread we last
+ set as (or learned as being) current. */
+ event->ptid = event->rs->general_thread;
+ }
}
if (rsa == NULL)
new file mode 100644
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int
+main ()
+{
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,74 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2018 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test backward compatibility with older GDBservers which did not
+# include ";thread:NNN" in T stop replies when debugging
+# single-threaded programs, even though they'd list the main thread in
+# response to qfThreadInfo/qsThreadInfo. See PR remote/22597.
+
+load_lib gdbserver-support.exp
+
+if { [skip_gdbserver_tests] } {
+ verbose "skipping gdbserver tests"
+ return -1
+}
+
+standard_testfile
+if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
+ return -1
+}
+
+# Make sure we're disconnected, in case we're testing with an
+# extended-remote board, therefore already connected.
+gdb_test "disconnect" ".*"
+
+# Start GDBserver, with ";thread:NNN" in T stop replies disabled,
+# emulating old gdbservers when debugging single-threaded programs.
+set res [gdbserver_start "--disable-packet=Tthread" $binfile]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+
+# Disable XML-based thread listing, and multi-process extensions.
+gdb_test_no_output "set remote threads-packet off"
+gdb_test_no_output "set remote multiprocess-feature-packet off"
+
+set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
+if ![gdb_assert {$res == 0} "connect"] {
+ return
+}
+
+# There should be only one thread listed.
+set test "info threads"
+gdb_test_multiple $test $test {
+ -re "2 Thread.*$gdb_prompt $" {
+ fail $test
+ }
+ -re "has terminated.*$gdb_prompt $" {
+ fail $test
+ }
+ -re "\\\* 1\[\t \]*Thread\[^\r\n\]*\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+gdb_breakpoint "main"
+
+# Bad GDB behaved like this:
+# (gdb) c
+# Cannot execute this command without a live selected thread.
+# (gdb)
+gdb_test "c" "Breakpoint $decimal, main.*" "continue to main"
--
2.14.3