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

[Pedro Alves <palves@redhat.com>]

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