qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [patch] Missing FORCE_RET on store ops
@ 2005-03-09 23:59 Paul Brook
  0 siblings, 0 replies; only message in thread
From: Paul Brook @ 2005-03-09 23:59 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]

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

[-- Attachment #2: patch.qemu_arm_forceret --]
[-- Type: text/x-diff, Size: 3353 bytes --]

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
 

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2005-03-10  0:23 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-09 23:59 [Qemu-devel] [patch] Missing FORCE_RET on store ops Paul Brook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).