[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Crash due to invalid env->current_tb
From: |
Adam Lackorzynski |
Subject: |
Re: [Qemu-devel] Crash due to invalid env->current_tb |
Date: |
Wed, 30 Apr 2008 22:36:36 +0200 |
User-agent: |
Mutt/1.5.17+20080114 (2008-01-14) |
On Wed Apr 30, 2008 at 17:28:04 +0200, Laurent Vivier wrote:
>
> Le mercredi 30 avril 2008 à 17:11 +0200, Adam Lackorzynski a écrit :
> > On Wed Apr 30, 2008 at 11:08:46 +0200, Alexander Graf wrote:
> > >
> > > On Apr 29, 2008, at 8:40 PM, Adam Lackorzynski wrote:
> > >
> > >>
> > >> On Tue Apr 29, 2008 at 20:09:00 +0300, Blue Swirl wrote:
> > >>> On 4/29/08, Adam Lackorzynski <address@hidden> wrote:
> > >>>> Hi,
> > >>>>
> > >>>> I've been experiencing crashes of latest svn Qemu, host ia32 and
> > >>>> target
> > >>>> arm, host gcc is 'gcc version 3.4.6 (Debian 3.4.6-7)'.
> > >>>> The segfault happens because of an invalid env->current_tb which
> > >>>> seems
> > >>>> to be caused by generated code. The following code in cpu_exec
> > >>>>
> > >>>> tc_ptr = tb->tc_ptr;
> > >>>> env->current_tb = tb;
> > >>>> gen_func = (void *)tc_ptr;
> > >>>> T0 = gen_func();
> > >>>> env->current_tb = NULL;
> > >>>>
> > >>>> is being compiled to the following
> > >>>>
> > >>>> mov 0x14(%ecx),%eax
> > >>>> mov %ecx,0x56c(%ebp)
> > >>>> xor %edi,%edi
> > >>>> call *%eax
> > >>>> mov %edi,0x56c(%ebp)
> > >>>>
> > >>>> After the call edi isn't 0 anymore and gets the bogus value. As
> > >>>> edi is
> > >>>> callee saved the code itself seems ok.
> > >>>> When I add a barrier before "env->current_tb = NULL" the xor is
> > >>>> placed
> > >>>> after the call and everything works fine. So might the problem be
> > >>>> that
> > >>>> generated code isn't preserving edi/registers?
> > >>>
> > >>> Right. How did you make the barrier? My version (attached) just
> > >>> crashes, I'm not fluent on i386 assembly. Maybe your version could
> > >>> serve as a temporary fix.
> > >>
> > >> I just added an 'asm volatile("")' to stop reordering of instructions
> > >> which of course isn't enough. The following works for me:
> > >>
> > >> ===================================================================
> > >> --- cpu-exec.c (revision 4276)
> > >> +++ cpu-exec.c (working copy)
> > >> @@ -690,6 +691,11 @@
> > >> fp.ip = tc_ptr;
> > >> fp.gp = code_gen_buffer + 2 * (1 << 20);
> > >> (*(void (*)(void)) &fp)();
> > >> +#elif defined(__i386)
> > >> + asm volatile ("call *%1\n"
> > >> + : "=a" (T0)
> > >> + : "r" (gen_func)
> > >> + : "esi", "edi");
> > >> #else
> > >> T0 = gen_func();
> > >> #endif
> > >
> > > There was a comment from Fabrice on how to do prologues in TCG to save /
> > > restore the clobbered values. Btw, ebx gets clobbered as well.
> >
> > tcg/README says that some registers are clobbered. So something like
> > this should be safe:
> >
> > Index: cpu-exec.c
> > ===================================================================
> > --- cpu-exec.c (revision 4276)
> > +++ cpu-exec.c (working copy)
> > @@ -690,6 +691,15 @@
> > fp.ip = tc_ptr;
> > fp.gp = code_gen_buffer + 2 * (1 << 20);
> > (*(void (*)(void)) &fp)();
> > +#elif defined(__i386)
> > + asm volatile ("push %%ebp\n"
> > + "push %%ebx\n"
> > + "call *%1\n"
> > + "pop %%ebx\n"
> > + "pop %%ebp\n"
> > + : "=a" (T0)
> > + : "r" (gen_func)
> > + : "esi", "edi", "ecx", "edx");
>
> Why don't you add ebp and ebx in the clobbered registers list (like
> "esi", "edi", "ecx", "edx") ?
For ebp it's more safe to use push as it depends whether the binary is
compiled with frame-pointer or without. When without you can put it into
the clobber list, when with you should not, we had some bad experience
with this (also see gcc bugzilla #11807).
T0 is register defined to ebx, so it's not needed (the push/pop is also
not needed), and also it does not work, gcc complains.
Adam
--
Adam address@hidden
Lackorzynski http://os.inf.tu-dresden.de/~adam/
- [Qemu-devel] Crash due to invalid env->current_tb, Adam Lackorzynski, 2008/04/29
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Blue Swirl, 2008/04/29
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Adam Lackorzynski, 2008/04/29
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Alexander Graf, 2008/04/30
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Adam Lackorzynski, 2008/04/30
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Adam Lackorzynski, 2008/04/30
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Blue Swirl, 2008/04/30
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Alexander Graf, 2008/04/30
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Blue Swirl, 2008/04/30
- Re: [Qemu-devel] Crash due to invalid env->current_tb, Laurent Vivier, 2008/04/30
- Re: [Qemu-devel] Crash due to invalid env->current_tb,
Adam Lackorzynski <=