From patchwork Thu Feb 27 16:17:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 38338 Received: (qmail 11238 invoked by alias); 27 Feb 2020 16:17:11 -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 11226 invoked by uid 89); 27 Feb 2020 16:17:11 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=attaching, how's, HX-Languages-Length:3156, hows X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Feb 2020 16:17:09 +0000 Received: by mail-wr1-f68.google.com with SMTP id p18so4090428wre.9 for ; Thu, 27 Feb 2020 08:17:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=lXQx49Zb5dmVTkeDN9B3OwjHn0P74Dl9armM2KLk1xE=; b=bj42Wp45juiXr7m7glz6myCWFX1HBuFK2TFnwnJ6zeR64pSsae+m1tPtbDYP5MjY4b bfWg6NoTXe56kFxbI+oy0LiTl8iJYo1y66WU86+bSfIb1eq3aFy1oWD9qCnI64F0zD/J KMR8ioufy21tTVqdexFKObvydXUM3fV0h4OlUt59jOThRvVKPVz93fhs7VgmPKgspGfD PNQMW10XKu/QmvcvfxnRiDecKsa52AIRuXVeog3QL7rwYxzR1m/eoHb7lb1T3KmRFfKc nytoCxaLwf0QHeLZnJ9zoplZOymyxP04OfhpGiYztbY+deHbDWKN+G0NDHYaNUfJN8Mr M+5g== Return-Path: Received: from localhost (host86-186-80-160.range86-186.btcentralplus.com. [86.186.80.160]) by smtp.gmail.com with ESMTPSA id d7sm8758582wmc.6.2020.02.27.08.17.06 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 27 Feb 2020 08:17:06 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Pedro Alves , Andrew Burgess Subject: [PATCHv2] gdb/remote: Restore support for 'S' stop reply packet Date: Thu, 27 Feb 2020 16:17:04 +0000 Message-Id: <20200227161704.16480-1-andrew.burgess@embecosm.com> In-Reply-To: <80314590-ba73-26f5-830d-698ff4ecb2d8@redhat.com> References: <80314590-ba73-26f5-830d-698ff4ecb2d8@redhat.com> X-IsSubscribed: yes Pedro, Thanks for your feedback. I agree with all your points, this version of the patch: 1. Doesn't query the target for the current thread, but just uses the first non-exited thread. 2. Gives a warning if the target has more than one non-exited thread. 3. Commit message is revised to reflect new implementation. How's this? Thanks, Andrew --- With this commit: commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 Date: Fri Jan 10 20:06:08 2020 +0000 Multi-target support There was a regression in GDB's support for older aspects of the remote protocol. Specifically, when a target sends the 'S' stop reply packet, which doesn't include a thread-id, then GDB has to figure out which thread actually stopped. Before the above commit GDB figured this out by using inferior_ptid in process_stop_reply, which contained the ptid of the current process/thread. With the above commit the inferior_ptid now has the value null_ptid inside process_stop_reply, this can be seen in do_target_wait, where we call switch_to_inferior_no_thread before calling do_target_wait_1. The solution I propose in this commit is to use the first non exited thread of the inferior. Additionally, GDB will give a warning if a multi-threaded target stops without passing a thread-id. There's no test included with this commit, if anyone has any ideas for how we could test this aspect of the remote protocol (short of writing a new gdbserver) then I'd love to know. It is possible to trigger this bug by attaching GDB to a running GDB, place a breakpoint on remote_parse_stop_reply, and manually change the contents of buf - when we get a 'T' based stop packet, replace it with an 'S' based packet, like this: (gdb) call memset (buf, "S05\0", 4) After this the GDB that is performing the remote debugging will crash with this error: inferior.c:279: internal-error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed. gdb/ChangeLog: * remote.c (remote_target::process_stop_reply): Use the first non-exited thread if the target didn't pass a thread-id. --- gdb/ChangeLog | 5 +++++ gdb/remote.c | 18 +++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 4a70ab3fb0d..4ac359ad5d9 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -7668,10 +7668,22 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply, *status = stop_reply->ws; ptid = stop_reply->ptid; - /* If no thread/process was reported by the stub, assume the current - inferior. */ + /* If no thread/process was reported by the stub then use the first + non-exited thread in the current inferior. */ if (ptid == null_ptid) - ptid = inferior_ptid; + { + for (thread_info *thr : all_non_exited_threads (this)) + { + if (ptid != null_ptid) + { + warning (_("multi-threaded target stopped without sending " + "a thread-id, using first non-exited thread")); + break; + } + ptid = thr->ptid; + } + gdb_assert (ptid != null_ptid); + } if (status->kind != TARGET_WAITKIND_EXITED && status->kind != TARGET_WAITKIND_SIGNALLED