diff mbox

[RFA] gdbserver crash in gdb/gdbserver/thread.c::thread_search_callback

Message ID 1450710922-29601-1-git-send-email-brobecker@adacore.com
State New, archived
Headers

Commit Message

Joel Brobecker Dec. 21, 2015, 3:15 p.m. UTC 
  Connecting GDB to a LynxOS-178 GDBserver causes GDBserver to crash:

    % gdbserver :4444 simple_main
    Process simple_main created; pid = 19
    Listening on port 4444
    Remote debugging from host 205.232.38.10
    Segmentation fault (core dumped)

We saw this crash on LynxOS and also when using GDBserver on Windows.

The crash happens in thread_search_callback where the function
calls the_target->thread_stopped (via the thread_stopped macro)
without verifying whether the callback is NULL or not.

For the record, the regression was introduced by:

    commit a67a9faef0e32886c83611cc7a0ba61e91123063
    Date:   Mon Nov 30 16:05:26 2015 +0000
    Subject: gdbserver:prepare_access_memory: pick another thread

This patch avoids the crash by checking the value of the callback
first, before calling it.

gdb/gdbserver/ChangeLog:

        * target.c (thread_search_callback): Add check that
        the thread_stopped target callback is not NULL before
        calling it.

Does the fix look good to you?
Tested on both Windows (native gdbserver) and LynxOS using
AdaCore's gdb-testsuite.

Thanks,

Comments

Pedro Alves Dec. 21, 2015, 4:53 p.m. UTC | #1 
On 12/21/2015 03:15 PM, Joel Brobecker wrote:
> Connecting GDB to a LynxOS-178 GDBserver causes GDBserver to crash:
> 
>     % gdbserver :4444 simple_main
>     Process simple_main created; pid = 19
>     Listening on port 4444
>     Remote debugging from host 205.232.38.10
>     Segmentation fault (core dumped)
> 
> We saw this crash on LynxOS and also when using GDBserver on Windows.
> 
> The crash happens in thread_search_callback where the function
> calls the_target->thread_stopped (via the thread_stopped macro)
> without verifying whether the callback is NULL or not.
> 
> For the record, the regression was introduced by:
> 
>     commit a67a9faef0e32886c83611cc7a0ba61e91123063
>     Date:   Mon Nov 30 16:05:26 2015 +0000
>     Subject: gdbserver:prepare_access_memory: pick another thread
> 

Whoops, sorry about that.

> gdb/gdbserver/ChangeLog:
> 
>         * target.c (thread_search_callback): Add check that
>         the thread_stopped target callback is not NULL before
>         calling it.
> 
> Does the fix look good to you?
> Tested on both Windows (native gdbserver) and LynxOS using
> AdaCore's gdb-testsuite.

LGTM.

Thanks,
Pedro Alves
Joel Brobecker Dec. 22, 2015, 3:27 p.m. UTC | #2 
> > gdb/gdbserver/ChangeLog:
> > 
> >         * target.c (thread_search_callback): Add check that
> >         the thread_stopped target callback is not NULL before
> >         calling it.
> > 
> > Does the fix look good to you?
> > Tested on both Windows (native gdbserver) and LynxOS using
> > AdaCore's gdb-testsuite.
> 
> LGTM.

Thanks, Pedro; pushed!
diff mbox

Patch

diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index b376ce8..59736e5 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -68,7 +68,9 @@  thread_search_callback (struct inferior_list_entry *entry, void *args)
   if (ptid_get_pid (entry->id) == ptid_get_pid (s->current_gen_ptid)
       && mythread_alive (ptid_of (thread)))
     {
-      if (s->stopped == NULL && thread_stopped (thread))
+      if (s->stopped == NULL
+	  && the_target->thread_stopped != NULL
+	  && thread_stopped (thread))
 	s->stopped = thread;
 
       if (s->first == NULL)