This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB


On 08/22/2017 11:56 PM, Sergio Durigan Junior wrote:
> 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.

... doesn't make sense because?  The thing is we're missing an explicit
rationale from first principles.

> 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.

Well, I was hoping you'd go first.  :-)

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]