[RFA] gdbserver with reversed arguments seg. fault

[RFA] gdbserver with reversed arguments seg. fault

[Denis PILAT]

Hi,

Following our discussion in the thread 
http://sources.redhat.com/ml/gdb-patches/2007-01/msg00059.html
I'm proposing the attached patch to fix a segmentation fault that occurs 
into gdbserver when you revert the binary filename with the "host:port" 
argument.
In inferiors.c the inferior_target_data() function is called with a NULL 
pointer argument. At the beginning I though of testing arguments but 
none of them are tested in this whole file so I concluded it's up to the 
caller to do so.

(I hope this time my patch is related to a version nearer from the HEAD :)
Denis
-- 

2007-01-03  Denis Pilat  <denis.pilat@st.com>

    * linux-low.c (linux_kill): handle the null case of all_threads.head.

Index: linux-low.c
===================================================================
--- linux-low.c (revision 545)
+++ linux-low.c (working copy)
@@ -255,9 +255,13 @@ static void
 linux_kill (void)
 {
   struct thread_info *thread = (struct thread_info *) all_threads.head;
-  struct process_info *process = get_thread_process (thread);
+  struct process_info *process;
   int wstat;

+  if (! thread)
+     return;
+
+  process = get_thread_process (thread);
   for_each_inferior (&all_threads, linux_kill_one_process);

   /* See the comment in linux_kill_one_process.  We did not kill the first

Re: [RFA] gdbserver with reversed arguments seg. fault

[Daniel Jacobowitz]

On Wed, Jan 03, 2007 at 10:29:49AM +0100, Denis PILAT wrote:
> Hi,
> 
> Following our discussion in the thread 
> http://sources.redhat.com/ml/gdb-patches/2007-01/msg00059.html
> I'm proposing the attached patch to fix a segmentation fault that occurs 
> into gdbserver when you revert the binary filename with the "host:port" 
> argument.
> In inferiors.c the inferior_target_data() function is called with a NULL 
> pointer argument. At the beginning I though of testing arguments but 
> none of them are tested in this whole file so I concluded it's up to the 
> caller to do so.

What exactly did you do to get this crash?  What's the backtrace look
like?  It didn't happen for me.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] gdbserver with reversed arguments seg. fault

[Denis PILAT]

Daniel Jacobowitz wrote:
> On Wed, Jan 03, 2007 at 10:29:49AM +0100, Denis PILAT wrote:
>   
>> Hi,
>>
>> Following our discussion in the thread 
>> http://sources.redhat.com/ml/gdb-patches/2007-01/msg00059.html
>> I'm proposing the attached patch to fix a segmentation fault that occurs 
>> into gdbserver when you revert the binary filename with the "host:port" 
>> argument.
>> In inferiors.c the inferior_target_data() function is called with a NULL 
>> pointer argument. At the beginning I though of testing arguments but 
>> none of them are tested in this whole file so I concluded it's up to the 
>> caller to do so.
>>     
>
> What exactly did you do to get this crash?  What's the backtrace look
> like?  It didn't happen for me.
>
>   
Here is the output :
  > ./gdbserver ~/tmp/hello/hello.out :2222
  Cannot exec :2222: No such file or directory.
  Process :2222 created; pid = 25945

  Child exited with retcode = 7f
  Could not open remote device: Invalid argument.
  Killing inferior
  Segmentation fault (core dumped)

Here is the backtrace:
  Program received signal SIGSEGV, Segmentation fault.
  inferior_target_data (inferior=0x0)
      at 
/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/gdb/gdb/gdbserver/inferiors.c:227
  227     {
  (gdb) bt
  #0  inferior_target_data (inferior=0x0)
      at 
/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/gdb/gdb/gdbserver/inferiors.c:227
  #1  0x0804e2a9 in main (argc=3, argv=0xbfff9414)
      at 
/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/gdb/gdb/gdbserver/server.c:620

Re: [RFA] gdbserver with reversed arguments seg. fault

[Daniel Jacobowitz]

On Wed, Jan 03, 2007 at 10:29:49AM +0100, Denis PILAT wrote:
> 2007-01-03  Denis Pilat  <denis.pilat@st.com>
> 
>    * linux-low.c (linux_kill): handle the null case of all_threads.head.

Thanks, I see what happened now.  I've committed this version.  Please
avoid format=flowed patches in the future - it was pretty mangled.

-- 
Daniel Jacobowitz
CodeSourcery

2007-01-03  Denis Pilat  <denis.pilat@st.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-low.c (linux_kill): Handle being called with no threads.
	* win32-i386-low.c (win32_kill): Likewise.
	(get_child_debug_event): Clear current_process_handle.

Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.48
diff -u -p -r1.48 linux-low.c
--- linux-low.c	16 Nov 2006 15:08:25 -0000	1.48
+++ linux-low.c	3 Jan 2007 17:53:31 -0000
@@ -255,13 +255,17 @@ static void
 linux_kill (void)
 {
   struct thread_info *thread = (struct thread_info *) all_threads.head;
-  struct process_info *process = get_thread_process (thread);
+  struct process_info *process;
   int wstat;
 
+  if (thread == NULL)
+    return;
+
   for_each_inferior (&all_threads, linux_kill_one_process);
 
   /* See the comment in linux_kill_one_process.  We did not kill the first
      thread in the list, so do so now.  */
+  process = get_thread_process (thread);
   do
     {
       ptrace (PTRACE_KILL, pid_of (process), 0, 0);
Index: win32-i386-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-i386-low.c,v
retrieving revision 1.1
diff -u -p -r1.1 win32-i386-low.c
--- win32-i386-low.c	23 Jul 2006 03:52:15 -0000	1.1
+++ win32-i386-low.c	3 Jan 2007 17:53:31 -0000
@@ -582,6 +582,9 @@ win32_attach (unsigned long pid)
 static void
 win32_kill (void)
 {
+  if (current_process_handle == NULL)
+    return;
+
   TerminateProcess (current_process_handle, 0);
   for (;;)
     {
@@ -901,6 +904,7 @@ in:
       ourstatus->kind = TARGET_WAITKIND_EXITED;
       ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
       CloseHandle (current_process_handle);
+      current_process_handle = NULL;
       retval = main_thread_id;
       break;