[PATCH] fix gdbserver crash

[PATCH] fix gdbserver crash

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]

Hi,

gdbserver crashes in this situation:

[gdbserver] ./gdbserver :9999 /home/pedro/gdb/tests/threads_same_loop_100_32 1
[gdbserver] Process /home/pedro/gdb/tests/threads_same_loop_100_32 created; pid = 7773
[gdbserver] Listening on port 9999

[   gdb   ] (gdb) tar extended-remote :9999

[gdbserver] Remote debugging from host 127.0.0.1

[   gdb   ] Remote debugging using :9999
[   gdb   ] 0x00000000f7ef0810 in ?? ()
[   gdb   ] (gdb) kill
[   gdb   ] Kill the program being debugged? (y or n) y

(kill isn't important here, normal exit would be the same)

[gdbserver] Killing inferior

[   gdb   ] (gdb) r
[   gdb   ] Starting program:

[gdbserver] Process /home/pedro/gdb/tests/threads_same_loop_100_32 created; pid = 7799
[gdbserver] Cannot exec /home/pedro/gdb/tests/threads_same_loop_100_32: Bad address.

            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[   gdb   ] Running the default executable on the remote target failed; try "set remote exec-file"?
[   gdb   ] (gdb) set remote exec-file /home/pedro/gdb/tests/threads_same_loop_100_32
[   gdb   ] (gdb) r
[   gdb   ] Starting program:
[   gdb   ] Ignoring packet error, continuing...

[gdbserver] Child exited with retcode = 7f
[gdbserver] *** glibc detected *** /home/pedro/gdb/csl_trunk/build/gdb/gdbserver/gdbserver: double free or corruption (out): 0x00007f25148b3d30 ***
[gdbserver] ======= Backtrace: =========
[gdbserver] <snip>



Program received signal SIGSEGV, Segmentation fault.
0x00007f2514589c6f in abort () from /lib/libc.so.6

(gdb) bt
#0  0x00007f2514589c6f in abort () from /lib/libc.so.6
#1  0x00007f25145c2a7b in __libc_message () from /lib/libc.so.6
#2  0x00007f25145ca08a in _int_free () from /lib/libc.so.6
#3  0x00007f25145cdc1c in free () from /lib/libc.so.6
#4  0x000000000040943e in handle_v_run (
    own_buf=0x61b0f0 "vRun;2f686f6d652f706564726f2f6764622f74657374732f746872656164735f73616d655f6c6f6f705f3130305f3332", status=0x7fff1ccdbf56 "Wv", signal=0x7fff1ccdbf48) at ../../../src/gdb/gdbserver/server.c:1112
#5  0x000000000040963f in handle_v_requests (
    own_buf=0x61b0f0 "vRun;2f686f6d652f706564726f2f6764622f74657374732f746872656164735f73616d655f6c6f6f705f3130305f3332", status=0x7fff1ccdbf56 "Wv", signal=0x7fff1ccdbf48, packet_len=97, new_packet_len=0x7fff1ccdbf20)
    at ../../../src/gdb/gdbserver/server.c:1175
#6  0x000000000040a817 in main (argc=4, argv=0x7fff1ccdc048) at ../../../src/gdb/gdbserver/server.c:1778
(gdb)

The crash comes from this bit missing:

 Index: src/gdb/gdbserver/server.c
 ===================================================================
 --- src.orig/gdb/gdbserver/server.c     2008-10-10 12:11:26.000000000 +0100
 +++ src/gdb/gdbserver/server.c  2008-10-10 12:11:39.000000000 +0100
 @@ -1103,6 +1103,7 @@ handle_v_run (char *own_buf, char *statu
         }
 
        new_argv[0] = strdup (program_argv[0]);
 +      new_argv[1] = NULL;
      }
 
    /* Free the old argv.  */

So on the next vRun, we are trying to free a dangling pointer.  

`exec' had already failed due to that bad pointer before:

[gdbserver] Cannot exec /home/pedro/gdb/tests/threads_same_loop_100_32: Bad address.

The above fixes it, but, we're also ignoring all but argv[0] from the
last run.  Was that intended?

Since I started gdbserver like:

gdbserver :9999 PROG ARG

I was a bit surprised to see that the ARG didn't get passed to
the new inferior too in this case (vRun with set remote exec-file ""),
only PROG was.

The attached fixes the crash and adjusts it so the whole
last argv is passed.  OK?

Or if not OK, is the above patchlet better then?

-- 
Pedro Alves

[-- Attachment #2: vrun_crash.diff --]
[-- Type: text/x-diff, Size: 1549 bytes --]

2008-10-10  Pedro Alves  <pedro@codesourcery.com>

	* server.c (handle_v_run): If GDB didn't specify an argv, use the
	whole argv from the last run, not just argv[0].

---
 gdb/gdbserver/server.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2008-10-10 12:18:02.000000000 +0100
+++ src/gdb/gdbserver/server.c	2008-10-10 12:18:09.000000000 +0100
@@ -1096,23 +1096,31 @@ handle_v_run (char *own_buf, char *statu
 
   if (new_argv[0] == NULL)
     {
+      /* GDB didn't specify a program to run.  Try to use the argv
+	 from the last run: either from the last vRun with a non-empty
+	 argv, or from what the user specified if gdbserver was
+	 started as: `gdbserver :1234 PROG ARGS'.  */
+
       if (program_argv == NULL)
 	{
 	  write_enn (own_buf);
 	  return 0;
 	}
 
-      new_argv[0] = strdup (program_argv[0]);
+      /* We can reuse the old args.  We don't need this then.  */
+      free (new_argv);
     }
-
-  /* Free the old argv.  */
-  if (program_argv)
+  else
     {
-      for (pp = program_argv; *pp != NULL; pp++)
-	free (*pp);
-      free (program_argv);
+      /* Free the old argv.  */
+      if (program_argv)
+	{
+	  for (pp = program_argv; *pp != NULL; pp++)
+	    free (*pp);
+	  free (program_argv);
+	}
+      program_argv = new_argv;
     }
-  program_argv = new_argv;
 
   *signal = start_inferior (program_argv, status);
   if (*status == 'T')

Re: [PATCH] fix gdbserver crash

[Daniel Jacobowitz]

On Fri, Oct 10, 2008 at 12:29:27PM +0100, Pedro Alves wrote:
> 2008-10-10  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* server.c (handle_v_run): If GDB didn't specify an argv, use the
> 	whole argv from the last run, not just argv[0].

This looks right to me.  Thanks!

-- 
Daniel Jacobowitz
CodeSourcery