From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JrJ2h-0005p9-Jy for qemu-devel@nongnu.org; Wed, 30 Apr 2008 16:36:43 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JrJ2h-0005oo-61 for qemu-devel@nongnu.org; Wed, 30 Apr 2008 16:36:43 -0400 Received: from [199.232.76.173] (port=33745 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JrJ2g-0005ol-VT for qemu-devel@nongnu.org; Wed, 30 Apr 2008 16:36:43 -0400 Received: from os.inf.tu-dresden.de ([141.76.48.99]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JrJ2g-00073k-CX for qemu-devel@nongnu.org; Wed, 30 Apr 2008 16:36:42 -0400 Received: from erwin.inf.tu-dresden.de ([141.76.48.80] helo=os.inf.tu-dresden.de) by os.inf.tu-dresden.de with esmtps (TLSv1:AES128-SHA:128) (Exim 4.69) id 1JrJ2d-0007X6-1Q for qemu-devel@nongnu.org; Wed, 30 Apr 2008 22:36:40 +0200 Date: Wed, 30 Apr 2008 22:36:36 +0200 From: Adam Lackorzynski Subject: Re: [Qemu-devel] Crash due to invalid env->current_tb Message-ID: <20080430203636.GC8164@os.inf.tu-dresden.de> References: <20080429115614.GA15524@os.inf.tu-dresden.de> <20080429184011.GK17356@os.inf.tu-dresden.de> <20080430151132.GB6712@os.inf.tu-dresden.de> <1209569284.4312.35.camel@frecb07144> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <1209569284.4312.35.camel@frecb07144> 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 On Wed Apr 30, 2008 at 17:28:04 +0200, Laurent Vivier wrote: >=20 > Le mercredi 30 avril 2008 =E0 17:11 +0200, Adam Lackorzynski a =E9crit : > > On Wed Apr 30, 2008 at 11:08:46 +0200, Alexander Graf wrote: > > > > > > On Apr 29, 2008, at 8:40 PM, Adam Lackorzynski wrote: > > > > > >> > > >> On Tue Apr 29, 2008 at 20:09:00 +0300, Blue Swirl wrote: > > >>> On 4/29/08, Adam Lackorzynski wrote: > > >>>> Hi, > > >>>> > > >>>> I've been experiencing crashes of latest svn Qemu, host ia32 and = =20 > > >>>> target > > >>>> arm, host gcc is 'gcc version 3.4.6 (Debian 3.4.6-7)'. > > >>>> The segfault happens because of an invalid env->current_tb which = =20 > > >>>> seems > > >>>> to be caused by generated code. The following code in cpu_exec > > >>>> > > >>>> tc_ptr =3D tb->tc_ptr; > > >>>> env->current_tb =3D tb; > > >>>> gen_func =3D (void *)tc_ptr; > > >>>> T0 =3D gen_func(); > > >>>> env->current_tb =3D NULL; > > >>>> > > >>>> is being compiled to the following > > >>>> > > >>>> mov 0x14(%ecx),%eax > > >>>> mov %ecx,0x56c(%ebp) > > >>>> xor %edi,%edi > > >>>> call *%eax > > >>>> mov %edi,0x56c(%ebp) > > >>>> > > >>>> After the call edi isn't 0 anymore and gets the bogus value. As = =20 > > >>>> edi is > > >>>> callee saved the code itself seems ok. > > >>>> When I add a barrier before "env->current_tb =3D NULL" the xor is = =20 > > >>>> placed > > >>>> after the call and everything works fine. So might the problem be = =20 > > >>>> that > > >>>> generated code isn't preserving edi/registers? > > >>> > > >>> Right. How did you make the barrier? My version (attached) just > > >>> crashes, I'm not fluent on i386 assembly. Maybe your version could > > >>> serve as a temporary fix. > > >> > > >> I just added an 'asm volatile("")' to stop reordering of instructions > > >> which of course isn't enough. The following works for me: > > >> > > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > >> --- cpu-exec.c (revision 4276) > > >> +++ cpu-exec.c (working copy) > > >> @@ -690,6 +691,11 @@ > > >> fp.ip =3D tc_ptr; > > >> fp.gp =3D code_gen_buffer + 2 * (1 << 20); > > >> (*(void (*)(void)) &fp)(); > > >> +#elif defined(__i386) > > >> + asm volatile ("call *%1\n" > > >> + : "=3Da" (T0) > > >> + : "r" (gen_func) > > >> + : "esi", "edi"); > > >> #else > > >> T0 =3D gen_func(); > > >> #endif > > > > > > There was a comment from Fabrice on how to do prologues in TCG to sav= e /=20 > > > restore the clobbered values. Btw, ebx gets clobbered as well. > >=20 > > tcg/README says that some registers are clobbered. So something like > > this should be safe: > >=20 > > Index: cpu-exec.c > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > --- cpu-exec.c (revision 4276) > > +++ cpu-exec.c (working copy) > > @@ -690,6 +691,15 @@ > > fp.ip =3D tc_ptr; > > fp.gp =3D code_gen_buffer + 2 * (1 << 20); > > (*(void (*)(void)) &fp)(); > > +#elif defined(__i386) > > + asm volatile ("push %%ebp\n" > > + "push %%ebx\n" > > + "call *%1\n" > > + "pop %%ebx\n" > > + "pop %%ebp\n" > > + : "=3Da" (T0) > > + : "r" (gen_func) > > + : "esi", "edi", "ecx", "edx"); >=20 > Why don't you add ebp and ebx in the clobbered registers list (like > "esi", "edi", "ecx", "edx") ? For ebp it's more safe to use push as it depends whether the binary is compiled with frame-pointer or without. When without you can put it into the clobber list, when with you should not, we had some bad experience with this (also see gcc bugzilla #11807). T0 is register defined to ebx, so it's not needed (the push/pop is also not needed), and also it does not work, gcc complains. Adam --=20 Adam adam@os.inf.tu-dresden.de Lackorzynski http://os.inf.tu-dresden.de/~adam/