[gdbserver] Uninitialized variable in linux-low.c:handle_extended_wait

[gdbserver] Uninitialized variable in linux-low.c:handle_extended_wait

[Ulrich Weigand]

Hello,

I've got a test case where a newly created thread reports a SIGSTOP
event *before* its parent reports the PTRACE_EVENT_CLONE event.

This is supposed to be handled via the "stopped_pids" list, but it
looks like this doesn't actually work, because in this case the
handle_extended_wait routine accesses an uninitialized variable:

      unsigned long new_pid;
      int ret, status;

      ptrace (PTRACE_GETEVENTMSG, inferior_pid, 0, &new_pid);

      /* If we haven't already seen the new PID stop, wait for it now.  */
      if (! pull_pid_from_list (&stopped_pids, new_pid))
        {
          /* The new child has a pending SIGSTOP.  We can't affect it until it
             hits the SIGSTOP, but we're already attached.  */

          do {
            ret = waitpid (new_pid, &status, __WALL);
          } while (ret == -1 && errno == EINTR);
[...]
        }
[...]
      /* Normally we will get the pending SIGSTOP.  But in some cases
         we might get another signal delivered to the group first.
         If we do, be sure not to lose it.  */
      if (WSTOPSIG (status) == SIGSTOP)
        {
          if (stopping_threads)
            new_process->stopped = 1;
          else
            ptrace (PTRACE_CONT, new_pid, 0, 0);
        }
      else
        {
          new_process->stop_expected = 1;
          if (stopping_threads)
            {
              new_process->stopped = 1;
              new_process->status_pending_p = 1;
              new_process->status_pending = status;
            }
          else
            /* Pass the signal on.  This is what GDB does - except
               shouldn't we really report it instead?  */
            ptrace (PTRACE_CONT, new_pid, 0, WSTOPSIG (status));
        }


Note how "status" is used uninitialized if pull_pid_from_list
returns true.  In my case, this causes the "else" branch to be
taken, which sets "stop_expected", which causes gdbserver to
hang later on while waiting on the SIGSTOP which actually 
already arrived ...

I'm not quite sure how to handle this -- I assume processes
on the stopped_pids list should always be handled as if they
got a SIGSTOP?  The following patch fixes the hang for me ...

Does this make sense?

Bye,
Ulrich


ChangeLog:

	* linux-low.c (handle_extended_wait): Do not use "status"
	variable uninitialized.

--- linux-low.c.orig    2008-07-11 05:21:43.185268918 +0200
+++ linux-low.c 2008-07-11 05:22:48.855806784 +0200
@@ -156,7 +156,7 @@
   if (event == PTRACE_EVENT_CLONE)
     {
       unsigned long new_pid;
-      int ret, status;
+      int ret, status = W_STOPCODE (SIGSTOP);

       ptrace (PTRACE_GETEVENTMSG, inferior_pid, 0, &new_pid);



-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [gdbserver] Uninitialized variable in linux-low.c:handle_extended_wait

[Daniel Jacobowitz]

On Fri, Jul 11, 2008 at 05:37:05AM +0200, Ulrich Weigand wrote:
> Note how "status" is used uninitialized if pull_pid_from_list
> returns true.  In my case, this causes the "else" branch to be
> taken, which sets "stop_expected", which causes gdbserver to
> hang later on while waiting on the SIGSTOP which actually 
> already arrived ...
> 
> I'm not quite sure how to handle this -- I assume processes
> on the stopped_pids list should always be handled as if they
> got a SIGSTOP?  The following patch fixes the hang for me ...
> 
> Does this make sense?

Completely; your patch is OK.  The code only triggers rarely, so I
just merged the version I wrote for linux-nat.c some time ago to
gdbserver.  Apparently I did it wrong :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [gdbserver] Uninitialized variable in linux-low.c:handle_extended_wait

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> Completely; your patch is OK.  The code only triggers rarely, so I
> just merged the version I wrote for linux-nat.c some time ago to
> gdbserver.  Apparently I did it wrong :-)

I've checked the patch in now.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com