From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1D9BSp-0002DH-DB for qemu-devel@nongnu.org; Wed, 09 Mar 2005 19:23:45 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1D9BSa-00027Z-Cw for qemu-devel@nongnu.org; Wed, 09 Mar 2005 19:23:29 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1D9BSa-00023A-0P for qemu-devel@nongnu.org; Wed, 09 Mar 2005 19:23:28 -0500 Received: from [65.74.133.9] (helo=mail.codesourcery.com) by monty-python.gnu.org with esmtp (TLSv1:DES-CBC3-SHA:168) (Exim 4.34) id 1D9B5o-0006qQ-HJ for qemu-devel@nongnu.org; Wed, 09 Mar 2005 18:59:56 -0500 From: Paul Brook MIME-Version: 1.0 Date: Wed, 9 Mar 2005 23:59:53 +0000 Content-Type: Multipart/Mixed; boundary="Boundary-00=_534LCyAn3ikmFjx" Message-Id: <200503092359.53765.paul@codesourcery.com> Subject: [Qemu-devel] [patch] Missing FORCE_RET on store ops 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 --Boundary-00=_534LCyAn3ikmFjx Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline It is quite common for the arm function prologue and return sequence to be a single instruction. This combined with their conditional execution capabilities makes them particularly sensitive to missing FORCE_RET() markers. The i386 store-to-memory macro in softmmu-header.h ends in an if..else block, so is triggers this problem. Attached patch adds the necessary FORCE_RET markers to store operations. I can't put the FORCE_RET in softmmu-header.h because that file is used elsewhere. In case anyone is interested I used the following commands to check for ops with multiple exit points: objdump -dr op.o | \ sed -e '/>:$\|ldmdb/!d'-e 's/.*<\(.*\)>:/~\1:/' -e 's/.*ldmdb.*/!/' | \ sed -e ':1;N;s/\n//;t1' | sed -e's/~/\n/g' | grep '!!' Where "ldmdb" is the arm return instruction. This triggers in one other place, but I've verified that this is a false alarm (ldmdb also has other uses). Paul --Boundary-00=_534LCyAn3ikmFjx Content-Type: text/x-diff; charset="us-ascii"; name="patch.qemu_arm_forceret" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="patch.qemu_arm_forceret" Index: target-i386/op.c =================================================================== RCS file: /cvsroot/qemu/qemu/target-i386/op.c,v retrieving revision 1.33 diff -u -p -r1.33 op.c --- target-i386/op.c 3 Mar 2005 01:14:55 -0000 1.33 +++ target-i386/op.c 9 Mar 2005 23:35:11 -0000 @@ -1842,11 +1842,13 @@ void OPPROTO op_fsts_ST0_A0(void) #else stfl(A0, (float)ST0); #endif + FORCE_RET(); } void OPPROTO op_fstl_ST0_A0(void) { stfq(A0, (double)ST0); + FORCE_RET(); } void OPPROTO op_fstt_ST0_A0(void) @@ -1868,6 +1870,7 @@ void OPPROTO op_fist_ST0_A0(void) if (val != (int16_t)val) val = -32768; stw(A0, val); + FORCE_RET(); } void OPPROTO op_fistl_ST0_A0(void) @@ -1882,6 +1885,7 @@ void OPPROTO op_fistl_ST0_A0(void) d = ST0; val = lrint(d); stl(A0, val); + FORCE_RET(); } void OPPROTO op_fistll_ST0_A0(void) @@ -1896,6 +1900,7 @@ void OPPROTO op_fistll_ST0_A0(void) d = ST0; val = llrint(d); stq(A0, val); + FORCE_RET(); } void OPPROTO op_fbld_ST0_A0(void) @@ -2228,6 +2233,7 @@ void OPPROTO op_fnstsw_A0(void) int fpus; fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11; stw(A0, fpus); + FORCE_RET(); } void OPPROTO op_fnstsw_EAX(void) @@ -2240,6 +2246,7 @@ void OPPROTO op_fnstsw_EAX(void) void OPPROTO op_fnstcw_A0(void) { stw(A0, env->fpuc); + FORCE_RET(); } void OPPROTO op_fldcw_A0(void) Index: target-i386/ops_mem.h =================================================================== RCS file: /cvsroot/qemu/qemu/target-i386/ops_mem.h,v retrieving revision 1.5 diff -u -p -r1.5 ops_mem.h --- target-i386/ops_mem.h 8 Jan 2005 18:58:29 -0000 1.5 +++ target-i386/ops_mem.h 9 Mar 2005 23:35:11 -0000 @@ -51,33 +51,39 @@ void OPPROTO glue(glue(op_ldl, MEMSUFFIX void OPPROTO glue(glue(op_stb, MEMSUFFIX), _T0_A0)(void) { glue(stb, MEMSUFFIX)(A0, T0); + FORCE_RET(); } void OPPROTO glue(glue(op_stw, MEMSUFFIX), _T0_A0)(void) { glue(stw, MEMSUFFIX)(A0, T0); + FORCE_RET(); } void OPPROTO glue(glue(op_stl, MEMSUFFIX), _T0_A0)(void) { glue(stl, MEMSUFFIX)(A0, T0); + FORCE_RET(); } #if 0 void OPPROTO glue(glue(op_stb, MEMSUFFIX), _T1_A0)(void) { glue(stb, MEMSUFFIX)(A0, T1); + FORCE_RET(); } #endif void OPPROTO glue(glue(op_stw, MEMSUFFIX), _T1_A0)(void) { glue(stw, MEMSUFFIX)(A0, T1); + FORCE_RET(); } void OPPROTO glue(glue(op_stl, MEMSUFFIX), _T1_A0)(void) { glue(stl, MEMSUFFIX)(A0, T1); + FORCE_RET(); } /* SSE/MMX support */ @@ -93,6 +99,7 @@ void OPPROTO glue(glue(op_stq, MEMSUFFIX uint64_t *p; p = (uint64_t *)((char *)env + PARAM1); glue(stq, MEMSUFFIX)(A0, *p); + FORCE_RET(); } void OPPROTO glue(glue(op_ldo, MEMSUFFIX), _env_A0)(void) @@ -109,6 +116,7 @@ void OPPROTO glue(glue(op_sto, MEMSUFFIX p = (XMMReg *)((char *)env + PARAM1); glue(stq, MEMSUFFIX)(A0, p->XMM_Q(0)); glue(stq, MEMSUFFIX)(A0 + 8, p->XMM_Q(1)); + FORCE_RET(); } #ifdef TARGET_X86_64 @@ -135,11 +143,13 @@ void OPPROTO glue(glue(op_ldq, MEMSUFFIX void OPPROTO glue(glue(op_stq, MEMSUFFIX), _T0_A0)(void) { glue(stq, MEMSUFFIX)(A0, T0); + FORCE_RET(); } void OPPROTO glue(glue(op_stq, MEMSUFFIX), _T1_A0)(void) { glue(stq, MEMSUFFIX)(A0, T1); + FORCE_RET(); } #endif --Boundary-00=_534LCyAn3ikmFjx--