Re: [patch 2/3] attach-fail-reasons: Say more than ptrace: Operation not permitted.

Pedro Alves
2012-03-13T14:35:12Z

On 03/13/2012 01:54 PM, Jan Kratochvil wrote:

> On Tue, 13 Mar 2012 11:33:19 +0100, Pedro Alves wrote:
>> On 03/13/2012 09:43 AM, Jan Kratochvil wrote:
>>> I agree, this is affecting all the /proc operations.
>
> OK, so not completely all.
>
>
>> Yeah, although if you're already attached to the process, then the risk of /proc/..
>> hitting the wrong pid is much lower, though it does exist.
>
> How? It remains as 'Z (zombie)' with non-zero 'TracerPid' and even root
> cannot reuse that PID without killing the tracer.

E.g., with a multi-threaded exec, the execing thread just disappears
from /proc/TID as in reality the kernel changed the execing thread's TID, so
it's possible we end trying to open the non-existing /proc/TID/... before handling
the exec event. I think there were other ways for a tracee to be completely
killed/removed behind the tracer's back, but maybe that was only with older kernels,
if it ever was.

>>> (2b) If introducing a new framework to gdbserver code we should bring in STL
>>> and not the libiberty/gdb code. But we cannot yet use STL.
>>
>> Although I'm a C++ supporter, I really don't think such C++ arguments hold
>> much weight for the current C based code base.
>
> Offtopic here but I think this code was another argument for C++, wasn't it?

It was like saying that if you had posted this code written in C++ doing this
dynamic string growth with delete[]/new[] to reallocate (instead of std::string, not
knowing its existence), and justified it by claiming the right solution is to
switch to Java. Eventually. And only maybe.

> gdb/
> 2012-03-13 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * common/linux-procfs.c (linux_proc_get_int): New, from
> linux_proc_get_tgid, change its LWPID type to pid_t, add parameter
> field.
> (linux_proc_get_tgid): Only call linux_proc_get_int.
> (linux_proc_get_tracerpid): New.
> (linux_proc_pid_has_state): New, from linux_proc_pid_is_zombie.
> (linux_proc_pid_is_stopped, linux_proc_pid_is_zombie): Only call
> linux_proc_pid_has_state.
> * common/linux-procfs.h (linux_proc_get_tracerpid): New declaration.
> * common/linux-ptrace.c: Include linux-procfs.h and buffer.h.
> (linux_ptrace_attach_warnings): New.
> * common/linux-ptrace.h (struct buffer, linux_ptrace_attach_warnings):
> New declaration.
> * linux-nat.c: Include exceptions.h, linux-ptrace.h and buffer.h.
> (linux_nat_attach): New variables ex, buffer, message and message_s.
> Wrap to_attach by TRY_CATCH and call linux_ptrace_attach_warnings.
>
> gdb/gdbserver/
> 2012-03-13 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * linux-low.c (linux_attach_lwp_1): New variable buffer. Call
> linux_ptrace_attach_warnings.
>
> gdb/testsuite/
> 2012-03-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdb.base/attach-twice.c: New files.
> * gdb.base/attach-twice.exp: New files.
>

Looks good too.

--
Pedro Alves