This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]