[waffle] [PATCH 4/4] wgl: attempt to fix the final test

[Jose Fonseca]

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 <emil.l.velikov at gmail.com>
>>>>> ---
>>>>>    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.

Jose