| Message ID | 7F0653B2-0EE2-4242-BE91-C40B055FD169@arm.com |
|---|---|
| State | New |
| Headers | show |
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,
On 02/01/2019 12:30 PM, Pedro Alves wrote: > The restart gdb step checks whether gdbserver crashes / or misbehaves > while detaching from the process. Forgot to say, make sure to test with --target_board=native-extended-gdbserver Thanks, Pedro Alves
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 44016d2310..8c5a51f23c 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -1188,18 +1188,19 @@ 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); if (err != 0) { - std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err); + remove_process (proc); + std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err); error ("Cannot attach to process %ld: %s", pid, reason.c_str ()); } - proc = linux_add_process (pid, 1); - /* Don't ignore the initial SIGSTOP if we just attached to this process. It will be collected by wait shortly. */ initial_thread = find_thread_ptid (ptid_t (pid, pid, 0)); diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index f9bfdd7307..e960c10d40 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -295,6 +295,9 @@ attach_inferior (int pid) /* myattach should return -1 if attaching is unsupported, 0 if it succeeded, and call error() otherwise. */ + if (find_process_pid (pid) != nullptr) + error ("Already attached to process %d\n", pid); + if (myattach (pid) != 0) return -1;