* [Qemu-devel] [6095] User-mode GDB stub improvements - handle fork
@ 2008-12-18 22:44 Aurelien Jarno
2009-01-06 16:19 ` Martin Mohring
0 siblings, 1 reply; 4+ messages in thread
From: Aurelien Jarno @ 2008-12-18 22:44 UTC (permalink / raw)
To: qemu-devel
Revision: 6095
http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6095
Author: aurel32
Date: 2008-12-18 22:44:04 +0000 (Thu, 18 Dec 2008)
Log Message:
-----------
User-mode GDB stub improvements - handle fork
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>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Modified Paths:
--------------
trunk/gdbstub.c
trunk/gdbstub.h
trunk/linux-user/main.c
trunk/linux-user/syscall.c
Modified: trunk/gdbstub.c
===================================================================
--- trunk/gdbstub.c 2008-12-18 22:43:56 UTC (rev 6094)
+++ trunk/gdbstub.c 2008-12-18 22:44:04 UTC (rev 6095)
@@ -1996,6 +1996,18 @@
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)
{
Modified: trunk/gdbstub.h
===================================================================
--- trunk/gdbstub.h 2008-12-18 22:43:56 UTC (rev 6094)
+++ trunk/gdbstub.h 2008-12-18 22:44:04 UTC (rev 6095)
@@ -13,6 +13,7 @@
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
Modified: trunk/linux-user/main.c
===================================================================
--- trunk/linux-user/main.c 2008-12-18 22:43:56 UTC (rev 6094)
+++ trunk/linux-user/main.c 2008-12-18 22:44:04 UTC (rev 6095)
@@ -162,6 +162,7 @@
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_end(int child)
{
+ if (child) {
+ gdbserver_fork(thread_env);
+ }
}
#endif
Modified: trunk/linux-user/syscall.c
===================================================================
--- trunk/linux-user/syscall.c 2008-12-18 22:43:56 UTC (rev 6094)
+++ trunk/linux-user/syscall.c 2008-12-18 22:44:04 UTC (rev 6095)
@@ -2960,17 +2960,17 @@
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)
@@ -2979,14 +2979,10 @@
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [6095] User-mode GDB stub improvements - handle fork
2008-12-18 22:44 [Qemu-devel] [6095] User-mode GDB stub improvements - handle fork Aurelien Jarno
@ 2009-01-06 16:19 ` Martin Mohring
2009-01-06 17:27 ` Laurent Desnogues
2009-01-06 19:08 ` Martin Mohring
0 siblings, 2 replies; 4+ messages in thread
From: Martin Mohring @ 2009-01-06 16:19 UTC (permalink / raw)
To: qemu-devel
I seem to have found the cause, why ARM user mode with nptl does not
work as before. see below.
Martin
Aurelien Jarno wrote:
> Revision: 6095
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6095
> Author: aurel32
> Date: 2008-12-18 22:44:04 +0000 (Thu, 18 Dec 2008)
>
> Log Message:
> -----------
> User-mode GDB stub improvements - handle fork
>
> 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>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>
> Modified Paths:
> --------------
> trunk/gdbstub.c
> trunk/gdbstub.h
> trunk/linux-user/main.c
> trunk/linux-user/syscall.c
>
> Modified: trunk/gdbstub.c
> ===================================================================
> --- trunk/gdbstub.c 2008-12-18 22:43:56 UTC (rev 6094)
> +++ trunk/gdbstub.c 2008-12-18 22:44:04 UTC (rev 6095)
> @@ -1996,6 +1996,18 @@
> 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);
> +}
>
In case of USE_NPTL is defined, it seems that either s or env can be
NULL, so I get a exception (yes, nptl works on arm in user mode).
My understanding is that this might be a race condition of the first
fork() call coming even before env or gdbserver_state is initialised
towards !NULL.
> #else
> static int gdb_chr_can_receive(void *opaque)
> {
>
> Modified: trunk/gdbstub.h
> ===================================================================
> --- trunk/gdbstub.h 2008-12-18 22:43:56 UTC (rev 6094)
> +++ trunk/gdbstub.h 2008-12-18 22:44:04 UTC (rev 6095)
> @@ -13,6 +13,7 @@
> 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
>
> Modified: trunk/linux-user/main.c
> ===================================================================
> --- trunk/linux-user/main.c 2008-12-18 22:43:56 UTC (rev 6094)
> +++ trunk/linux-user/main.c 2008-12-18 22:44:04 UTC (rev 6095)
> @@ -162,6 +162,7 @@
> pthread_cond_init(&exclusive_cond, NULL);
> pthread_cond_init(&exclusive_resume, NULL);
> pthread_mutex_init(&tb_lock, NULL);
> + gdbserver_fork(thread_env);
>
This gdbserver_fork(thread_env) call then causes the exception.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [6095] User-mode GDB stub improvements - handle fork
2009-01-06 16:19 ` Martin Mohring
@ 2009-01-06 17:27 ` Laurent Desnogues
2009-01-06 19:08 ` Martin Mohring
1 sibling, 0 replies; 4+ messages in thread
From: Laurent Desnogues @ 2009-01-06 17:27 UTC (permalink / raw)
To: qemu-devel
On Tue, Jan 6, 2009 at 5:19 PM, Martin Mohring
<martin.mohring@opensuse.org> wrote:
[...]
> Aurelien Jarno wrote:
>> Revision: 6095
>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6095
>> Author: aurel32
>> Date: 2008-12-18 22:44:04 +0000 (Thu, 18 Dec 2008)
>>
>> Log Message:
>> -----------
>> User-mode GDB stub improvements - handle fork
>>
>> 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>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>
>> Modified Paths:
>> --------------
>> trunk/gdbstub.c
>> trunk/gdbstub.h
>> trunk/linux-user/main.c
>> trunk/linux-user/syscall.c
>>
>> Modified: trunk/gdbstub.c
>> ===================================================================
>> --- trunk/gdbstub.c 2008-12-18 22:43:56 UTC (rev 6094)
>> +++ trunk/gdbstub.c 2008-12-18 22:44:04 UTC (rev 6095)
>> @@ -1996,6 +1996,18 @@
>> gdb_accept();
>> return 0;
>> }
>> +
>> +/* Disable gdb stub for child processes. */
>> +void gdbserver_fork(CPUState *env)
>> +{
>> + GDBState *s = gdbserver_state;
>> + if (s->fd < 0)
Other qemu gdb functions do this:
if (gdbserver_fd < 0 || s->fd < 0)
>> + return;
>> + close(s->fd);
>> + s->fd = -1;
>> + cpu_breakpoint_remove_all(env, BP_GDB);
>> + cpu_watchpoint_remove_all(env, BP_GDB);
>> +}
>>
> In case of USE_NPTL is defined, it seems that either s or env can be
> NULL, so I get a exception (yes, nptl works on arm in user mode).
> My understanding is that this might be a race condition of the first
> fork() call coming even before env or gdbserver_state is initialised
> towards !NULL.
Could you check if my proposed change above fixes your issue?
Laurent
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [6095] User-mode GDB stub improvements - handle fork
2009-01-06 16:19 ` Martin Mohring
2009-01-06 17:27 ` Laurent Desnogues
@ 2009-01-06 19:08 ` Martin Mohring
1 sibling, 0 replies; 4+ messages in thread
From: Martin Mohring @ 2009-01-06 19:08 UTC (permalink / raw)
To: qemu-devel
Martin Mohring wrote:
> I seem to have found the cause, why ARM user mode with nptl does not
> work as before. see below.
>
> Martin
>
> Aurelien Jarno wrote:
>
>> Revision: 6095
>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6095
>> Author: aurel32
>> Date: 2008-12-18 22:44:04 +0000 (Thu, 18 Dec 2008)
>>
>> Log Message:
>> -----------
>> User-mode GDB stub improvements - handle fork
>>
>> 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>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>
>> Modified Paths:
>> --------------
>> trunk/gdbstub.c
>> trunk/gdbstub.h
>> trunk/linux-user/main.c
>> trunk/linux-user/syscall.c
>>
>> Modified: trunk/gdbstub.c
>> ===================================================================
>> --- trunk/gdbstub.c 2008-12-18 22:43:56 UTC (rev 6094)
>> +++ trunk/gdbstub.c 2008-12-18 22:44:04 UTC (rev 6095)
>> @@ -1996,6 +1996,18 @@
>> 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);
>> +}
>>
>>
> In case of USE_NPTL is defined, it seems that either s or env can be
> NULL, so I get a exception (yes, nptl works on arm in user mode).
> My understanding is that this might be a race condition of the first
> fork() call coming even before env or gdbserver_state is initialised
> towards !NULL.
>
The reason why I wrote this and did not propose a fix like Laurent did
now was the fact that:
- gdbserver_fork() is called unconditionally before gdbserver_start()
has ever been called
- gdbserver_fork() as result accesses gdbserver data structures before
they are inited, so we have NULL pointer access
Another issue is that gdbserver_fork() is only defined when
CONFIG_USER_ONLY is not defined but gdbserver_fork() is called when
always when USE_NPTL is defined.
So switching on CONFIG_USER_ONLY and USE_NPTL will result in
gdbserver_fork() beeing undefined. That was why my impression was that
the code is somewhat incomplete.
Also, does debugging work in user mode at all if USE_NPTL is on? If not
I would remove the call to gdbserver_fork() in this case completely.
Martin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-01-06 19:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-18 22:44 [Qemu-devel] [6095] User-mode GDB stub improvements - handle fork Aurelien Jarno
2009-01-06 16:19 ` Martin Mohring
2009-01-06 17:27 ` Laurent Desnogues
2009-01-06 19:08 ` Martin Mohring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.