Re: [patch] Fix Linux attach to signalled/stopped processes

[Pedro Alves <pedro@codesourcery.com>]

Some quick comments,

A Friday 11 April 2008 22:02:21, Daniel Jacobowitz wrote:

 > +  /* Pass on the last signal, if appropriate.  */
> +  if (lp->status == 0 && GET_LWP (lp->ptid) == GET_LWP (inferior_ptid)
> +      && stop_signal != TARGET_SIGNAL_0 && signal_pass_state (stop_signal))
> +    lp->status = W_STOPCODE (target_signal_to_host (stop_signal));

You're writing to lp->status of inferior_ptid, which isn't garantied
to be the main thread or the thread that got the last signal (user
may have used the "thread" command to switch threads, or we're stopped
at a breakpoint in some other thread not the main one, for example, or
the kernel decided to send the signal to some other thread because the
main one was already stopped?).

>   /* We don't actually detach from the LWP that has an id equal to the
>      overall process id just yet.  */
>   if (GET_LWP (lp->ptid) != GET_PID (lp->ptid))
>@@ -1263,14 +1343,29 @@ static void
> linux_nat_detach (char *args, int from_tty)
> {
>   int pid;
>+  int status;
>+  enum target_signal sig;
>+
>   if (target_can_async_p ())
>     linux_nat_async (NULL, 0);
> 
>-  iterate_over_lwps (detach_callback, NULL);
>+  iterate_over_lwps (detach_callback, &status);
>

Passing &status seems bogus, since you're passing the status
in lp->status ?
 
>   /* Only the initial process should be left right now.  */
>   gdb_assert (num_lwps == 1);
> 
>+  /* Pass on any pending signal for the last LWP.  */
>+  status = lwp_list->status;
>+  if (WIFSTOPPED (status) && (args == NULL || *args == '\0'))
>+    {
>+      args = alloca (8);
>+      sprintf (args, "%d", (int) WSTOPSIG (status));
>+      fprintf_unfiltered (gdb_stdlog,
>+                         "LND: Sending signal %s to %s\n",
>+                         args,
>+                         target_pid_to_str (lwp_list->ptid));
>+    }
>+
>

I don't see where you're actually sending the signal.  Even if you
did, it isn't guarantied you are passing this signal.  It will miss it
if inferior_ptid isn't the main thread.  See comments above.

Also, why the alloca dance instead of using
"Sending signal %d to %s\n" directly, if you're not doing anything
else with args?  AKA, what's the plan for args?

What's supposed to happen if stop_signal was a SIGTRAP ?

-- 
Pedro Alves