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.


Jose
_______________________________________________
waffle mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/waffle

Reply via email to