* [Qemu-devel] Build broken
@ 2011-08-05 6:22 malc
2011-08-05 7:29 ` Kevin Wolf
2011-08-05 8:58 ` Stefan Hajnoczi
0 siblings, 2 replies; 10+ messages in thread
From: malc @ 2011-08-05 6:22 UTC (permalink / raw)
To: qemu-devel, kwolf
/home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
/home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
/home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 41c2379..42dc3e2 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
CoroutineUContext *co;
ucontext_t old_uc, uc;
jmp_buf old_env;
- union cc_arg arg;
+ union cc_arg arg = {0};
/* The ucontext functions preserve signal masks which incurs a system call
* overhead. setjmp()/longjmp() does not preserve signal masks but only
I guess gcc should yell not only here on ppc32 but on any machine where
pointer size is less than the size of two ints.
--
mailto:av1474@comtv.ru
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 6:22 [Qemu-devel] Build broken malc
@ 2011-08-05 7:29 ` Kevin Wolf
2011-08-05 8:48 ` Stefan Hajnoczi
2011-08-05 8:58 ` Stefan Hajnoczi
1 sibling, 1 reply; 10+ messages in thread
From: Kevin Wolf @ 2011-08-05 7:29 UTC (permalink / raw)
To: malc; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.08.2011 08:22, schrieb malc:
>
> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
>
> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
> index 41c2379..42dc3e2 100644
> --- a/coroutine-ucontext.c
> +++ b/coroutine-ucontext.c
> @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
> CoroutineUContext *co;
> ucontext_t old_uc, uc;
> jmp_buf old_env;
> - union cc_arg arg;
> + union cc_arg arg = {0};
>
> /* The ucontext functions preserve signal masks which incurs a system call
> * overhead. setjmp()/longjmp() does not preserve signal masks but only
>
> I guess gcc should yell not only here on ppc32 but on any machine where
> pointer size is less than the size of two ints.
Stefan, why does this code even exist again? I think at some point I had
it changed to just use a static variable in order to avoid doing this
kind of tricks with unions.
Interestingly, the buildbot doesn't seem to have failed on i386.
Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 7:29 ` Kevin Wolf
@ 2011-08-05 8:48 ` Stefan Hajnoczi
2011-08-05 9:09 ` Kevin Wolf
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Hajnoczi @ 2011-08-05 8:48 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, Stefan Hajnoczi
On Fri, Aug 5, 2011 at 8:29 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 05.08.2011 08:22, schrieb malc:
>>
>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
>>
>> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
>> index 41c2379..42dc3e2 100644
>> --- a/coroutine-ucontext.c
>> +++ b/coroutine-ucontext.c
>> @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
>> CoroutineUContext *co;
>> ucontext_t old_uc, uc;
>> jmp_buf old_env;
>> - union cc_arg arg;
>> + union cc_arg arg = {0};
>>
>> /* The ucontext functions preserve signal masks which incurs a system call
>> * overhead. setjmp()/longjmp() does not preserve signal masks but only
>>
>> I guess gcc should yell not only here on ppc32 but on any machine where
>> pointer size is less than the size of two ints.
>
> Stefan, why does this code even exist again? I think at some point I had
> it changed to just use a static variable in order to avoid doing this
> kind of tricks with unions.
virtfs are using coroutines in multiple threads at the same time.
Introducing a global variable wouldn't be thread-safe.
The real problem is that makecontext(3) has a bad function signature.
There's no nice fix - whatever we do will be ugly.
Using a union is the way it should be done in C. The code doesn't
look pretty but it doesn't introduce global state.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 6:22 [Qemu-devel] Build broken malc
2011-08-05 7:29 ` Kevin Wolf
@ 2011-08-05 8:58 ` Stefan Hajnoczi
2011-08-05 16:49 ` malc
1 sibling, 1 reply; 10+ messages in thread
From: Stefan Hajnoczi @ 2011-08-05 8:58 UTC (permalink / raw)
To: malc; +Cc: kwolf, qemu-devel
On Fri, Aug 5, 2011 at 7:22 AM, malc <av1474@comtv.ru> wrote:
>
> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
>
> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
> index 41c2379..42dc3e2 100644
> --- a/coroutine-ucontext.c
> +++ b/coroutine-ucontext.c
> @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
> CoroutineUContext *co;
> ucontext_t old_uc, uc;
> jmp_buf old_env;
> - union cc_arg arg;
> + union cc_arg arg = {0};
>
> /* The ucontext functions preserve signal masks which incurs a system call
> * overhead. setjmp()/longjmp() does not preserve signal masks but only
>
> I guess gcc should yell not only here on ppc32 but on any machine where
> pointer size is less than the size of two ints.
Makes sense. Are you going to commit a fix or send a signed-off-by patch?
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 8:48 ` Stefan Hajnoczi
@ 2011-08-05 9:09 ` Kevin Wolf
2011-08-05 9:37 ` Stefan Hajnoczi
0 siblings, 1 reply; 10+ messages in thread
From: Kevin Wolf @ 2011-08-05 9:09 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Stefan Hajnoczi
Am 05.08.2011 10:48, schrieb Stefan Hajnoczi:
> On Fri, Aug 5, 2011 at 8:29 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 05.08.2011 08:22, schrieb malc:
>>>
>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
>>>
>>> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
>>> index 41c2379..42dc3e2 100644
>>> --- a/coroutine-ucontext.c
>>> +++ b/coroutine-ucontext.c
>>> @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
>>> CoroutineUContext *co;
>>> ucontext_t old_uc, uc;
>>> jmp_buf old_env;
>>> - union cc_arg arg;
>>> + union cc_arg arg = {0};
>>>
>>> /* The ucontext functions preserve signal masks which incurs a system call
>>> * overhead. setjmp()/longjmp() does not preserve signal masks but only
>>>
>>> I guess gcc should yell not only here on ppc32 but on any machine where
>>> pointer size is less than the size of two ints.
>>
>> Stefan, why does this code even exist again? I think at some point I had
>> it changed to just use a static variable in order to avoid doing this
>> kind of tricks with unions.
>
> virtfs are using coroutines in multiple threads at the same time.
> Introducing a global variable wouldn't be thread-safe.
>
> The real problem is that makecontext(3) has a bad function signature.
> There's no nice fix - whatever we do will be ugly.
>
> Using a union is the way it should be done in C. The code doesn't
> look pretty but it doesn't introduce global state.
But it makes assumptions about the pointer size, which isn't a nice
thing. TLS isn't an option for compatibility with some OSes/architectures?
Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 9:09 ` Kevin Wolf
@ 2011-08-05 9:37 ` Stefan Hajnoczi
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2011-08-05 9:37 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, Stefan Hajnoczi
On Fri, Aug 5, 2011 at 10:09 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 05.08.2011 10:48, schrieb Stefan Hajnoczi:
>> On Fri, Aug 5, 2011 at 8:29 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> Am 05.08.2011 08:22, schrieb malc:
>>>>
>>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
>>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
>>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
>>>>
>>>> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
>>>> index 41c2379..42dc3e2 100644
>>>> --- a/coroutine-ucontext.c
>>>> +++ b/coroutine-ucontext.c
>>>> @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
>>>> CoroutineUContext *co;
>>>> ucontext_t old_uc, uc;
>>>> jmp_buf old_env;
>>>> - union cc_arg arg;
>>>> + union cc_arg arg = {0};
>>>>
>>>> /* The ucontext functions preserve signal masks which incurs a system call
>>>> * overhead. setjmp()/longjmp() does not preserve signal masks but only
>>>>
>>>> I guess gcc should yell not only here on ppc32 but on any machine where
>>>> pointer size is less than the size of two ints.
>>>
>>> Stefan, why does this code even exist again? I think at some point I had
>>> it changed to just use a static variable in order to avoid doing this
>>> kind of tricks with unions.
>>
>> virtfs are using coroutines in multiple threads at the same time.
>> Introducing a global variable wouldn't be thread-safe.
>>
>> The real problem is that makecontext(3) has a bad function signature.
>> There's no nice fix - whatever we do will be ugly.
>>
>> Using a union is the way it should be done in C. The code doesn't
>> look pretty but it doesn't introduce global state.
>
> But it makes assumptions about the pointer size, which isn't a nice
> thing. TLS isn't an option for compatibility with some OSes/architectures?
GThread TLS is portable but slow in my testing.
You are right, when we move to 128-bit pointers this code will break
:). We could at add a #warning if pointer size > 64-bits.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 8:58 ` Stefan Hajnoczi
@ 2011-08-05 16:49 ` malc
2011-08-06 6:12 ` Stefan Hajnoczi
2011-08-08 8:02 ` Kevin Wolf
0 siblings, 2 replies; 10+ messages in thread
From: malc @ 2011-08-05 16:49 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, qemu-devel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1282 bytes --]
On Fri, 5 Aug 2011, Stefan Hajnoczi wrote:
> On Fri, Aug 5, 2011 at 7:22 AM, malc <av1474@comtv.ru> wrote:
> >
> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
> >
> > diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
> > index 41c2379..42dc3e2 100644
> > --- a/coroutine-ucontext.c
> > +++ b/coroutine-ucontext.c
> > @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
> > CoroutineUContext *co;
> > ucontext_t old_uc, uc;
> > jmp_buf old_env;
> > - union cc_arg arg;
> > + union cc_arg arg = {0};
> >
> > /* The ucontext functions preserve signal masks which incurs a system call
> > * overhead. setjmp()/longjmp() does not preserve signal masks but only
> >
> > I guess gcc should yell not only here on ppc32 but on any machine where
> > pointer size is less than the size of two ints.
>
> Makes sense. Are you going to commit a fix or send a signed-off-by patch?
>
If the author(s)(you and Kevin? just you?) agree with the above i can just
push it.
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 16:49 ` malc
@ 2011-08-06 6:12 ` Stefan Hajnoczi
2011-08-08 8:02 ` Kevin Wolf
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2011-08-06 6:12 UTC (permalink / raw)
To: malc; +Cc: kwolf, qemu-devel
On Fri, Aug 5, 2011 at 5:49 PM, malc <av1474@comtv.ru> wrote:
> On Fri, 5 Aug 2011, Stefan Hajnoczi wrote:
>
>> On Fri, Aug 5, 2011 at 7:22 AM, malc <av1474@comtv.ru> wrote:
>> >
>> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
>> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
>> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
>> >
>> > diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
>> > index 41c2379..42dc3e2 100644
>> > --- a/coroutine-ucontext.c
>> > +++ b/coroutine-ucontext.c
>> > @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
>> > CoroutineUContext *co;
>> > ucontext_t old_uc, uc;
>> > jmp_buf old_env;
>> > - union cc_arg arg;
>> > + union cc_arg arg = {0};
>> >
>> > /* The ucontext functions preserve signal masks which incurs a system call
>> > * overhead. setjmp()/longjmp() does not preserve signal masks but only
>> >
>> > I guess gcc should yell not only here on ppc32 but on any machine where
>> > pointer size is less than the size of two ints.
>>
>> Makes sense. Are you going to commit a fix or send a signed-off-by patch?
>>
>
> If the author(s)(you and Kevin? just you?) agree with the above i can just
> push it.
The change makes sense to me.
Kevin?
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-05 16:49 ` malc
2011-08-06 6:12 ` Stefan Hajnoczi
@ 2011-08-08 8:02 ` Kevin Wolf
2011-08-08 10:40 ` malc
1 sibling, 1 reply; 10+ messages in thread
From: Kevin Wolf @ 2011-08-08 8:02 UTC (permalink / raw)
To: malc; +Cc: Stefan Hajnoczi, qemu-devel
Am 05.08.2011 18:49, schrieb malc:
> On Fri, 5 Aug 2011, Stefan Hajnoczi wrote:
>
>> On Fri, Aug 5, 2011 at 7:22 AM, malc <av1474@comtv.ru> wrote:
>>>
>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 'coroutine_new':
>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' may be used uninitialized in this function
>>> /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' was declared here
>>>
>>> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
>>> index 41c2379..42dc3e2 100644
>>> --- a/coroutine-ucontext.c
>>> +++ b/coroutine-ucontext.c
>>> @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
>>> CoroutineUContext *co;
>>> ucontext_t old_uc, uc;
>>> jmp_buf old_env;
>>> - union cc_arg arg;
>>> + union cc_arg arg = {0};
>>>
>>> /* The ucontext functions preserve signal masks which incurs a system call
>>> * overhead. setjmp()/longjmp() does not preserve signal masks but only
>>>
>>> I guess gcc should yell not only here on ppc32 but on any machine where
>>> pointer size is less than the size of two ints.
>>
>> Makes sense. Are you going to commit a fix or send a signed-off-by patch?
>>
>
> If the author(s)(you and Kevin? just you?) agree with the above i can just
> push it.
Feel free to push it. (Original code was by Anthony, then heavily
modified by me, and after that modified again by Stefan)
Acked-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Build broken
2011-08-08 8:02 ` Kevin Wolf
@ 2011-08-08 10:40 ` malc
0 siblings, 0 replies; 10+ messages in thread
From: malc @ 2011-08-08 10:40 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel
On Mon, 8 Aug 2011, Kevin Wolf wrote:
> Am 05.08.2011 18:49, schrieb malc:
> > On Fri, 5 Aug 2011, Stefan Hajnoczi wrote:
> >
[..snip..]
>
> Feel free to push it. (Original code was by Anthony, then heavily
> modified by me, and after that modified again by Stefan)
>
> Acked-by: Kevin Wolf <kwolf@redhat.com>
Pushed.
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-08-08 10:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-05 6:22 [Qemu-devel] Build broken malc
2011-08-05 7:29 ` Kevin Wolf
2011-08-05 8:48 ` Stefan Hajnoczi
2011-08-05 9:09 ` Kevin Wolf
2011-08-05 9:37 ` Stefan Hajnoczi
2011-08-05 8:58 ` Stefan Hajnoczi
2011-08-05 16:49 ` malc
2011-08-06 6:12 ` Stefan Hajnoczi
2011-08-08 8:02 ` Kevin Wolf
2011-08-08 10:40 ` malc
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.