On 19/08/14 18:44, Emil Velikov wrote:
On 19 August 2014 16:51, Jose Fonseca jfons...@vmware.com 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 emil.l.veli...@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.
Indeed that's the one. Which begs the question why we don't crash on
linux
I'm sure it overflows on Linux too. valgrind might complain.
But MSVC emits code to check for stack overflow -- some sort of canary.
There is no crash per se -- but an error dialog.
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.
Right, it will be truncated. There should be no overflow here though.
Thank you Jose, I owe you one :)
-Emil
You're welcome!
Jose
___
waffle mailing list
waffle@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/waffle
