Re: [PATCH] MIPS/GDB/linux-nat.c: Fix a child detach condition for uClibc-ng

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Maciej W. Rozycki <macro@mips.com> [2018-07-05 13:32:06 +0100]:

> On Tue, 3 Jul 2018, Andrew Burgess wrote:
> 
> > > Current implementation expects that WIFSTOPPED (W_STOPCODE (0)) is true,
> > > but in the uClibc-ng it is false on MIPS. The patch adds a "detach"
> > > helper variable to avoid this corner case: WIFSTOPPED applied
> > > only to a status filled by a waitpid call that should never return
> > > the status with zero stop signal.
> > 
> > I took a quick look through this patch, and have a little feedback.
> > 
> > You might want to expand your commit message to explain _why_ MIPS is
> > different (having a signal 127), I didn't know this, and it puzzled me
> > for a while as to why your above text didn't just indicate a bug in
> > uClibc-ng :)
> 
>  I think it is a bug in uClibc-ng after all, because you can't receive 
> signal #0 on Linux.  This is what the kernel has (in kernel/signal.c):
> 
> 		/*
> 		 * The null signal is a permissions and process existence
> 		 * probe.  No signal is actually delivered.
> 		 */
> 		if (!error && sig) {
> 			error = do_send_sig_info(sig, info, p, false);
> 			/*
> 			 * If lock_task_sighand() failed we pretend the task
> 			 * dies after receiving the signal. The window is tiny,
> 			 * and the signal is private anyway.
> 			 */
> 			if (unlikely(error == -ESRCH))
> 				error = 0;
> 		}
> 
> and also:
> 
> 	if (!ret && sig)
> 		ret = do_send_sig_info(sig, info, p, true);
> 
> and documentation for kill(2) agrees:
> 
>       "If  sig  is 0, then no signal is sent, but error checking is still per-
>        formed; this can be used to check for the existence of a process ID  or
>        process group ID."
> 
> > [PATCH] gdb: Avoid using W_STOPCODE(0) as this is ambiguous on MIPS
> > 
> > The MIPS target supports 127 signals, and this can create an ambiguity
> > in process wait statuses.  A status value of 0x007f could potentially
> > indicate a process that has exited with signal 127, or a process that
> > has stopped with signal 0.
> > 
> > In uClibc-ng the interpretation of 0x007f is that the process has
> > exited with signal 127 rather than stopped with signal 0, and so,
> > WIFSTOPPED (W_STOPCODE (0)) will be false rather than true as it would
> > be on most other platforms.
> 
>  So there is no ambiguity, 0x007f means that the process has exited with 
> signal 127 and nothing else, and while I agree the change might be a good 
> workaround for uClibc-ng's oddity, I think uClibc-ng's implementation of 
> WIFSTOPPED has also to be fixed to reflect reality.  Which also means the 
> commit description needs to be updated accordingly.

OK.  I think we're mostly agreeing with each other, it's just the
part about uClibc-ng which I'd like to push on a bit more.

Lets consider a couple of cases, first signal 0x33.

  Status |
   Value | Meaning
  ------------------
    0033 | Killed by signal 0x33
    337f | Stopped by signal 0x33

Now, signal 127, or 0xf7:

    007f | Killed by signal 0x7f
    7f7f | Stopped by signal 0x7f

And if we _pretend_ for a moment, that signal 0x00 can be sent:

    0000 | Killed by signal 0x00
    007f | Stopped by signal 0x00

So the ambiguity arises if we can send both signal 0x7f AND signal
0x00.  But, as you point out the kernel prevents sending signal 0x00,
and for all targets except MIPS, signal 0x7f is also prevented.  Most
targets seems to have 32 or 64 signals.  MIPS has 128.

But this shouldn't be a problem, the kernel (as you point out)
prevents us sending signal 0x00.

The problem is that signal 0x00 isn't coming from the kernel, it's
being synthesised in GDB with 'W_STOPCODE (0)', which builds the
status 007f.

I might even be bold enough to declare that 'W_STOPCODE (0)' should be
declare a bad thing .... maybe ...

Still, I don't see how uClibc-ng is doing anything wrong, a status is
only a stop status if (a) it ends in 0x7f, and (b) the signal number
is non-zero, this seems pretty reasonable really.

Thanks,
Andrew



> 
>  FWIW,
> 
>   Maciej