Re: [Qemu-devel] Crash due to invalid env->current_tb

[Adam Lackorzynski <adam@os.inf.tu-dresden.de>]

On Thu May 01, 2008 at 18:02:46 +0300, Blue Swirl wrote:
> On 5/1/08, Adam Lackorzynski <adam@os.inf.tu-dresden.de> wrote:
> >  For 64bit target T0 is 64bits so "=a" does not work and "=A" is needed.
> >  The strange thing is that I need to throw away the upper 32bits because
> >  otherwise it won't work. gen_func is defined to return just long but T0
> >  is unsigned long long, this seems inconsistent. The 'and' does not
> >  appear in 32bit targets so it does not harm there.
> 
> This is because in this special case, T0 is not used as target CPU
> temporary, but instead to return next TB address. On i386 this is 32
> bits, so only EAX is needed. TCG does not touch EDX, so it contains
> garbage. This also means that moving EDX to high word of T0 and then
> throwing the high word away may be slightly wasteful.

So I played a bit more with this by trying out the 'and' and the tmp
variable approaches. With the tmp variables the generated code looks ok
whereas with the 'and' approach it looks especially scary with gcc-4.3
(gcc-3.4 looks ok). I have two versions now, one condensed and ugly and
then one with separate parts for 32 and 64 targets. I think this one
should be prefered.


Index: cpu-exec.c
===================================================================
--- cpu-exec.c	(revision 4291)
+++ cpu-exec.c	(working copy)
@@ -690,7 +691,25 @@
 		fp.ip = tc_ptr;
 		fp.gp = code_gen_buffer + 2 * (1 << 20);
 		(*(void (*)(void)) &fp)();
+#elif defined(__i386)
+#if (TARGET_LONG_BITS == 32)
+		asm volatile ("push %%ebp\n"
+		              "call *%1\n"
+			      "pop %%ebp\n"
+		              : "=a" (T0)
+			      : "a" (gen_func)
+			      : "ecx", "esi", "edi", "edx", "cc");
 #else
+		unsigned long tmp;
+		asm volatile ("push %%ebp\n"
+		              "call *%1\n"
+			      "pop %%ebp\n"
+		              : "=a" (tmp)
+			      : "a" (gen_func)
+			      : "ebx", "ecx", "esi", "edi", "edx", "cc");
+		T0 = tmp;
+#endif
+#else
                 T0 = gen_func();
 #endif
                 env->current_tb = NULL;



Index: cpu-exec.c
===================================================================
--- cpu-exec.c	(revision 4291)
+++ cpu-exec.c	(working copy)
@@ -690,7 +691,31 @@
 		fp.ip = tc_ptr;
 		fp.gp = code_gen_buffer + 2 * (1 << 20);
 		(*(void (*)(void)) &fp)();
+#elif defined(__i386)
+#if (TARGET_LONG_BITS == 32)
+#define CLOBBER
+#define OUTPUT T0
+#define OP
+#define OUTPUT2
 #else
+#define CLOBBER  ,"ebx"
+#define OUTPUT   *((unsigned long *)&T0)
+#define OUTPUT2  , [upperT0] "=m" (*((unsigned long *)&T0 + 1))
+#define OP       "movl $0, %[upperT0]\n"
+#endif
+		asm volatile ("push %%ebp\n"
+		              "call *%[func]\n"
+			      "pop %%ebp\n"
+			      OP
+		              : "=a" (OUTPUT) OUTPUT2
+			      : [func] "a" (gen_func)
+			      : "ecx", "esi", "edi", "edx", "cc" CLOBBER
+			      );
+#undef CLOBBER
+#undef OUTPUT
+#undef OUTPUT2
+#undef OP
+#else
                 T0 = gen_func();
 #endif
                 env->current_tb = NULL;



Adam
-- 
Adam                 adam@os.inf.tu-dresden.de
  Lackorzynski         http://os.inf.tu-dresden.de/~adam/