Missing FORCE_RET on store ops

PATCH New Paul Brook 1 participant 1 message Mar 10, 2005

Share & tools

Paul Brook

author

2005-03-10 00:23 UTC #0 lore raw

PATCH 1/1 Missing FORCE_RET on store ops

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:

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

Index: target-i386/op.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/op.c,v
retrieving revision 1.33

op.c +7 -0

diff -u -p -r1.33 op.c
@@ -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

ops_mem.h +10 -0

diff -u -p -r1.5 ops_mem.h
@@ -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