diff options
author | Pedro Alves <palves@redhat.com> | 2018-01-11 00:23:04 +0000 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2018-01-11 00:23:04 +0000 |
commit | 3cada74087687907311b52781354ff551e10a0ed (patch) | |
tree | a40aaa7d40a4f622e838940c00c7f187bf28e2da | |
parent | 888a89da7fa5d219695234c3a8dc7b8a57dfe8ee (diff) | |
download | fsf-binutils-gdb-3cada74087687907311b52781354ff551e10a0ed.zip fsf-binutils-gdb-3cada74087687907311b52781354ff551e10a0ed.tar.gz fsf-binutils-gdb-3cada74087687907311b52781354ff551e10a0ed.tar.bz2 |
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 5cd63fda035d4ba949e6478406162c4673b3c9ef
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 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 added by this commit
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. Maciej confirmed this worked for him as well.
No regressions on master (against master gdbserver).
gdb/ChangeLog:
2018-01-11 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/testsuite/ChangeLog:
2018-01-11 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.
-rw-r--r-- | gdb/ChangeLog | 6 | ||||
-rw-r--r-- | gdb/remote.c | 8 | ||||
-rw-r--r-- | gdb/testsuite/ChangeLog | 6 | ||||
-rw-r--r-- | gdb/testsuite/gdb.server/stop-reply-no-thread.c | 22 | ||||
-rw-r--r-- | gdb/testsuite/gdb.server/stop-reply-no-thread.exp | 74 |
5 files changed, 115 insertions, 1 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 17f7c8b..4c629e5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2018-01-11 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'. + 2018-01-10 Pedro Alves <palves@redhat.com> * language.h (language_get_symbol_name_matcher): Rename ... diff --git a/gdb/remote.c b/gdb/remote.c index 81c772a..a1cd9ae 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -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) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 9a3d0f1..d95b144 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2018-01-11 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. + 2018-01-10 Pedro Alves <palves@redhat.com> PR gdb/22670 diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread.c b/gdb/testsuite/gdb.server/stop-reply-no-thread.c new file mode 100644 index 0000000..a9058de --- /dev/null +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread.c @@ -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; +} diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread.exp b/gdb/testsuite/gdb.server/stop-reply-no-thread.exp new file mode 100644 index 0000000..2bb8c7a --- /dev/null +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread.exp @@ -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" |