This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Date: Tue, 22 Aug 2017 18:56:20 -0400
- Subject: Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BD0436146A
- References: <20170822140448.19153-1-sergiodj@redhat.com> <3ef5d58f-6d27-8f07-b45d-db7f883837a4@redhat.com>
On Tuesday, August 22 2017, Pedro Alves wrote:
> On 08/22/2017 03:04 PM, Sergio Durigan Junior wrote:
>> The fix for PR gdb/20609:
>>
>> commit bb805577d2b212411fb7b0a2d01644567fac4e8d
>> Author: Jan Kratochvil <jan.kratochvil@redhat.com>
>> 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.
Thanks for the review.
Well, what can I say. My fix looked right from my perspective, and I
confess that at the beginning I had the same thought: remote.c is
causing the problem by making inferior_ptid point to a non existing
thread. However, I quickly found that the culprit was on the call chain
leading to exec_file_locate_attach and concentrated my focus on that.
Your patch looks more complete and to the point indeed. Although it
seems to me, from what I observed, that calling breakpoint_re_set on
exec_file_locate_attach when dealing with a remote inferior doesn't make
sense either.
Anyway, I'll resubmit my patch using your approach and leave my first
patch aside for a bit, until I hear what you think about not calling
breakpoint_re_set on this specific case.
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/