From patchwork Tue Aug 22 18:08:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 22316 Received: (qmail 35145 invoked by alias); 22 Aug 2017 18:09:04 -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 35109 invoked by uid 89); 22 Aug 2017 18:09:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No 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, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=disaster 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; Tue, 22 Aug 2017 18:09:01 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4B0F6C056788 for ; Tue, 22 Aug 2017 18:09:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4B0F6C056788 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5316260BE4; Tue, 22 Aug 2017 18:08:55 +0000 (UTC) Subject: Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB To: Sergio Durigan Junior , GDB Patches References: <20170822140448.19153-1-sergiodj@redhat.com> Cc: Jan Kratochvil From: Pedro Alves Message-ID: <3ef5d58f-6d27-8f07-b45d-db7f883837a4@redhat.com> Date: Tue, 22 Aug 2017 19:08:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170822140448.19153-1-sergiodj@redhat.com> On 08/22/2017 03:04 PM, Sergio Durigan Junior wrote: > The fix for PR gdb/20609: > > commit bb805577d2b212411fb7b0a2d01644567fac4e8d > Author: Jan Kratochvil > Date: Thu Sep 29 17:38:16 2016 +0200 > > Introduced the concept of deferring the call to breakpoint_re_set on > certain useful occasions. However, there is one specific scenario > where delaying needs to be done and still isn't: the case when we're > starting a GDB to debug a remote inferior without specifying a local > binary, as in for example: > > ./gdb -nx -q --data-directory=data-directory -ex "tar ext :1234" \ > -ex "set remote exec-file /bin/ls" -ex r > > In this case, when calling exec_file_locate_attach to locate the > inferior, GDB is incorrectly resetting the breakpoints without a > thread/inferior even running, which causes an assertion to be > triggered: > > binutils-gdb/gdb/thread.c:1609: internal-error: scoped_restore_current_thread::scoped_restore_current_thread(): Assertion `tp != NULL' failed. > A problem internal to GDB has been detected, > further debugging may prove unreliable. > Quit this debugging session? (y or n) > > The right thing to do is to defer resetting the breakpoints when > locating the binary, which is what this patch does. Hmm, I think we're missing more rationale. There may well be other reasons for doing that, but this case just looks like a case of remote.c breaking invariants to me -- making inferior_ptid point to a non-existing thread and then calling common code is recipe for disaster. Seems to me that the fix is just to not do that? See patch below. It fixes your test for me as well, though I haven't run the full testsuite. From: Pedro Alves Date: 2017-08-22 18:42:27 +0100 fix remote.c --- gdb/remote.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index ff59a0f..ea21163 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -3831,19 +3831,16 @@ add_current_inferior_and_thread (char *wait_status) { struct remote_state *rs = get_remote_state (); int fake_pid_p = 0; - ptid_t ptid; inferior_ptid = null_ptid; /* Now, if we have thread information, update inferior_ptid. */ - ptid = get_current_thread (wait_status); + ptid_t curr_ptid = get_current_thread (wait_status); - if (!ptid_equal (ptid, null_ptid)) + if (curr_ptid != null_ptid) { if (!remote_multi_process_p (rs)) fake_pid_p = 1; - - inferior_ptid = ptid; } else { @@ -3851,14 +3848,17 @@ add_current_inferior_and_thread (char *wait_status) (such as kill) won't work. This variable serves (at least) double duty as both the pid of the target process (if it has such), and as a flag indicating that a target is active. */ - inferior_ptid = magic_null_ptid; + curr_ptid = magic_null_ptid; fake_pid_p = 1; } - remote_add_inferior (fake_pid_p, ptid_get_pid (inferior_ptid), -1, 1); + remote_add_inferior (fake_pid_p, ptid_get_pid (curr_ptid), -1, 1); - /* Add the main thread. */ - add_thread_silent (inferior_ptid); + /* Add the main thread and switch to it. Don't try reading + registers yes, since we haven't fetched the target description + yet. */ + thread_info *tp = add_thread_silent (curr_ptid); + switch_to_thread_no_regs (tp); } /* Print info about a thread that was found already stopped on