Patchwork Fiber switching and stack protection

login
register
mail settings
Submitter Bob Breuer
Date April 13, 2012, 2:53 p.m.
Message ID <4F883DE3.2050501@mc.net>
Download mbox | patch
Permalink /patch/152327/
State New
Headers show

Comments

Bob Breuer - April 13, 2012, 2:53 p.m.
On 4/13/2012 6:25 AM, Pavel Dovgaluk wrote:
>> -----Original Message-----
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>> Sent: Thursday, April 12, 2012 8:57 PM
>> To: Stefan Weil
>> Cc: Kevin Wolf; 'qemu-devel'; Pavel Dovgaluk
>> Subject: Re: [Qemu-devel] Fiber switching and stack protection
>>
>> Il 12/04/2012 18:31, Stefan Weil ha scritto:
>>> Am 12.04.2012 12:18, schrieb Paolo Bonzini:
>>>> Il 12/04/2012 12:13, Kevin Wolf ha scritto:
>>>>> I guess it's this non-thread-local TLS once again, basically a
>>>>> compiler bug.
> 
>   You are right, this is a compiler bug with non-thread-local TLS.
> 
>>>>> Paolo, wasn't there a compiler option that works around the problem?
>>>>>
>>>> I asked to test it (-D_MT) but never got any answer.
>>>
>>> I'd be surprised if defining _MT helped against compiler bugs.
>>
>> Well, -mthreads fixed it, and it should be the same as -D_MT -lmingwthrd.  But we shouldn't
>> need libmingwthrd, or do we?
> 
>   I've tried to add -mthreads to compiler options but nothing has changed, qemu still fails.


This is likely a different bug than the original thread related bug.
I'm using the mingw gcc 4.6.2, and I see the same thing as shown here:
http://virtuallyfun.superglobalmegacorp.com/?p=1846

Something goes wrong during optimization with gcc 4.6.2, but it doesn't
appear to be TLS related.  Digging into it a bit,
qemu_coroutine_switch() seems to break if it gets inlined.  Can anyone
else confirm if this one-line patch works for them?



Bob
Pavel Dovgaluk - April 16, 2012, 6:24 a.m.
> >>>>> Paolo, wasn't there a compiler option that works around the problem?
> >>>>>
> >>>> I asked to test it (-D_MT) but never got any answer.
> >>>
> >>> I'd be surprised if defining _MT helped against compiler bugs.
> >>
> >> Well, -mthreads fixed it, and it should be the same as -D_MT
> >> -lmingwthrd.  But we shouldn't need libmingwthrd, or do we?
> >
> >   I've tried to add -mthreads to compiler options but nothing has changed, qemu still fails.
> 
> This is likely a different bug than the original thread related bug.
> I'm using the mingw gcc 4.6.2, and I see the same thing as shown here:
> http://virtuallyfun.superglobalmegacorp.com/?p=1846
> 
> Something goes wrong during optimization with gcc 4.6.2, but it doesn't appear to be TLS
> related.  Digging into it a bit,
> qemu_coroutine_switch() seems to break if it gets inlined.  Can anyone else confirm if this
> one-line patch works for them?
> 
> diff --git a/coroutine-win32.c b/coroutine-win32.c index 4179609..504873b 100644
> --- a/coroutine-win32.c
> +++ b/coroutine-win32.c
> @@ -36,6 +36,7 @@ typedef struct
>  static __thread CoroutineWin32 leader;
>  static __thread Coroutine *current;
> 
> +__attribute__ ((noinline))
>  CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
>                                        CoroutineAction action)  {

  I applied your patch to qemu 1.0 and it stopped crashing.

  By the way, I also created implementation of coroutine-win32.c, which
uses WinAPI TLS functions directly (without using __thread specifiers).
It works fine too.


Pavel Dovgaluk
Roy Tam - April 16, 2012, 6:39 a.m.
2012/4/16 Pavel Dovgaluk <Pavel.Dovgaluk@ispras.ru>:
>
>[snipped]
>
>  By the way, I also created implementation of coroutine-win32.c, which
> uses WinAPI TLS functions directly (without using __thread specifiers).
> It works fine too.
>

Patch is welcome. :D

>
> Pavel Dovgaluk
>
>
Paolo Bonzini - April 16, 2012, 7 a.m.
Il 16/04/2012 08:24, Pavel Dovgaluk ha scritto:
>   By the way, I also created implementation of coroutine-win32.c, which
> uses WinAPI TLS functions directly (without using __thread specifiers).
> It works fine too.

Cool, can you submit it?

Paolo
Pavel Dovgaluk - April 16, 2012, 8:18 a.m.
> Il 16/04/2012 08:24, Pavel Dovgaluk ha scritto:
> >   By the way, I also created implementation of coroutine-win32.c,
> > which uses WinAPI TLS functions directly (without using __thread specifiers).
> > It works fine too.
> 
> Cool, can you submit it?

 I submitted the patch in a separate message.

Pavel Dovgaluk

Patch

diff --git a/coroutine-win32.c b/coroutine-win32.c
index 4179609..504873b 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -36,6 +36,7 @@  typedef struct
 static __thread CoroutineWin32 leader;
 static __thread Coroutine *current;

+__attribute__ ((noinline))
 CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
                                       CoroutineAction action)
 {