[Qemu-devel] [1/2] User-mode GDB stub improvements

* [Qemu-devel] [1/2] User-mode GDB stub improvements - handle fork
@ 2008-12-12 19:07 Daniel Jacobowitz
  2008-12-18 22:55 ` Aurelien Jarno
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Jacobowitz @ 2008-12-12 19:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook

From: Daniel Jacobowitz <dan@codesourcery.com>

Close gdbserver in child processes, so that only one stub tries to talk
to GDB at a time.  Updated from an earlier patch by Paul Brook.

Signed-off-by: Daniel Jacobowitz <dan@codesourcery.com>

---

This patch lets GDB debug a parent process, even if it calls fork.
Otherwise you end up with two QEMU processes sharing a single fd
for the GDB socket, and both writing replies to it.  This will more or
less stutter along for a while, with GDB discarding lots of errors,
but eventually the two processes will diverge and things will blow up
depending on which response GDB sees first.

Index: linux-user/syscall.c
===================================================================

--- linux-user/syscall.c	(revision 5995)
+++ linux-user/syscall.c	(working copy)
@@ -2958,17 +2958,17 @@ static int do_fork(CPUState *env, unsign
             return -EINVAL;
         fork_start();
         ret = fork();
-#if defined(USE_NPTL)
-        /* There is a race condition here.  The parent process could
-           theoretically read the TID in the child process before the child
-           tid is set.  This would require using either ptrace
-           (not implemented) or having *_tidptr to point at a shared memory
-           mapping.  We can't repeat the spinlock hack used above because
-           the child process gets its own copy of the lock.  */
         if (ret == 0) {
+            /* Child Process.  */
             cpu_clone_regs(env, newsp);
             fork_end(1);
-            /* Child Process.  */
+#if defined(USE_NPTL)
+            /* There is a race condition here.  The parent process could
+               theoretically read the TID in the child process before the child
+               tid is set.  This would require using either ptrace
+               (not implemented) or having *_tidptr to point at a shared memory
+               mapping.  We can't repeat the spinlock hack used above because
+               the child process gets its own copy of the lock.  */
             if (flags & CLONE_CHILD_SETTID)
                 put_user_u32(gettid(), child_tidptr);
             if (flags & CLONE_PARENT_SETTID)
@@ -2977,14 +2977,10 @@ static int do_fork(CPUState *env, unsign
             if (flags & CLONE_SETTLS)
                 cpu_set_tls (env, newtls);
             /* TODO: Implement CLONE_CHILD_CLEARTID.  */
+#endif
         } else {
             fork_end(0);
         }
-#else
-        if (ret == 0) {
-            cpu_clone_regs(env, newsp);
-        }
-#endif
     }
     return ret;
 }
Index: linux-user/main.c
===================================================================
--- linux-user/main.c	(revision 5995)
+++ linux-user/main.c	(working copy)
@@ -162,6 +162,7 @@ void fork_end(int child)
         pthread_cond_init(&exclusive_cond, NULL);
         pthread_cond_init(&exclusive_resume, NULL);
         pthread_mutex_init(&tb_lock, NULL);
+        gdbserver_fork(thread_env);
     } else {
         pthread_mutex_unlock(&exclusive_lock);
         pthread_mutex_unlock(&tb_lock);
@@ -254,6 +255,9 @@ void fork_start(void)
 
 void fork_end(int child)
 {
+    if (child) {
+        gdbserver_fork(thread_env);
+    }
 }
 #endif
 
Index: gdbstub.c
===================================================================
--- gdbstub.c	(revision 5995)
+++ gdbstub.c	(working copy)
@@ -1996,6 +1996,18 @@ int gdbserver_start(int port)
     gdb_accept();
     return 0;
 }
+
+/* Disable gdb stub for child processes.  */
+void gdbserver_fork(CPUState *env)
+{
+    GDBState *s = gdbserver_state;
+    if (s->fd < 0)
+      return;
+    close(s->fd);
+    s->fd = -1;
+    cpu_breakpoint_remove_all(env, BP_GDB);
+    cpu_watchpoint_remove_all(env, BP_GDB);
+}
 #else
 static int gdb_chr_can_receive(void *opaque)
 {
Index: gdbstub.h
===================================================================
--- gdbstub.h	(revision 5995)
+++ gdbstub.h	(working copy)
@@ -13,6 +13,7 @@ void gdb_set_stop_cpu(CPUState *env);
 int gdb_handlesig (CPUState *, int);
 void gdb_exit(CPUState *, int);
 int gdbserver_start(int);
+void gdbserver_fork(CPUState *);
 #else
 int gdbserver_start(const char *port);
 #endif

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 2+ messages in thread