RE: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

RE: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

[Paul Robinson]

> From: 
> qemu-devel-bounces+paul.robinson=scisys.co.uk@nongnu.org 
> [mailto:qemu-devel-bounces+paul.robinson=scisys.co.uk@nongnu.o
> rg] On Behalf Of Thiemo Seufer
> Sent: 02 February 2007 04:02
> To: Juergen Lock
> Cc: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] Re: strange crash on 
> FreeBSD-current/amd64 (pointertruncation?)
> 
> Juergen Lock wrote:
> > On Wed, Jan 24, 2007 at 09:00:19PM +0100, Juergen Lock wrote:
> > > Hi!
> > > 
> > >  I got a report of qemu segfaulting here on FreeBSD-current/amd64:
> > > 
> > > > #0  main_loop () at 
> /usr/ports-cvs/emulators/qemu/work/qemu-snapshot-2007-01-11_05
> /vl.c:6125
> > > > 6125                    env = env->next_cpu;
> > > > [New Thread 0x801e10190 (LWP 100214)]
> > > > (gdb) print env
> > > > $1 = (CPUX86State *) 0xac10000
> > > > (gdb) print first_cpu
> > > > $2 = (CPUX86State *) 0x80ac10000
> > 
> > Ok Jung-uk Kim found the following fix: (Thanx!)
> > 
> > --- qemu/cpu-exec.c.orig	Wed Jan 31 16:58:03 2007
> > +++ qemu/cpu-exec.c	Wed Jan 31 17:08:11 2007
> > @@ -226,9 +226,9 @@
> >  
> >  int cpu_exec(CPUState *env1)
> >  {
> > -    int saved_T0, saved_T1;
> > +    long saved_T0, saved_T1;
> >  #if defined(reg_T2)
> > -    int saved_T2;
> > +    long saved_T2;
> 
> I used target_ulong instead.
> 
> 
> Thiemo
> 
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
> 
> 

But the T0, T1, and T2 registers are being saved for the benefit of the
host not the target.
A target_ulong is not big enough on a 64 bit host with a 32 bit target.
 

There's another, related problem here.
I've seen it on an x86_64 host with a sparc32 target but it might happen
with other 32 bit targets.

-- dyngen-exec.h --

#define AREG1 "rbx"
#define AREG2 "r12"

-- target-sparc/exec.h --

register uint32_t T0 asm(AREG1);
register uint32_t T1 asm(AREG2);

-- cpu-exec.h --

int cpu_exec(CPUState *env1)
{
	...
	saved_T0 = T0;
	saved_T1 = T1; 
	...
	gen_func();
	...
	T0 = saved_T0; <---<< the upper 32 bits of %rbx are lost :-(
	T1 = saved_T1;
	....
}

The problem is caused by the uint32_t in the definitions of T0, T1, and
T2.
Changing it to uint64_t breaks something else - I don't know what.
A solution that works for me is to add declarations to
target-sparc/exec.h

#if defined __x86_64__
register uint64_t T0_64 asm(AREG1);
register uint64_t T1_64 asm(AREG2);
#endif
#if defined __x86_64__
register uint64_t T2_64 asm(AREG3);
#endif

and then change some lines in cpu-exec.c

	unsigned long saved_T0, saved_T1, saved_T2;

	saved_T0 = T0_64;
	saved_T1 = T1_64;
	saved_T2 = T2_64;
	...

	...
	T2_64 = saved_T2;
	T1_64 = saved_T1;
	T0_64 = saved_T0;
	...
	

Paul R.

RE: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

[Gwenole Beauchesne]

On Fri, 2 Feb 2007, Paul Robinson wrote:

> But the T0, T1, and T2 registers are being saved for the benefit of the
> host not the target.

FWIW, I use the following patch for Virtual Box on x86_64. The proper fix
would be to not globally allocate registers for the whole program but only
for the micro-ops. Then, make the necessary save/restore around the
gen_func call.

--- vbox-1.3.3/src/recompiler/new/cpu-exec.c.64bit-fixes	2007-01-23 10:30:18.000000000 +0100
+++ vbox-1.3.3/src/recompiler/new/cpu-exec.c	2007-02-02 18:16:19.000000000 +0100
@@ -228,7 +228,23 @@ static inline TranslationBlock *tb_find_
 
 int cpu_exec(CPUState *env1)
 {
-    int saved_T0, saved_T1, saved_T2;
+    /* Preserve callee-saved registers */
+#ifdef AREG0
+    register unsigned long reg_AREG0 asm(AREG0);
+    volatile unsigned long saved_AREG0;
+#endif
+#ifdef AREG1
+    register unsigned long reg_AREG1 asm(AREG1);
+    volatile unsigned long saved_AREG1;
+#endif
+#ifdef AREG2
+    register unsigned long reg_AREG2 asm(AREG2);
+    volatile unsigned long saved_AREG2;
+#endif
+#ifdef AREG3
+    register unsigned long reg_AREG3 asm(AREG3);
+    volatile unsigned long saved_AREG3;
+#endif
     CPUState *saved_env;
 #ifdef reg_EAX
     int saved_EAX;
@@ -319,10 +335,17 @@ int cpu_exec(CPUState *env1)
     /* first we save global registers */
     saved_env = env;
     env = env1;
-    saved_T0 = T0;
-    saved_T1 = T1;
-#if defined(reg_T2)
-    saved_T2 = T2;
+#ifdef AREG0
+    saved_AREG0 = reg_AREG0;
+#endif
+#ifdef AREG1
+    saved_AREG1 = reg_AREG1;
+#endif
+#ifdef AREG2
+    saved_AREG2 = reg_AREG2;
+#endif
+#ifdef AREG3
+    saved_AREG3 = reg_AREG3;
 #endif
 #if defined(__sparc__) && !defined(HOST_SOLARIS)
     /* we also save i7 because longjmp may not restore it */
@@ -656,9 +679,18 @@ int cpu_exec(CPUState *env1)
 #else
 #error unsupported target CPU
 #endif
-    T0 = saved_T0;
-    T1 = saved_T1;
-    T2 = saved_T2;
+#ifdef AREG0
+    reg_AREG0 = saved_AREG0;
+#endif
+#ifdef AREG1
+    reg_AREG1 = saved_AREG1;
+#endif
+#ifdef AREG2
+    reg_AREG2 = saved_AREG2;
+#endif
+#ifdef AREG3
+    reg_AREG3 = saved_AREG3;
+#endif
     env = saved_env;
     return ret;
 }

RE: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

[Gwenole Beauchesne]

On Fri, 2 Feb 2007, Gwenole Beauchesne wrote:

> The proper fix would be to not globally allocate registers for the whole
> program but only for the micro-ops. Then, make the necessary
> save/restore around the gen_func call.

Hmm, I realized in the train that this wouldn't work for QEMU.

> +    /* Preserve callee-saved registers */
> +#ifdef AREG0
> +    register unsigned long reg_AREG0 asm(AREG0);
> +    volatile unsigned long saved_AREG0;
> +#endif

BTW, better read (unsigned long) as (void *) or uintptr_t in case you ever
want to port QEMU to Win64 or other strange LLP64 platform.

Regards,
Gwenole.

Re: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

[Rob Landley]

On Friday 02 February 2007 2:03 pm, Gwenole Beauchesne wrote:
> On Fri, 2 Feb 2007, Gwenole Beauchesne wrote:
> 
> > The proper fix would be to not globally allocate registers for the whole
> > program but only for the micro-ops. Then, make the necessary
> > save/restore around the gen_func call.
> 
> Hmm, I realized in the train that this wouldn't work for QEMU.
> 
> > +    /* Preserve callee-saved registers */
> > +#ifdef AREG0
> > +    register unsigned long reg_AREG0 asm(AREG0);
> > +    volatile unsigned long saved_AREG0;
> > +#endif
> 
> BTW, better read (unsigned long) as (void *) or uintptr_t in case you ever
> want to port QEMU to Win64 or other strange LLP64 platform.

Is there an LLP64 platform other than Windows-64?  I know there can't be a 
standards compliant Unix platform (including MacOS X) that isn't LP64:

The LP64 standard is here:
http://www.unix.org/whitepapers/64bit.html>the LP64 standard

The rationale for that standard is here:
http://www.unix.org/version2/whatsnew/lp64_wp.html

And the insane legacy reasons Windows decided on a broken approach are 
explained here:
http://blogs.msdn.com/oldnewthing/archive/2005/01/31/363790.aspx

Considering that Windows 64 currently has less of a userbase than Itanium, I'd 
personally wait and see Microsoft's first attempt at a 64 bit solution turns 
out any better than Intel's before putting much effort into supporting it.  
So far it _sounds_ like the software equivalent of Itanic...

Rob
-- 
"Perfection is reached, not when there is no longer anything to add, but
when there is no longer anything to take away." - Antoine de Saint-Exupery

Re: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

[Paul Brook]

On Thursday 08 February 2007 13:09, Rob Landley wrote:
> On Friday 02 February 2007 2:03 pm, Gwenole Beauchesne wrote:
> > On Fri, 2 Feb 2007, Gwenole Beauchesne wrote:
> > > The proper fix would be to not globally allocate registers for the
> > > whole program but only for the micro-ops. Then, make the necessary
> > > save/restore around the gen_func call.
> >
> > Hmm, I realized in the train that this wouldn't work for QEMU.
> >
> > > +    /* Preserve callee-saved registers */
> > > +#ifdef AREG0
> > > +    register unsigned long reg_AREG0 asm(AREG0);
> > > +    volatile unsigned long saved_AREG0;
> > > +#endif
> >
> > BTW, better read (unsigned long) as (void *) or uintptr_t in case you
> > ever want to port QEMU to Win64 or other strange LLP64 platform.

(void *) is no better that (unsigned long).
(void *) will break on 64-bit ILP32 targets (eg. ia64-hpux, and some ppc 
targets).

Paul

Re: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

[Gwenole Beauchesne]

On Thu, 8 Feb 2007, Rob Landley wrote:

> Is there an LLP64 platform other than Windows-64?

I don't know, probably OS/400? Though I doubt about any interest in QEMU
there. ;-)

> And the insane legacy reasons Windows decided on a broken approach are 
> explained here:
> http://blogs.msdn.com/oldnewthing/archive/2005/01/31/363790.aspx

Or, <http://yarchive.net/comp/longlong.html>
--
1) They are indeed using "LLP64", and their goals include:
-	"Porting from win32 to win64 should be simple.
-	Supporting win64 and win32 with a single source base is our goal.
-	No new programming models.
-	Require minimal change to existing win32 code data models."
--

And BTW, 64-bit data representation models were discussed back to 1992 so 
they probably could have started dev education by then and choose an LP64? 
In practise, that would have been very difficult, IMHO.

> So far it _sounds_ like the software equivalent of Itanic...

Oh, please, don't use that silly name. This only makes me think about how
people can be clueless about the Itanium (architecture, benefits, etc.)  
and what it brought. In particular, and likely not limited to, a
reasonnable C++ ABI, __sync_*() primitives in GCC, etc.

This is of course off-topic to qemu-devel@ ;-)
-- 
Gwenole.

Re: [Qemu-devel] Re: strange crash on FreeBSD-current/amd64 (pointertruncation?)

[Rob Landley]

On Friday 09 February 2007 2:06 am, Gwenole Beauchesne wrote:
> In particular, and likely not limited to, a reasonnable C++ ABI

I don't do C++, but Garrett the uClibc++ maintainer used to be at the desk 
next to mine.  He would find this statment hilarious.

Among other things, any _sane_ C++ ABI will deal with nested classes and 
namespaces by listing the innermost one first, so that when you have:

parent1.parent2.parent3.child.member
parent1.parent2.parent3.child.walrus
parent1.parent2.parent3.child.thingy

You don't strcmp your way through 80 bytes of guaranteed-to-be-identical data 
every time you want to do linking or RTTI or anything that needs to find a 
symbol.  This is just one random example of how the people who were doing 
this weren't THINKING.

I'll stop now.

Rob
-- 
"Perfection is reached, not when there is no longer anything to add, but
when there is no longer anything to take away." - Antoine de Saint-Exupery