From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LBU6O-0002sw-Qs for qemu-devel@nongnu.org; Sat, 13 Dec 2008 08:00:12 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LBU6N-0002rz-HL for qemu-devel@nongnu.org; Sat, 13 Dec 2008 08:00:12 -0500 Received: from [199.232.76.173] (port=45529 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LBU6N-0002rW-8B for qemu-devel@nongnu.org; Sat, 13 Dec 2008 08:00:11 -0500 Received: from fmmailgate03.web.de ([217.72.192.234]:39147) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LBU6M-0003aZ-Cc for qemu-devel@nongnu.org; Sat, 13 Dec 2008 08:00:10 -0500 Received: from smtp05.web.de (fmsmtp05.dlan.cinetic.de [172.20.4.166]) by fmmailgate03.web.de (Postfix) with ESMTP id 90E41F560EA6 for ; Sat, 13 Dec 2008 14:00:05 +0100 (CET) Received: from [88.64.11.10] (helo=[192.168.1.198]) by smtp05.web.de with asmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.109 #226) id 1LBU6H-00039a-00 for qemu-devel@nongnu.org; Sat, 13 Dec 2008 14:00:05 +0100 Message-ID: <4943B1B6.9010707@web.de> Date: Sat, 13 Dec 2008 13:59:34 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1229125944.3898.39.camel@cocoduo.atr> <1229126410.3898.42.camel@cocoduo.atr> <49438B8B.8050709@web.de> <1229171501.3898.53.camel@cocoduo.atr> In-Reply-To: <1229171501.3898.53.camel@cocoduo.atr> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig149EE00DB6C8623291191DCC" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [linux-user] Fixed Qemu crash using Gdbstub 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 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig149EE00DB6C8623291191DCC Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Lionel Landwerlin wrote: > Le samedi 13 d=C3=A9cembre 2008 =C3=A0 11:16 +0100, Jan Kiszka a =C3=A9= crit : >> Lionel Landwerlin wrote: >>> I just forgot to remove 2 printf ... >>> Here the good patch : >>> >>> >>> >>> >>> >From 2b3fe65ea3f2ee8dd3efbb52b66a2f4e53b788ea Mon Sep 17 00:00:00 20= 01 >>> From: Lionel Landwerlin >>> Date: Sat, 13 Dec 2008 00:32:04 +0100 >>> Subject: [PATCH] [linux-user] Fixed Qemu crash using Gdbstub >>> >>> When using gdb with qemu (via gdbstub), if your emulated >>> application is multithreaded and does a segfault then qemu >>> crashes. >>> >>> Qemu crashes because the break/watch points are shared between >>> cpus. The TAILQ structure which handles the list of break/watch= >>> points is copied inside each CPUState structure. When the last >>> breakpoint is removed (this happens on a segfault), it is >>> removed across all cpus but because of the copied TAILQ >>> structure a same breakpoint can be freed N times with N the >>> current number of cpus. >> OK, now I got the problem: user space emulation spawns additional VCPU= s >> to emulate fork. Those VCPUs are cloned via cpu_copy which simply >> duplicates the CPUState of the parent, including the breakpoint and >> watchpoint TAILQ headers. This is doomed to fail. >> >> But your approach to let the cloned VCPU point to the same TAILQ heade= r >> as its parent is not correct as well. It will cause troubles to gdbstu= b >> which manages breakpoints on all VCPUs by adding duplicate instances o= n >> a per-VCPU base. If you inject a breakpoint before a fork and then >> remove it afterwards, gdbstub will report an error because it will onl= y >> find the breakpoint once, not n times (n =3D number of VCPUs). >> >> What you have to do is to cleanly duplicate the breakpoint and >> watchpoint lists on cpu_copy (filter out BP_CPU types for cleanness >> reasons, although they do not occur in user emulation ATM). >=20 > Hello Jan, >=20 > Thanks for reviewing my patch. >=20 > Duplication of all break/watchpoints will makes the patch bigger, > because it will required break/watchpoint_copy functions etc... >=20 > Another problem is that threads are also emulated by vcpus in user > emulation. But we also need to share break/watchpoints between threads.= > This explain the way my patch do the thing. >=20 > Finally, this makes the modification a lot more complicated than what I= > expected, because breakpoints on emulated forks should not apply. Sorry, but shouldn't we prefer correct solutions over simpler but broken ones...? Before my gdbstub changes, break/watchpoints were per-VCPU and automatically duplicated on cpu_copy (as they were stored in a static array inside CPUState). Now they are kept in lists, but still per-VCPU. All that has to be done now is to fix cpu_copy to take this into account. If that takes additional simple helpers to clone breakpoints, so what? Jan --------------enig149EE00DB6C8623291191DCC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAklDsbkACgkQniDOoMHTA+mD8ACfXJt0roZfjFvE5/8nR1Jb+zTe WF0Anjo/x3S1/fsZVRdYj45QhYJPwKTK =4/mm -----END PGP SIGNATURE----- --------------enig149EE00DB6C8623291191DCC--