On 19 August 2014 16:51, Jose Fonseca <[email protected]> wrote: > On 19/08/14 16:41, Jose Fonseca wrote: >> >> On 19/08/14 14:43, Emil Velikov wrote: >>> >>> On 19/08/14 14:34, Emil Velikov wrote: >>>> >>>> On 19/08/14 13:42, Jose Fonseca wrote: >>>>> >>>>> On 12/08/14 16:37, Emil Velikov wrote: >>>>>> >>>>>> MSVC helps us out with the final test by undicating that we're >>>>>> corrupting the stack, which begs the question - at which point are we >>>>>> messing up with the calling conventions. This patch attempts to >>>>>> resolve >>>>>> that yet the bug still persists :'( >>>>>> >>>>>> Signed-off-by: Emil Velikov <[email protected]> >>>>>> --- >>>>>> src/waffle/core/wcore_error_unittest.c | 4 ++++ >>>>>> third_party/threads/threads.h | 2 +- >>>>>> 2 files changed, 5 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/src/waffle/core/wcore_error_unittest.c >>>>>> b/src/waffle/core/wcore_error_unittest.c >>>>>> index 5176031..8b9b334 100644 >>>>>> --- a/src/waffle/core/wcore_error_unittest.c >>>>>> +++ b/src/waffle/core/wcore_error_unittest.c >>>>>> @@ -148,7 +148,11 @@ struct thread_arg { >>>>>> }; >>>>>> >>>>>> /// The start routine given to threads in test >>>>>> wcore_error.thread_local. >>>>>> +#if !defined(_WIN32) >>>>>> static bool >>>>>> +#else >>>>>> +static bool __stdcall >>>>>> +#endif >>>>>> thread_start(struct thread_arg *a) >>>>>> { >>>>>> static const enum waffle_error error_codes[NUM_THREADS] = { >>>>>> diff --git a/third_party/threads/threads.h >>>>>> b/third_party/threads/threads.h >>>>>> index 4e7dba2..eb024dd 100644 >>>>>> --- a/third_party/threads/threads.h >>>>>> +++ b/third_party/threads/threads.h >>>>>> @@ -117,7 +117,7 @@ typedef pthread_once_t once_flag; >>>>>> >>>>>> /*---------------------------- types ----------------------------*/ >>>>>> typedef void (*tss_dtor_t)(void*); >>>>>> -typedef int (*thrd_start_t)(void*); >>>>>> +typedef int (__stdcall *thrd_start_t)(void*); >>>>>> >>>>>> struct xtime { >>>>>> time_t sec; >>>>>> >>>>> >>>>> >>>>> Sorry, I've been on PTO and I haven't caught up with email. >>>>> >>>>> What is the problem here again? >>>>> >>>> We end up with stack corruption in test_wcore_error_thread_local(). >>>> >>>> The documentation indicates [1] that the function pointer passed to >>>> _beginthreadex (in our third_party/threads code) should use __stdcall >>>> calling >>>> convention. I'm not entirely sure which/how many pieces we need to >>>> annotate >>>> due to the amount of function pointers passed around :'( >>>> >>> Actually _beginthreadex() is correctly setup, yet pack.func(pack.arg) >>> (from >>> impl_thrd_routine) may not be. So many functions passing around func >>> pointers >>> is making my head spin. >>> >>> You should be able to reproduce by building waffle and giving 'make >>> check'* a try. >>> >> >> I will try to repro. >> >> But this patch doesn't look right, because we don't call beginthreadex >> with the thrd_start_t pointer. Instead we call _beginthreadex( >> impl_thrd_routine), which does have the __stdcall already. >> >> >> BTW, this code is being used in llvmpipe's multi-threaded rasterized and >> it is known to work. >> >> I suspect the problem is elsewhere. > > > This is the problem: > > thrd_join(threads[i], (int *) &exit_codes[i]); > > a bool takes one byte, where as int takes four. > > If the cast wasn't here the compiler would have warned about the type > mismtach. > Indeed that's the one. Which begs the question why we don't crash on linux but I'll leave that one for some other time. There seems to be another booger a few lines above
a->thread_id = i; The former is int, while the latter is intptr_t. Thank you Jose, I owe you one :) -Emil > Jose > > _______________________________________________ > waffle mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/waffle _______________________________________________ waffle mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/waffle
