From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 119729 invoked by alias); 26 Feb 2020 18:52:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 119721 invoked by uid 89); 26 Feb 2020 18:52:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.8 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=HX-Spam-Relays-External:209.85.128.67, H*RU:209.85.128.67, thread_info, internal-error X-HELO: mail-wm1-f67.google.com Received: from mail-wm1-f67.google.com (HELO mail-wm1-f67.google.com) (209.85.128.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Feb 2020 18:52:20 +0000 Received: by mail-wm1-f67.google.com with SMTP id q9so401617wmj.5 for ; Wed, 26 Feb 2020 10:52:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=fqCW5aubJ0NfW9Ml+me7xmVCwT99aioITCa6GVd1XXg=; b=NlXEJhs2/r11usskI+OlVl4DDky43X/jz+weMq0mr04cWIdsxl+vzauNMgDGFiF3Q9 KXH4Bd1LzRnTHMgn7zzBA1R7o87sz2cGYNJ8WS9aOWe4toyiF3xSK26xoKOiDwbYcOXw IZCplOe056BsUVlcKICmu9J17A53/t09eSUWV9RM3wJL6eQ0WXbWqFGxxlU+BMw83pln eu93hbPGYqCUCOen5vb66a8obX0OG638kJ/uA6OcybkNpf01NrEuNj8QOl3l1tZkjIMg iNRThI0kQOqz1pLf4uVQZ7tNq7SMsntnGcWkl1p78n8zY7yIfC1aPY2XyxR1agE2LKZW INIw== Return-Path: Received: from localhost (host86-186-80-160.range86-186.btcentralplus.com. [86.186.80.160]) by smtp.gmail.com with ESMTPSA id h128sm4216636wmh.33.2020.02.26.10.52.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 26 Feb 2020 10:52:17 -0800 (PST) Date: Wed, 26 Feb 2020 18:52:00 -0000 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: palves@redhat.com Subject: Re: [PATCH] gdb/remote: Ask target for current thread if it doesn't tell us Message-ID: <20200226185215.GH3317@embecosm.com> References: <20200131000600.11699-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200131000600.11699-1-andrew.burgess@embecosm.com> X-Fortune: A woman was in love with fourteen soldiers. It was clearly platoonic. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00981.txt.bz2 Any feedback on this patch? I'd like to get this, or something like this merged soon-ish. Thanks, Andrew * Andrew Burgess [2020-01-31 00:06:00 +0000]: > 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 that, if we don't get a > thread id in the stop reply, then we should just ask the target for > the current thread. > > 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): Don't use > inferior_ptid, instead ask the remote for the current thread. > > Change-Id: I06f6fe5cc9a5cfc6d7157772aa7582f44c413bd5 > --- > 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 be2987707ff..94ed57ebf33 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 > + thread in the current inferior. */ > if (ptid == null_ptid) > - ptid = inferior_ptid; > + { > + ptid = remote_current_thread (null_ptid); > + if (ptid == null_ptid) > + { > + /* We didn't get a useful answer back from the remote target so > + we need to pick something - we can't just report null_ptid. > + Lets just pick the first thread in GDB's current inferior. */ > + struct thread_info *thread > + = first_thread_of_inferior (current_inferior ()); > + gdb_assert (thread != nullptr); > + ptid = thread->ptid; > + } > + } > > if (status->kind != TARGET_WAITKIND_EXITED > && status->kind != TARGET_WAITKIND_SIGNALLED > -- > 2.14.5 >