From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LBwjQ-0004BP-KZ for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:34:24 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LBwjP-0004B6-Vj for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:34:24 -0500 Received: from [199.232.76.173] (port=50003 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LBwjP-0004B1-Qf for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:34:23 -0500 Received: from soufre.accelance.net ([213.162.48.15]:60732) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LBwjP-0001FV-5Y for qemu-devel@nongnu.org; Sun, 14 Dec 2008 14:34:23 -0500 Received: from [192.168.0.3] (potipota.net [88.168.176.51]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by soufre.accelance.net (Postfix) with ESMTP id C2FAD45028 for ; Sun, 14 Dec 2008 20:34:21 +0100 (CET) Subject: Re: [Qemu-devel] Re: [linux-user] Fixed Qemu crash using Gdbstub From: Lionel Landwerlin In-Reply-To: <49451588.2070708@web.de> References: <1229125944.3898.39.camel@cocoduo.atr> <1229126410.3898.42.camel@cocoduo.atr> <49438B8B.8050709@web.de> <1229171501.3898.53.camel@cocoduo.atr> <4943B1B6.9010707@web.de> <1229174473.3898.61.camel@cocoduo.atr> <4943BD66.60109@web.de> <1229189833.3898.69.camel@cocoduo.atr> <49451588.2070708@web.de> Content-Type: text/plain; charset=UTF-8 Date: Sun, 14 Dec 2008 20:34:17 +0100 Message-Id: <1229283257.3898.91.camel@cocoduo.atr> Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Le dimanche 14 d=C3=A9cembre 2008 =C3=A0 15:17 +0100, Jan Kiszka a =C3=A9= crit : > Lionel Landwerlin wrote: > > Le samedi 13 d=C3=A9cembre 2008 =C3=A0 14:49 +0100, Jan Kiszka a =C3=A9= crit : > >> Lionel Landwerlin wrote: > >> Subject: [PATCH] Adopt cpu_copy to new breakpoint API > >> > >> Latest changes to the cpu_breakpoint/watchpoint API broke cpu_copy. = This > >> patch fixes it by cloning the breakpoint and watchpoint lists > >> appropriately. > >> > >> Thanks to Lionel Landwerlin for pointing out. > >> > >> Signed-off-by: Jan Kiszka > >> --- > >> > >> exec.c | 24 +++++++++++++++++++++++- > >> 1 files changed, 23 insertions(+), 1 deletions(-) > >> > >> diff --git a/exec.c b/exec.c > >> index 44f6a42..193a43c 100644 > >> --- a/exec.c > >> +++ b/exec.c > >> @@ -1654,12 +1654,34 @@ void cpu_abort(CPUState *env, const char *fm= t, ...) > >> CPUState *cpu_copy(CPUState *env) > >> { > >> CPUState *new_env =3D cpu_init(env->cpu_model_str); > >> - /* preserve chaining and index */ > >> CPUState *next_cpu =3D new_env->next_cpu; > >> int cpu_index =3D new_env->cpu_index; > >> +#if defined(TARGET_HAS_ICE) > >> + CPUBreakpoint *bp; > >> + CPUWatchpoint *wp; > >> +#endif > >> + > >> memcpy(new_env, env, sizeof(CPUState)); > >> + > >> + /* Preserve chaining and index. */ > >> new_env->next_cpu =3D next_cpu; > >> new_env->cpu_index =3D cpu_index; > >> + > >> + /* Clone all break/watchpoints. > >> + Note: Once we support ptrace with hw-debug register access, = make sure > >> + BP_CPU break/watchpoints are handled correctly on clone. */ > >> + TAILQ_INIT(&env->breakpoints); > >> + TAILQ_INIT(&env->watchpoints); > >> +#if defined(TARGET_HAS_ICE) > >> + TAILQ_FOREACH(bp, &env->breakpoints, entry) { > >> + cpu_breakpoint_insert(new_env, bp->pc, bp->flags, NULL); > >> + } > >> + TAILQ_FOREACH(wp, &env->watchpoints, entry) { > >> + cpu_watchpoint_insert(new_env, wp->vaddr, (~wp->len_mask) += 1, > >> + wp->flags, NULL); > >> + } > >> +#endif > >> + > >> return new_env; > >> } > >> =20 > >> > >=20 > > Jan, > >=20 > > Well the patch seems pretty better as qemu does not crash anymore :) > > There might be other problems, because gdbstub doesn't stop where I k= now > > it should. I'm investigating... >=20 > OK. If you have a testcase, I would also look into this next week. >=20 > >=20 > > You might want to add this patch too, there is something strange with > > TAILQ 'first' structure member. It's not updated on deletion of > > all/first elements. > >=20 > > Regards, > >=20 > >>From 78ba0dbf0c9e5d73022fecdbf1869274b8224949 Mon Sep 17 00:00:00 200= 1 > > From: Lionel Landwerlin > > Date: Sat, 13 Dec 2008 14:05:18 +0100 > > Subject: [PATCH] Fix suspicious TAILQ management > >=20 > > TAILQ first pointer is not updated when the last element is > > removed. > > --- > > sys-queue.h | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > >=20 > > diff --git a/sys-queue.h b/sys-queue.h > > index ad5c8fb..37bedde 100644 > > --- a/sys-queue.h > > +++ b/sys-queue.h > > @@ -202,7 +202,8 @@ struct { = \ > > (elm)->field.tqe_prev; = \ > > else = \ > > (head)->tqh_last =3D (elm)->field.tqe_prev; = \ > > - *(elm)->field.tqe_prev =3D (elm)->field.tqe_next; = \ > > + if ((head)->tqh_first =3D=3D (elm)) = \ > > + (head)->tqh_first =3D (elm)->field.tqe_next; = \ >=20 > That's fishy. The elm's prev field should point to the head, thus the > head should be updated to elm's next (ie. NULL). Could you dig deeper > what the state of all involved structures are and maybe track down when > they become inconsistent? Alternatively, please provide a testcase. In fact when you're not using gdbstub, there is no break/watch points. So this problem never appears. And when you're using gdbstub, this problem only appears, when all break/watch points are removed (ie when a SIGSEGV is raised). In this case, you're almost everytime restart qemu anytime soon, so break/watch points (and first member of the TAILQ lists) are not used anymore. My current test case is a piece a proprietary software, so I don't think I will be able to give it to you, but I can probably rewrite something simple using ~10/15 threads. I'm testing on sh4 binary, do you think you need a sh4 root filesystem to reproduce the problem ? --=20 =EF=BB=BFLione Landwerlin =20 =EF=BB=BF O p e n W i d e 14, rue Gaillon 75002 Paris