Re: [PATCH] Share ptrace options discovery/linux native code between GDB and gdbserver

[Pedro Alves <palves@redhat.com>]

On 08/06/2013 04:28 PM, Luis Machado wrote:

>>> >> +/* Signals to block to make that sigsuspend work.  */
>>> >> +static sigset_t blocked_mask;
>> >
>> > This isn't right.  On linux-nat.c, this is a global because
>> > it holds the LinuxThreads signals too, initialized by
>> > lin_thread_get_thread_signals.  Now the block_child_signals
>> > in linux-nat.c will no longer block the right signals, as
>> > linux-nat.c:blocked_mask still exists, and that is the
>> > one that gets the LinuxThreads signals added, not this
>> > one, but it's this one that block_child_signals operates
>> > on ...
>> >
>> > On gdbserver, the my_waitpid wrapper does:
>> >
>> >    if (flags & __WALL)
>> >      {
>> >        sigset_t block_mask, org_mask, wake_mask;
>> >        ^^^^^^^^^^^^^^^^^^^
>> >
>> > It's a local, for a reason.  It's because ...
>> >
>> >        int wnohang;
>> >
>> >        wnohang = (flags & WNOHANG) != 0;
>> >        flags &= ~(__WALL | __WCLONE);
>> >        flags |= WNOHANG;
>> >
>> >        /* Block all signals while here.  This avoids knowing about
>> >           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> > 	 LinuxThread's signals.  */
>> >           ^^^^^^^^^^^^^^^^^^^^^^^^^
>> >        sigfillset (&block_mask);
>> >        ^^^^^^^^^^^^^^^^^^^^^^^^^
>> >
>> > ... we block all signals.  The comment alludes exactly to
>> > the avoiding of GDBserver doing things differently, and
>> > avoiding the need for the block_mask global.
>> >
>> >        sigprocmask (SIG_BLOCK, &block_mask, &org_mask);
>> >
>> >
> Ok. But what about its use on 
> common/linux-ptrace.c:linux_check_ptrace_features. Why does GDB check 
> for features differently? Or is this shared mask not needed there at all 
> and thus we don't need to call block_child_signals and 
> restore_child_signals_mask at all?
> 
> I'll send an updated patch once i fully understand the details on 
> signal-blocking.

Okay, I've sent a patch (along with the rationale) for this at:

 http://sourceware.org/ml/gdb-patches/2013-08/msg00385.html

Let me know what you think.  I believe that should simplify
your patch.

-- 
Pedro Alves