This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP
- From: Pedro Alves <palves at redhat dot com>
- To: Mark Kettenis <mark dot kettenis at xs4all dot nl>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 03 Mar 2015 15:12:48 +0000
- Subject: Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP
- Authentication-results: sourceware.org; auth=none
- References: <201503031439 dot t23EdHZv020814 at glazunov dot sibelius dot xs4all dot nl>
On 03/03/2015 02:39 PM, Mark Kettenis wrote:
>> From: Pedro Alves <firstname.lastname@example.org>
>> Date: Tue, 3 Mar 2015 13:33:44 +0000
>> Tested on x86-64 Fedora 20, -m32.
>> 2015-03-03 Pedro Alves <email@example.com>
>> * i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
>> the lwp field of ptid. Pass the full ptid to get_thread_regcache.
>> * inf-ptrace.c (get_ptrace_pid): New function.
>> (inf_ptrace_resume): Use it.
>> * linux-nat.c (linux_resume_one_lwp): Pass the LWP's ptid ummodified
>> to the lower layer.
>> gdb/ChangeLog | 9 +++++++++
>> gdb/i386-linux-nat.c | 5 ++---
>> gdb/inf-ptrace.c | 25 ++++++++++++++++++++++---
>> gdb/linux-nat.c | 6 +-----
>> 4 files changed, 34 insertions(+), 11 deletions(-)
> I have some fear this is going to break non-Linux targets.
>> --- a/gdb/inf-ptrace.c
>> +++ b/gdb/inf-ptrace.c
>> @@ -289,6 +289,22 @@ inf_ptrace_stop (struct target_ops *self, ptid_t ptid)
>> kill (-inferior_process_group (), SIGINT);
>> +/* Return which PID to pass to ptrace in order to observe/control the
>> + tracee identified by PTID. */
>> +static pid_t
>> +get_ptrace_pid (ptid_t ptid)
>> + pid_t pid;
>> + /* If we have an LWPID to work with, use it. Otherwise, we're
>> + dealing with a non-threaded program/target. */
>> + pid = ptid_get_lwp (ptid);
>> + if (pid == 0)
>> + pid = ptid_get_pid (ptid);
>> + return pid;
>> /* Resume execution of thread PTID, or all threads if PTID is -1. If
>> STEP is nonzero, single-step it. If SIGNAL is nonzero, give it
>> that signal. */
>> @@ -297,13 +313,16 @@ static void
>> inf_ptrace_resume (struct target_ops *ops,
>> ptid_t ptid, int step, enum gdb_signal signal)
>> - pid_t pid = ptid_get_pid (ptid);
>> + pid_t pid;
>> int request;
> Please don't introduce a blank line here.
Darn, LOL, I removed one, added another...:
i386_linux_resume (struct target_ops *ops,
ptid_t ptid, int step, enum gdb_signal signal)
- int pid = ptid_get_pid (ptid);
+ int pid = ptid_get_lwp (ptid);
I'll remove the one I added...
>> - if (pid == -1)
>> + if (ptid_equal (minus_one_ptid, ptid))
>> /* Resume all threads. Traditionally ptrace() only supports
>> single-threaded processes, so simply resume the inferior. */
>> - pid = ptid_get_pid (inferior_ptid);
>> + pid = get_ptrace_pid (inferior_ptid);
> This defenitely should remain ptid_get_pid(); you want to resume the
> entire process and not an individual thread.
My thinking is that it doesn't matter in practice.
Or is it that if there are multiple kernel threads in the
process, ptrace(PTRACE_CONT, PID) on bsd actually resumes
them all? Then other things must be broken anyway.
I was assuming that on BSD targets that use this method,
there would only be one thread in the core thread list, and
it would either have LWPID==0, or have PID==LWPID, thus it didn't
matter if get_ptrace_pid returned the PID or the LWPID.
If there anything that actually creates other threads with
a different LWPID on these targets?
>> + else
>> + pid = get_ptrace_pid (ptid);
> This should work for OpenBSD, and probably works for FreeBSD. It
> won't work for NetBSD, but they will probably need their own
> implementation of this function, so it's probably fine.