From patchwork Tue Jan 9 01:28:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 25281 Received: (qmail 120570 invoked by alias); 9 Jan 2018 01:28:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 120557 invoked by uid 89); 9 Jan 2018 01:28:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=replies, sends, thr, disconnected X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 09 Jan 2018 01:28:08 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 80235C04AC54; Tue, 9 Jan 2018 01:28:07 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 790F45D6A2; Tue, 9 Jan 2018 01:28:05 +0000 (UTC) Subject: [PATCH] Fix backwards compatibility with old GDBservers (PR remote/22597) (Re: GDB 8.1 release -- 2018-01-08 update) To: "Maciej W. Rozycki" , Joel Brobecker References: <20180108074937.fq44jr4qkdphgeew@adacore.com> <77539a17-7418-86ac-09b0-bb68650a610d@redhat.com> Cc: gdb-patches@sourceware.org, "Maciej W. Rozycki" , tom@tromey.com, simon.marchi@ericsson.com From: Pedro Alves Message-ID: Date: Tue, 9 Jan 2018 01:28:04 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <77539a17-7418-86ac-09b0-bb68650a610d@redhat.com> 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 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 , 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 , 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? From 120d9979a082b349590f64faa40dba0c04716646 Mon Sep 17 00:00:00 2001 From: Pedro Alves 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 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 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 diff --git a/gdb/remote.c b/gdb/remote.c index 81c772a5451..a1cd9ae1df3 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/gdb.server/stop-reply-no-thread.c b/gdb/testsuite/gdb.server/stop-reply-no-thread.c new file mode 100644 index 00000000000..a9058de0483 --- /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 . */ + +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 00000000000..2bb8c7ac7c5 --- /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 . + +# 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