diff mbox

Fix crash of gdbserver when kill threads

Message ID 510f2362-8d33-4c3c-9a13-5d187f26abdf@SVR-ORW-FEM-04.mgc.mentorg.com
State New, archived
Headers

Commit Message

Hui Zhu June 23, 2014, 10:09 a.m. UTC 
  gdbserver :1234 gdb.base/watch_thread_num
gdb gdb.base/watch_thread_num
(gdb) b 48
Breakpoint 1 at 0x400737: file ../../../binutils-gdb/gdb/testsuite/gdb.base/watch_thread_num.c, line 48.
(gdb) c
Continuing.

Breakpoint 1, main () at ../../../binutils-gdb/gdb/testsuite/gdb.base/watch_thread_num.c:48
48	    thread_result = thread_function ((void *) i);
(gdb) k
Kill the program being debugged? (y or n) y
gdbserver :1234 gdb.base/watch_thread_num
Process gdb.base/watch_thread_num created; pid = 9719
Listening on port 1234
Remote debugging from host 127.0.0.1
Killing all inferiors
Segmentation fault (core dumped)

Backtrace:
(gdb) bt
#0  find_inferior (list=<optimized out>, func=func@entry=0x423990 <kill_one_lwp_callback>, arg=arg@entry=0x7fffe97405dc)
    at ../../../binutils-gdb/gdb/gdbserver/inferiors.c:199
#1  0x0000000000425bff in linux_kill (pid=10130) at ../../../binutils-gdb/gdb/gdbserver/linux-low.c:966
#2  0x000000000040ae8c in kill_inferior_callback (entry=<optimized out>) at ../../../binutils-gdb/gdb/gdbserver/server.c:2934
#3  0x0000000000405c61 in for_each_inferior (list=<optimized out>, action=action@entry=0x40ae60 <kill_inferior_callback>)
    at ../../../binutils-gdb/gdb/gdbserver/inferiors.c:57
#4  0x000000000040d5e2 in process_serial_event () at ../../../binutils-gdb/gdb/gdbserver/server.c:3767
#5  handle_serial_event (err=<optimized out>, client_data=<optimized out>) at ../../../binutils-gdb/gdb/gdbserver/server.c:3880
#6  0x0000000000412cda in handle_file_event (event_file_desc=event_file_desc@entry=4)
    at ../../../binutils-gdb/gdb/gdbserver/event-loop.c:434
#7  0x000000000041357a in process_event () at ../../../binutils-gdb/gdb/gdbserver/event-loop.c:189
#8  start_event_loop () at ../../../binutils-gdb/gdb/gdbserver/event-loop.c:552
#9  0x0000000000403088 in main (argc=3, argv=0x7fffe9740938) at ../../../binutils-gdb/gdb/gdbserver/server.c:3283

The cause of this issue is when linux_kill call "find_inferior (&all_threads, kill_one_lwp_callback , &pid)"
to kill all the lwp of pid.
In linux_wait_for_event, it will delete_lwp any lwp in all_threads if it
get exit event of it.  Then it make find_inferior crash.

I make a patch that let kill_one_lwp_callback return 1, then after
linux_wait_for_event is called(Maybe all_threads is changed), find_inferior
will return.
And change call "find_inferior (&all_threads, kill_one_lwp_callback , &pid)"
to be a loop.  It will stop when all_threads doesn't have any lwp is pid.

It pass regression test in x86_64 Linux. 

Thanks,
Hui

2014-06-23  Hui Zhu  <hui@codesourcery.com>

	* linux-low.c (kill_one_lwp_callback): Change last return to 1.
	(linux_kill): Call find_inferior with a loop.

Comments

Hui Zhu June 29, 2014, 3:28 a.m. UTC | #1 
Hi Pedro,

This issue is introduced by the patches for PR 12702 that was pushed by you.
Maybe you could take a look on this patch.

Thanks,
Hui

On 06/23/14 18:09, Hui Zhu wrote:
> gdbserver :1234 gdb.base/watch_thread_num
> gdb gdb.base/watch_thread_num
> (gdb) b 48
> Breakpoint 1 at 0x400737: file ../../../binutils-gdb/gdb/testsuite/gdb.base/watch_thread_num.c, line 48.
> (gdb) c
> Continuing.
>
> Breakpoint 1, main () at ../../../binutils-gdb/gdb/testsuite/gdb.base/watch_thread_num.c:48
> 48	    thread_result = thread_function ((void *) i);
> (gdb) k
> Kill the program being debugged? (y or n) y
> gdbserver :1234 gdb.base/watch_thread_num
> Process gdb.base/watch_thread_num created; pid = 9719
> Listening on port 1234
> Remote debugging from host 127.0.0.1
> Killing all inferiors
> Segmentation fault (core dumped)
>
> Backtrace:
> (gdb) bt
> #0  find_inferior (list=<optimized out>, func=func@entry=0x423990 <kill_one_lwp_callback>, arg=arg@entry=0x7fffe97405dc)
>      at ../../../binutils-gdb/gdb/gdbserver/inferiors.c:199
> #1  0x0000000000425bff in linux_kill (pid=10130) at ../../../binutils-gdb/gdb/gdbserver/linux-low.c:966
> #2  0x000000000040ae8c in kill_inferior_callback (entry=<optimized out>) at ../../../binutils-gdb/gdb/gdbserver/server.c:2934
> #3  0x0000000000405c61 in for_each_inferior (list=<optimized out>, action=action@entry=0x40ae60 <kill_inferior_callback>)
>      at ../../../binutils-gdb/gdb/gdbserver/inferiors.c:57
> #4  0x000000000040d5e2 in process_serial_event () at ../../../binutils-gdb/gdb/gdbserver/server.c:3767
> #5  handle_serial_event (err=<optimized out>, client_data=<optimized out>) at ../../../binutils-gdb/gdb/gdbserver/server.c:3880
> #6  0x0000000000412cda in handle_file_event (event_file_desc=event_file_desc@entry=4)
>      at ../../../binutils-gdb/gdb/gdbserver/event-loop.c:434
> #7  0x000000000041357a in process_event () at ../../../binutils-gdb/gdb/gdbserver/event-loop.c:189
> #8  start_event_loop () at ../../../binutils-gdb/gdb/gdbserver/event-loop.c:552
> #9  0x0000000000403088 in main (argc=3, argv=0x7fffe9740938) at ../../../binutils-gdb/gdb/gdbserver/server.c:3283
>
> The cause of this issue is when linux_kill call "find_inferior (&all_threads, kill_one_lwp_callback , &pid)"
> to kill all the lwp of pid.
> In linux_wait_for_event, it will delete_lwp any lwp in all_threads if it
> get exit event of it.  Then it make find_inferior crash.
>
> I make a patch that let kill_one_lwp_callback return 1, then after
> linux_wait_for_event is called(Maybe all_threads is changed), find_inferior
> will return.
> And change call "find_inferior (&all_threads, kill_one_lwp_callback , &pid)"
> to be a loop.  It will stop when all_threads doesn't have any lwp is pid.
>
> It pass regression test in x86_64 Linux.
>
> Thanks,
> Hui
>
> 2014-06-23  Hui Zhu  <hui@codesourcery.com>
>
> 	* linux-low.c (kill_one_lwp_callback): Change last return to 1.
> 	(linux_kill): Call find_inferior with a loop.
>
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -944,7 +944,9 @@ kill_one_lwp_callback (struct inferior_l
>         pid = linux_wait_for_event (thread->entry.id, &wstat, __WALL);
>       } while (pid > 0 && WIFSTOPPED (wstat));
>
> -  return 0;
> +  /* Let find_inferior return because maybe other lwp in the list will
> +     be deleted by delete_lwp.  */
> +  return 1;
>   }
>
>   static int
> @@ -963,7 +965,9 @@ linux_kill (int pid)
>        first, as PTRACE_KILL will not work otherwise.  */
>     stop_all_lwps (0, NULL);
>
> -  find_inferior (&all_threads, kill_one_lwp_callback , &pid);
> +  /* Keep call kill_one_lwp_callback until find_inferior cannot find any
> +     lwps that is for pid.  */
> +  while (find_inferior (&all_threads, kill_one_lwp_callback , &pid) != NULL);
>
>     /* See the comment in linux_kill_one_lwp.  We did not kill the first
>        thread in the list, so do so now.  */
>
Pedro Alves July 2, 2014, 9:07 a.m. UTC | #2 
On 06/29/2014 04:28 AM, Hui Zhu wrote:
> Hi Pedro,
> 
> This issue is introduced by the patches for PR 12702 that was pushed by you.
> Maybe you could take a look on this patch.

Thanks.  I'll need to think a bit about whether this is the right
solution.

I've added it to the 7.8 regression queue at:

 https://sourceware.org/gdb/wiki/GDB_7.8_Release

I'll look better at it as soon as I have a chance.
diff mbox

Patch

--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -944,7 +944,9 @@  kill_one_lwp_callback (struct inferior_l
       pid = linux_wait_for_event (thread->entry.id, &wstat, __WALL);
     } while (pid > 0 && WIFSTOPPED (wstat));
 
-  return 0;
+  /* Let find_inferior return because maybe other lwp in the list will
+     be deleted by delete_lwp.  */
+  return 1;
 }
 
 static int
@@ -963,7 +965,9 @@  linux_kill (int pid)
      first, as PTRACE_KILL will not work otherwise.  */
   stop_all_lwps (0, NULL);
 
-  find_inferior (&all_threads, kill_one_lwp_callback , &pid);
+  /* Keep call kill_one_lwp_callback until find_inferior cannot find any
+     lwps that is for pid.  */
+  while (find_inferior (&all_threads, kill_one_lwp_callback , &pid) != NULL);
 
   /* See the comment in linux_kill_one_lwp.  We did not kill the first
      thread in the list, so do so now.  */