Re: [Qemu-devel] qemu 1.6.1

[Paolo Bonzini]

Il 26/10/2013 11:51, Stefan Weil ha scritto:
> Am 24.10.2013 23:47, schrieb Paolo Bonzini:
>> Il 24/10/2013 17:37, Stefan Weil ha scritto:
>>> Yes, that works, too. It also fixes the problem with the assertion
>>> (tested with Wine).
>>>
>>> No, we cannot remove from_, because the same interface is also used
>>> for Linux and other hosts which don't have a 'current' variable.
>>> Or we would have to call qemu_coroutine_self() to get the current
>>> coroutine.
>> Yes, I was thinking of using qemu_coroutine_self().
>>
>> By the way, can you post the two assembly language outputs for just
>>
>> -     CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_);
>> +     CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, current);
>>
>> which AIUI works and is enough to fix the bug?
>>
>> Paolo
> 
> See disassembled code below. I removed compiler option -fstack-protector-all
> to simplify the assembler code and tested that the result was not affected
> by this removal.
> 
> The C and assembler code from the test is also available at
> http://qemu.weilnetz.de/test/coroutine-win32/.

Here is the code with annotations

     broken                                   works
  -------------------------------------------------------------------------
     push   %ebx
     sub    $0x18,%esp                        sub    $0x1c,%esp               
                                              mov    %ebx,0x14(%esp)          
                                              mov    %esi,0x18(%esp)          
                            
     movl   $0x6d62a8,(%esp)                  movl   $0x6d62a8,(%esp)         
     mov    0x24(%esp),%ebx                   mov    0x24(%esp),%ebx            
  ebx = to;
     call   ___emutls_get_address             call   ___emutls_get_address      
  eax = &current;
                            
                                              mov    (%eax),%esi                
  esi = current;
                            
     mov    %ebx,(%eax)                       mov    %ebx,(%eax)                
  current = to;

     mov    0x28(%esp),%eax                   mov    0x28(%esp),%eax            
  eax = action
     mov    %eax,0x24(%ebx)                   mov    %eax,0x24(%ebx)            
  to->action = action
     mov    0x20(%ebx),%eax                   mov    0x20(%ebx),%eax            
  eax = to->fiber
     mov    %eax,(%esp)                       mov    %eax,(%esp)                
  "push" to->fiber
     call   *0x835fc0                         call   *0x835fc0                  
  SwitchToFiber(to->fiber)
     sub    $0x4,%esp                         sub    $0x4,%esp                  
  undo PASCAL calling convention
                            
**   mov    0x20(%esp),%eax                                                     
  eax = from
     mov    0x24(%eax),%eax                   mov    0x24(%esi),%eax            
  eax = from->action
                            
                                              mov    0x14(%esp),%ebx          
                                              mov    0x18(%esp),%esi          
     add    $0x18,%esp                        add    $0x1c,%esp               
     pop    %ebx                                                              
     ret                                      ret                             


I think the problem is that 0x20(%esp) gets somehow corrupted at the
instruction I highlighted with **.

The simplest fix then would be to add a barrier() before and after
SwitchToFiber.

Paolo