From patchwork Thu Oct 17 00:54:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 35079 Received: (qmail 127285 invoked by alias); 17 Oct 2019 00:54:25 -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 126978 invoked by uid 89); 17 Oct 2019 00:54:25 -0000 Authentication-Results: sourceware.org; auth=none 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 autolearn=ham version=3.3.1 spammy=Valgrind, termination X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 17 Oct 2019 00:54:23 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1571273662; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UT7cYySx26Sq46op9yUvfiLychY15/Nvuys4LuZQnAc=; b=erKlKgf0rRfP156OwAarixbtrbL9xlzURkFTOdH2kmLmv16KxVLiXaLm5zkr9Z2LEGYalm AJTMOmox+e8CVpRKcuplL6O93D1x/p2FAUO8Y8s32umTFNwOrlz/yUR5Um5vx1LUPpLvZm i3oFlNws2Q9+lVO6FJbPpQUsRLRSbD4= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-370-T5fZaAiBMzSwLas1_Ce5dg-1; Wed, 16 Oct 2019 20:54:19 -0400 Received: by mail-wr1-f72.google.com with SMTP id o10so166992wrm.22 for ; Wed, 16 Oct 2019 17:54:19 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id a13sm474473wrf.73.2019.10.16.17.54.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Oct 2019 17:54:17 -0700 (PDT) Subject: [PATCH 14.5/23] Avoid another inferior_ptid reference in gdb/remote.c (Re: [PATCH 14/23] Tweak handling of remote errors in response to resumption packet) To: "Aktemur, Tankut Baris" References: <20190906232807.6191-1-palves@redhat.com> <20190906232807.6191-15-palves@redhat.com> Cc: "gdb-patches@sourceware.org" From: Pedro Alves Message-ID: Date: Thu, 17 Oct 2019 01:54:16 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 On 10/9/19 2:34 PM, Aktemur, Tankut Baris wrote: > * On September 7, 2019 1:28 AM, Pedro Alves wrote: > A similar problem could occur in case of an exit event, too -- for instance, > if the remote target does not support the multi-process packet or if the > packet is disabled. > > This can be checked by modifying the test > > testsuite/gdb.server/connect-without-multi-process.exp > > to continue the inferior to termination. Oh, excellent, this is the same issue that Philippe discovered with Valgrind, for which I sent a patch earlier: https://sourceware.org/ml/gdb-patches/2019-10/msg00512.html I did not realize you had already analyzed it. I was having second thoughts on whether we can use general_thread, and for that I like your patch better than mine. In haste, had completely forgot that patch #14 added first_remote_resumed_thread. Also, I wrote a new test from scratch, not realizing we already had connect-without-multi-process.exp. Sigh. Well, I actually originally wrote that one too, so I just completely forgot it. :-D Thanks for finding it. > I'm attaching these as a patch in case one wants to test. It should be applied > after the main multi-target patch. I tweaked it a bit further, reused the commit log from my version of the patch, and rebased it on top of patch #14. Here it is: From 25f279aa79a9860c64d228a9eb7fc974c2367c64 Mon Sep 17 00:00:00 2001 From: Tankut Baris Aktemur Date: Thu, 17 Oct 2019 01:34:19 +0100 Subject: [PATCH] Avoid another inferior_ptid reference in gdb/remote.c The multi-target patch makes inferior_ptid point to null_ptid before calling into target_wait, which catches bad uses of inferior_ptid, since the current selected thread in gdb shouldn't have much relation to the thread that reports an event. One such bad use is found in remote_target::remote_parse_stop_reply, where we handle the 'W' or 'X' packets (process exit), and the remote target does not support the multi-process extensions, i.e., it does not report the PID of the process that exited. With the multi-target patch, that would result in a failed assertion, trying to find the inferior for process pid 0. gdb/ChangeLog: yyyy-mm-dd Tankut Baris Aktemur Pedro Alves * remote.c (remote_target::remote_parse_stop_reply) : If no process is specified, return null_ptid instead of inferior_ptid. (remote_target::wait_as): Handle TARGET_WAITKIND_EXITED / TARGET_WAITKIND_SIGNALLED with no pid. gdb/testsuite/ChangeLog: yyyy-mm-dd Tankut Baris Aktemur Pedro Alves * gdb.server/connect-without-multi-process.exp: Also test continuing to end. --- gdb/remote.c | 18 +++++++++++++----- .../gdb.server/connect-without-multi-process.exp | 7 ++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 8f3fdf098d..ad8a14d80b 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -7446,7 +7446,6 @@ Packet: '%s'\n"), case 'W': /* Target exited. */ case 'X': { - int pid; ULONGEST value; /* GDB used to accept only 2 hex chars here. Stubs should @@ -7470,8 +7469,9 @@ Packet: '%s'\n"), event->ws.value.sig = GDB_SIGNAL_UNKNOWN; } - /* If no process is specified, assume inferior_ptid. */ - pid = inferior_ptid.pid (); + /* If no process is specified, return null_ptid, and let the + caller figure out the right process to use. */ + int pid = 0; if (*p == '\0') ; else if (*p == ';') @@ -7847,8 +7847,16 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status, int options) event_ptid = first_remote_resumed_thread (); } else - /* A process exit. Invalidate our notion of current thread. */ - record_currthread (rs, minus_one_ptid); + { + /* A process exit. Invalidate our notion of current thread. */ + record_currthread (rs, minus_one_ptid); + /* It's possible that the packet did not include a pid. */ + if (event_ptid == null_ptid) + event_ptid = first_remote_resumed_thread (); + /* EVENT_PTID could still be NULL_PTID. Double-check. */ + if (event_ptid == null_ptid) + event_ptid = magic_null_ptid; + } return event_ptid; } diff --git a/gdb/testsuite/gdb.server/connect-without-multi-process.exp b/gdb/testsuite/gdb.server/connect-without-multi-process.exp index fba20a6a0b..6ba35bf508 100644 --- a/gdb/testsuite/gdb.server/connect-without-multi-process.exp +++ b/gdb/testsuite/gdb.server/connect-without-multi-process.exp @@ -14,7 +14,7 @@ # along with this program. If not, see . */ # Check that we can connect to GDBserver with the multiprocess -# extensions disabled, and run to main. +# extensions disabled, run to main, and finish the process. load_lib gdbserver-support.exp @@ -52,6 +52,11 @@ proc do_test {multiprocess} { "target $gdbserver_protocol" gdb_test "continue" "main .*" "continue to main" + + # The W/X packets do not include the PID of the exiting process + # without the multi-process extensions. Check that we handle + # process exit correctly in that case. + gdb_continue_to_end } foreach multiprocess { "off" "auto" } {