This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] gdbserver: When attaching, add process before lwps
- From: Pedro Alves <palves at redhat dot com>
- To: Alan Hayward <Alan dot Hayward at arm dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, nd <nd at arm dot com>
- Date: Fri, 1 Feb 2019 12:30:00 +0000
- Subject: Re: [PATCH] gdbserver: When attaching, add process before lwps
- References: <20190125104834.2595-1-alan.hayward@arm.com> <0e75277b-fa80-129d-e82b-3f854bb33124@redhat.com> <7F0653B2-0EE2-4242-BE91-C40B055FD169@arm.com>
On 02/01/2019 11:20 AM, Alan Hayward wrote:
>
>
>> On 25 Jan 2019, at 18:23, Pedro Alves <palves@redhat.com> wrote:
>>
>> On 01/25/2019 10:48 AM, Alan Hayward wrote:
>>> The recent BP/WP changes for AArch64 swapping the order in add_lwp()
>>> so that the process was added before the lwp. This was due to the lwp
>>> creation requiring the process data.
>>>
>>> This also needs changing in linux_attach().
>>>
>>> Fixes gdb.server/ext-attach.exp on Aarch64.
>>>
>>> (This regression was hidden due to the racy nature of the gdb.server
>>> tests - now they are no longer racy it'll be easier to spot. Also
>>> checked X86).
>>>
>>> gdb/gdbserver/ChangeLog:
>>>
>>> 2019-01-25 Alan Hayward <alan.hayward@arm.com>
>>>
>>> * linux-low.c (linux_attach): Add process before lwp.
>>
>>> ---
>>> gdb/gdbserver/linux-low.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>>> index 17cce24d76..1ab2cfb1eb 100644
>>> --- a/gdb/gdbserver/linux-low.c
>>> +++ b/gdb/gdbserver/linux-low.c
>>> @@ -1188,6 +1188,8 @@ linux_attach (unsigned long pid)
>>> ptid_t ptid = ptid_t (pid, pid, 0);
>>> int err;
>>>
>>> + proc = linux_add_process (pid, 1);
>>> +
>>> /* Attach to PID. We will check for other threads
>>> soon. */
>>> err = linux_attach_lwp (ptid);
>>> @@ -1198,8 +1200,6 @@ linux_attach (unsigned long pid)
>>> error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
>>> }
>>>
>>> - proc = linux_add_process (pid, 1);
>>> -
>>
>> This fails to consider the error conditions. That error call
>> visible above throws an exception.
>>
>> - If GDBserver is already attached to the process, this will
>> create another process_info object for the already-attached
>> process, and leave it behind.
>>
>> For this case, it would seem better to check whether we're
>> already attached to a given PID in common code, around
>> server.c:attach_inferior.
>
> Assuming you are suggesting adding a check if the process object
> exists in gdbserver, rather than adding a new target func which
> then calls out to /proc.
Yes. I don't really understand what the call to /proc would do.
>
>>
>> - If GDBserver isn't already attached, but the attach fails,
>> this will likewise leave a stale process_info object behind.
>>
>
> I think I’m probably missing something here, but if the attach
> fails for any reason then gdbserver will error and exit - so it
> won’t matter if the object is not deleted?
Try "gdbserver --multi" + "target extended-remote".
With that, gdbserver won't exit when a process exits. The
connection stays open, ready for a "run" or "attach" command
to spawn new processes on the server side.
>
>
> New patch with the above changes. Ok?
Can you add a test for that error case to gdb.base/attach.exp, please?
(gdb) attach PID
(gdb) add-iferior
(gdb) inferior 2
(gdb) attach PID # should fail.
# restart gdb
The restart gdb step checks whether gdbserver crashes / or misbehaves
while detaching from the process.
Thanks,
--
Thanks,
Pedro Alves