Skip to content
This repository was archived by the owner on Aug 17, 2022. It is now read-only.

Commit 936a312

Browse files
committed
Fix backwards compatibility with old GDBservers (PR remote/22597)
At <https://sourceware.org/ml/gdb-patches/2017-12/msg00285.html>, Maciej reported that commit: commit 5cd63fd Date: Wed Oct 4 18:21:10 2017 +0100 Subject: Fix "Remote 'g' packet reply is too long" problems with multiple inferiors made GDB stop working with older stubs. Any attempt to continue execution after the initial connection fails with: [...] 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) The 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 still GDB added a bogus second thread. The reason GDB started adding a second thread after 5cd63fd 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 added by this commit emulates old gdbserver making use of that. I've compared a testrun at 5cd63fd^ (before regression) with 'current master+patch', against old gdbserver at f8b73d1^. 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. Maciej confirmed this worked for him as well. No regressions on master (against master gdbserver). gdb/ChangeLog: 2018-01-11 Pedro Alves <[email protected]> PR remote/22597 * remote.c (remote_parse_stop_reply): Default to the last-set general thread instead of to 'magic_null_ptid'. gdb/testsuite/ChangeLog: 2018-01-11 Pedro Alves <[email protected]> PR remote/22597 * gdb.server/stop-reply-no-thread.c: New file. * gdb.server/stop-reply-no-thread.exp: New file.
1 parent c03862a commit 936a312

File tree

5 files changed

+115
-1
lines changed

5 files changed

+115
-1
lines changed

gdb/ChangeLog

+6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
2018-01-11 Pedro Alves <[email protected]>
2+
3+
PR remote/22597
4+
* remote.c (remote_parse_stop_reply): Default to the last-set
5+
general thread instead of to 'magic_null_ptid'.
6+
17
2018-01-10 Pedro Alves <[email protected]>
28

39
* language.h (language_get_symbol_name_matcher): Rename ...

gdb/remote.c

+7-1
Original file line numberDiff line numberDiff line change
@@ -6939,7 +6939,13 @@ Packet: '%s'\n"),
69396939
event->ptid = read_ptid (thr + strlen (";thread:"),
69406940
NULL);
69416941
else
6942-
event->ptid = magic_null_ptid;
6942+
{
6943+
/* Either the current thread hasn't changed,
6944+
or the inferior is not multi-threaded.
6945+
The event must be for the thread we last
6946+
set as (or learned as being) current. */
6947+
event->ptid = event->rs->general_thread;
6948+
}
69436949
}
69446950

69456951
if (rsa == NULL)

gdb/testsuite/ChangeLog

+6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
2018-01-11 Pedro Alves <[email protected]>
2+
3+
PR remote/22597
4+
* gdb.server/stop-reply-no-thread.c: New file.
5+
* gdb.server/stop-reply-no-thread.exp: New file.
6+
17
2018-01-10 Pedro Alves <[email protected]>
28

39
PR gdb/22670
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/* This testcase is part of GDB, the GNU debugger.
2+
3+
Copyright 2018 Free Software Foundation, Inc.
4+
5+
This program is free software; you can redistribute it and/or modify
6+
it under the terms of the GNU General Public License as published by
7+
the Free Software Foundation; either version 3 of the License, or
8+
(at your option) any later version.
9+
10+
This program is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
GNU General Public License for more details.
14+
15+
You should have received a copy of the GNU General Public License
16+
along with this program. If not, see <http://www.gnu.org/licenses/>. */
17+
18+
int
19+
main ()
20+
{
21+
return 0;
22+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# This testcase is part of GDB, the GNU debugger.
2+
#
3+
# Copyright 2018 Free Software Foundation, Inc.
4+
#
5+
# This program is free software; you can redistribute it and/or modify
6+
# it under the terms of the GNU General Public License as published by
7+
# the Free Software Foundation; either version 3 of the License, or
8+
# (at your option) any later version.
9+
#
10+
# This program is distributed in the hope that it will be useful,
11+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
# GNU General Public License for more details.
14+
#
15+
# You should have received a copy of the GNU General Public License
16+
# along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
18+
# Test backward compatibility with older GDBservers which did not
19+
# include ";thread:NNN" in T stop replies when debugging
20+
# single-threaded programs, even though they'd list the main thread in
21+
# response to qfThreadInfo/qsThreadInfo. See PR remote/22597.
22+
23+
load_lib gdbserver-support.exp
24+
25+
if { [skip_gdbserver_tests] } {
26+
verbose "skipping gdbserver tests"
27+
return -1
28+
}
29+
30+
standard_testfile
31+
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
32+
return -1
33+
}
34+
35+
# Make sure we're disconnected, in case we're testing with an
36+
# extended-remote board, therefore already connected.
37+
gdb_test "disconnect" ".*"
38+
39+
# Start GDBserver, with ";thread:NNN" in T stop replies disabled,
40+
# emulating old gdbservers when debugging single-threaded programs.
41+
set res [gdbserver_start "--disable-packet=Tthread" $binfile]
42+
set gdbserver_protocol [lindex $res 0]
43+
set gdbserver_gdbport [lindex $res 1]
44+
45+
# Disable XML-based thread listing, and multi-process extensions.
46+
gdb_test_no_output "set remote threads-packet off"
47+
gdb_test_no_output "set remote multiprocess-feature-packet off"
48+
49+
set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
50+
if ![gdb_assert {$res == 0} "connect"] {
51+
return
52+
}
53+
54+
# There should be only one thread listed.
55+
set test "info threads"
56+
gdb_test_multiple $test $test {
57+
-re "2 Thread.*$gdb_prompt $" {
58+
fail $test
59+
}
60+
-re "has terminated.*$gdb_prompt $" {
61+
fail $test
62+
}
63+
-re "\\\* 1\[\t \]*Thread\[^\r\n\]*\r\n$gdb_prompt $" {
64+
pass $test
65+
}
66+
}
67+
68+
gdb_breakpoint "main"
69+
70+
# Bad GDB behaved like this:
71+
# (gdb) c
72+
# Cannot execute this command without a live selected thread.
73+
# (gdb)
74+
gdb_test "c" "Breakpoint $decimal, main.*" "continue to main"

0 commit comments

Comments
 (0)