From patchwork Thu Mar 17 23:26:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 11371 Received: (qmail 81208 invoked by alias); 17 Mar 2016 23:27:24 -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 81182 invoked by uid 89); 17 Mar 2016 23:27:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=Tell X-HELO: mail-wm0-f43.google.com Received: from mail-wm0-f43.google.com (HELO mail-wm0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 17 Mar 2016 23:27:13 +0000 Received: by mail-wm0-f43.google.com with SMTP id p65so14909309wmp.0 for ; Thu, 17 Mar 2016 16:27:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=7vg/Y5jh8Uj71wyuHRMffZsap0TZcdlV5SiUOitU8gA=; b=CMsvhB41P+kXMlid5Xu/PEGlNOt1Hu0hkRcxmYOoHFLXJD91L5Q0DPG1TDiMoDJnU2 lj1VrXK0PXuf8BhzTVCorFo2V9GpJ/VZuDgPsGQ/lOF6AmCk+J4HoRqJkF1M1jmhsA1I hZKGB3ORdRbbtby521IUTmCuWH7WdS1qFRqRvQ0unAqv/HxmnU3WSYV2+Q9BQIGpgc8J fthk+ABMaJGpygh9OXCqoZoqr9Oh85+9IDC3nMRyIYz9euGCfVR0PADZU3l4PfKmcSha HNkoVwyqb/8Qk+pdn84Lq7zGsyta9TQtDYHsMQikI2AjFng+OwCUHjkLyKnA48iNEVeF vfug== X-Gm-Message-State: AD7BkJIYa8VB1j+1SE8KcvNxGTlo6IsqyYlCejE7//rEio/QEU2IHPWKXSLruhXsiSinMg== X-Received: by 10.28.22.74 with SMTP id 71mr13314122wmw.47.1458257230436; Thu, 17 Mar 2016 16:27:10 -0700 (PDT) Received: from localhost ([46.189.28.226]) by smtp.gmail.com with ESMTPSA id j18sm32401330wmd.2.2016.03.17.16.27.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 Mar 2016 16:27:09 -0700 (PDT) Date: Thu, 17 Mar 2016 23:26:58 +0000 From: Andrew Burgess To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/remote: Don't use vKill if multi-process features are disabled Message-ID: <20160317232658.GY14100@embecosm.com> References: <1458252144-3496-1-git-send-email-andrew.burgess@embecosm.com> <56EB2DD1.4090103@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <56EB2DD1.4090103@redhat.com> X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes * Pedro Alves [2016-03-17 22:21:05 +0000]: > On 03/17/2016 10:02 PM, Andrew Burgess wrote: > > The below was tested using native gdbserver on x86-64 Fedora Linux > > with no regressions. > > > > --- > > > > The gdb remote protocol documentation is clear that the vKill command > > should not be used unless the multi-process feature is reported as > > supported by the remote target. > > > > Currently within gdb we check to see if the vKill packet is enabled or > > not before using the vKill command, however, the only way to disable > > vKill is from the gdb console, the result is that vKill will be sent to > > targets that don't support it, and never claimed to support it. > > Why was that a problem? You're right, if my remote target was well behaved it shouldn't cause a problem... ... I have a new patch that is now just about bringing gdb into line with the documentation, that is, not sending vKill unless that remote target says it's supported. > > > > > After this commit I guard use of vKill with a check to see if the > > multi-process feature is enabled or not. I have removed the ability to > > disable vkill specifically from the console, the user must now disable > > the whole multi-process feature set as one. > > > > I did consider leaving the separate vKill control switch in addition to > > the multi-process control switch, but this seemed unnecessary, and I > > worried that in the future another bug could be introduced where > > PACKET_vKill was used to guard sending a vKill. > > Please leave the control switch in. It's useful for testing, to emulate > targets that don't support the packet. Done. OK to apply? Thanks, Andrew --- The gdb remote protocol documentation is clear that the vKill command should not be used unless the multi-process feature is reported as supported by the remote target. Currently in gdb we only check that PACKET_vKill is enabled before sending a vKill. The problem is that when a remote does not support multi-process features, it is PACKET_multi_process that is disabled. There is a desire to keep the PACKET_vKill control as a separate independent mechanism for controlling just the vKill packet, so this commit makes sending a vKill dependent on checking that both PACKET_vKill, and PACKET_multi_process are enabled. This should prevent sending vKill packets to targets that don't support them. gdb/ChangeLog: * remote.c (remote_kill): Use remote_multi_process_p. (remote_vkill): Add check of remote_multi_process_p. --- gdb/ChangeLog | 5 +++++ gdb/remote.c | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2df6ccd..0bcb430 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2016-03-17 Andrew Burgess + + * remote.c (remote_kill): Use remote_multi_process_p. + (remote_vkill): Add check of remote_multi_process_p. + 2016-03-17 Jan Kratochvil * linux-thread-db.c (check_pid_namespace_match): Extend the message. diff --git a/gdb/remote.c b/gdb/remote.c index af0a08a..8c9e073 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -8907,7 +8907,7 @@ remote_kill (struct target_ops *ops) int pid = ptid_get_pid (inferior_ptid); struct remote_state *rs = get_remote_state (); - if (packet_support (PACKET_vKill) != PACKET_DISABLE) + if (remote_multi_process_p (rs)) { /* If we're stopped while forking and we haven't followed yet, kill the child task. We need to do this before killing the @@ -8948,7 +8948,8 @@ remote_kill (struct target_ops *ops) static int remote_vkill (int pid, struct remote_state *rs) { - if (packet_support (PACKET_vKill) == PACKET_DISABLE) + if (!remote_multi_process_p (rs) + || (packet_support (PACKET_vKill) == PACKET_DISABLE)) return -1; /* Tell the remote target to detach. */