From patchwork Thu Oct 17 22:50: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: 35127 Received: (qmail 26542 invoked by alias); 17 Oct 2019 23:00:45 -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 26382 invoked by uid 89); 17 Oct 2019 23:00:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= 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; Thu, 17 Oct 2019 23:00:24 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A1CDA3086213 for ; Thu, 17 Oct 2019 22:50:40 +0000 (UTC) Received: from localhost.localdomain (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2F81E100EA01 for ; Thu, 17 Oct 2019 22:50:40 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 14/24] Tweak handling of remote errors in response to resumption packet Date: Thu, 17 Oct 2019 23:50:16 +0100 Message-Id: <20191017225026.30496-15-palves@redhat.com> In-Reply-To: <20191017225026.30496-1-palves@redhat.com> References: <20191017225026.30496-1-palves@redhat.com> With current master, on a Fedora 27 machine with a kernel with buggy watchpoint support, I see: (gdb) PASS: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: hardware breakpoints work continue Continuing. warning: Remote failure reply: E01 Remote communication error. Target disconnected.: Connection reset by peer. (gdb) FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: watchpoints work continue The program is not being run. (gdb) FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: breakpoint after the first fork (the program is no longer running) The FAILs themselves aren't what's interesting here. What is interesting is that with the main multi-target patch applied, I was getting this: (gdb) PASS: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: hardware breakpoints work continue Continuing. warning: Remote failure reply: E01 /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inferior.c:285: internal-error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: watchpoints work (GDB internal error) The problem is that in remote_target::wait_as, we're hitting this: switch (buf[0]) { case 'E': /* Error of some sort. */ /* We're out of sync with the target now. Did it continue or not? Not is more likely, so report a stop. */ rs->waiting_for_stop_reply = 0; warning (_("Remote failure reply: %s"), buf); status->kind = TARGET_WAITKIND_STOPPED; status->value.sig = GDB_SIGNAL_0; break; which leaves event_ptid as null_ptid. At the end of the function, we then reach: else if (status->kind != TARGET_WAITKIND_EXITED && status->kind != TARGET_WAITKIND_SIGNALLED) { if (event_ptid != null_ptid) record_currthread (rs, event_ptid); else event_ptid = inferior_ptid; <<<<< here } and the trouble is that with the multi-target patch, we'll get here with inferior_ptid as null_ptid too. That is done exactly to find these implicit assumptions that inferior_ptid is a good choice for default thread, which isn't generaly true. I first thought of fixing this in the "case 'E'" path, but, given that this "event_ptid = inferior_ptid" path is also taken when the remote target does not support threads at all, no thread-related packets or extensions, it's better to fix it in latter path, to handle all scenarios that miss reporting a thread. That's what this patch does. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * remote.c (first_remote_resumed_thread): New. (remote_target::wait_as): Use it as default event_ptid instead of inferior_ptid. --- gdb/remote.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/gdb/remote.c b/gdb/remote.c index f74881a3c1..553f09a6e0 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -7697,6 +7697,17 @@ remote_target::wait_ns (ptid_t ptid, struct target_waitstatus *status, int optio } } +/* Return the first resumed thread. */ + +static ptid_t +first_remote_resumed_thread () +{ + for (thread_info *tp : all_non_exited_threads (minus_one_ptid)) + if (tp->resumed) + return tp->ptid; + return null_ptid; +} + /* Wait until the remote machine stops, then return, storing status in STATUS just as `wait' would. */ @@ -7833,7 +7844,7 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status, int options) if (event_ptid != null_ptid) record_currthread (rs, event_ptid); else - event_ptid = inferior_ptid; + event_ptid = first_remote_resumed_thread (); } else /* A process exit. Invalidate our notion of current thread. */