From: Julian Seward Date: Thu, 20 Jun 2002 10:25:37 +0000 (+0000) Subject: Fix subtle bug in the interaction between pthread_create and thread_wrapper, X-Git-Tag: svn/VALGRIND_1_0_3~38 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7bcbe59c1f3805e5749240192ec5076edd76a9b1;p=thirdparty%2Fvalgrind.git Fix subtle bug in the interaction between pthread_create and thread_wrapper, exposed by scheduling changes caused by commit vg_scheduler.c rev 1.70. We cannot simply pass the __attr pointer to the child, since it could point to stuff on the parent's stack, which might not exist by the time the child looked at it. Prior to scheduler.c rev 1.70 the child would have been scheduled as soon as created, so the access was made before the parent could clear the stuff from its stack. From rev 1.70 and after the parent continues after creating the child, causing invalid stack accesses when the child finally runs. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@455 --- diff --git a/coregrind/arch/x86-linux/vg_libpthread.c b/coregrind/arch/x86-linux/vg_libpthread.c index 3220cca1a7..3510ed3767 100644 --- a/coregrind/arch/x86-linux/vg_libpthread.c +++ b/coregrind/arch/x86-linux/vg_libpthread.c @@ -472,11 +472,12 @@ void thread_exit_wrapper ( void* ret_val ) root function return a value, it arranges to run the thread's cleanup handlers and exit correctly. */ -/* Struct used to convey info from pthread_create to - thread_wrapper. */ +/* Struct used to convey info from pthread_create to thread_wrapper. + Must be careful not to pass to the child thread any pointers to + objects which might be on the parent's stack. */ typedef struct { - pthread_attr_t* attr; + int attr__detachstate; void* (*root_fn) ( void* ); void* arg; } @@ -490,15 +491,15 @@ static __attribute__((noreturn)) void thread_wrapper ( NewThreadInfo* info ) { - int res; - pthread_attr_t* attr; + int res; + int attr__detachstate; void* (*root_fn) ( void* ); void* arg; void* ret_val; - attr = info->attr; - root_fn = info->root_fn; - arg = info->arg; + attr__detachstate = info->attr__detachstate; + root_fn = info->root_fn; + arg = info->arg; /* Free up the arg block that pthread_create malloced. */ VALGRIND_MAGIC_SEQUENCE(res, (-1) /* default */, @@ -506,12 +507,11 @@ void thread_wrapper ( NewThreadInfo* info ) my_assert(res == 0); /* Minimally observe the attributes supplied. */ - if (attr) { - my_assert(attr->__detachstate == PTHREAD_CREATE_DETACHED - || attr->__detachstate == PTHREAD_CREATE_JOINABLE); - if (attr->__detachstate == PTHREAD_CREATE_DETACHED) - pthread_detach(pthread_self()); - } + if (attr__detachstate != PTHREAD_CREATE_DETACHED + && attr__detachstate != PTHREAD_CREATE_JOINABLE) + pthread_error("thread_wrapper: invalid attr->__detachstate"); + if (attr__detachstate == PTHREAD_CREATE_DETACHED) + pthread_detach(pthread_self()); /* The root function might not return. But if it does we simply move along to thread_exit_wrapper. All other ways out for the @@ -564,7 +564,11 @@ pthread_create (pthread_t *__restrict __thread, sizeof(NewThreadInfo), 0, 0, 0); my_assert(info != NULL); - info->attr = (pthread_attr_t*)__attr; + if (__attr) + info->attr__detachstate = __attr->__detachstate; + else + info->attr__detachstate = PTHREAD_CREATE_JOINABLE; + info->root_fn = __start_routine; info->arg = __arg; VALGRIND_MAGIC_SEQUENCE(tid_child, VG_INVALID_THREADID /* default */, diff --git a/coregrind/vg_libpthread.c b/coregrind/vg_libpthread.c index 3220cca1a7..3510ed3767 100644 --- a/coregrind/vg_libpthread.c +++ b/coregrind/vg_libpthread.c @@ -472,11 +472,12 @@ void thread_exit_wrapper ( void* ret_val ) root function return a value, it arranges to run the thread's cleanup handlers and exit correctly. */ -/* Struct used to convey info from pthread_create to - thread_wrapper. */ +/* Struct used to convey info from pthread_create to thread_wrapper. + Must be careful not to pass to the child thread any pointers to + objects which might be on the parent's stack. */ typedef struct { - pthread_attr_t* attr; + int attr__detachstate; void* (*root_fn) ( void* ); void* arg; } @@ -490,15 +491,15 @@ static __attribute__((noreturn)) void thread_wrapper ( NewThreadInfo* info ) { - int res; - pthread_attr_t* attr; + int res; + int attr__detachstate; void* (*root_fn) ( void* ); void* arg; void* ret_val; - attr = info->attr; - root_fn = info->root_fn; - arg = info->arg; + attr__detachstate = info->attr__detachstate; + root_fn = info->root_fn; + arg = info->arg; /* Free up the arg block that pthread_create malloced. */ VALGRIND_MAGIC_SEQUENCE(res, (-1) /* default */, @@ -506,12 +507,11 @@ void thread_wrapper ( NewThreadInfo* info ) my_assert(res == 0); /* Minimally observe the attributes supplied. */ - if (attr) { - my_assert(attr->__detachstate == PTHREAD_CREATE_DETACHED - || attr->__detachstate == PTHREAD_CREATE_JOINABLE); - if (attr->__detachstate == PTHREAD_CREATE_DETACHED) - pthread_detach(pthread_self()); - } + if (attr__detachstate != PTHREAD_CREATE_DETACHED + && attr__detachstate != PTHREAD_CREATE_JOINABLE) + pthread_error("thread_wrapper: invalid attr->__detachstate"); + if (attr__detachstate == PTHREAD_CREATE_DETACHED) + pthread_detach(pthread_self()); /* The root function might not return. But if it does we simply move along to thread_exit_wrapper. All other ways out for the @@ -564,7 +564,11 @@ pthread_create (pthread_t *__restrict __thread, sizeof(NewThreadInfo), 0, 0, 0); my_assert(info != NULL); - info->attr = (pthread_attr_t*)__attr; + if (__attr) + info->attr__detachstate = __attr->__detachstate; + else + info->attr__detachstate = PTHREAD_CREATE_JOINABLE; + info->root_fn = __start_routine; info->arg = __arg; VALGRIND_MAGIC_SEQUENCE(tid_child, VG_INVALID_THREADID /* default */, diff --git a/vg_libpthread.c b/vg_libpthread.c index 3220cca1a7..3510ed3767 100644 --- a/vg_libpthread.c +++ b/vg_libpthread.c @@ -472,11 +472,12 @@ void thread_exit_wrapper ( void* ret_val ) root function return a value, it arranges to run the thread's cleanup handlers and exit correctly. */ -/* Struct used to convey info from pthread_create to - thread_wrapper. */ +/* Struct used to convey info from pthread_create to thread_wrapper. + Must be careful not to pass to the child thread any pointers to + objects which might be on the parent's stack. */ typedef struct { - pthread_attr_t* attr; + int attr__detachstate; void* (*root_fn) ( void* ); void* arg; } @@ -490,15 +491,15 @@ static __attribute__((noreturn)) void thread_wrapper ( NewThreadInfo* info ) { - int res; - pthread_attr_t* attr; + int res; + int attr__detachstate; void* (*root_fn) ( void* ); void* arg; void* ret_val; - attr = info->attr; - root_fn = info->root_fn; - arg = info->arg; + attr__detachstate = info->attr__detachstate; + root_fn = info->root_fn; + arg = info->arg; /* Free up the arg block that pthread_create malloced. */ VALGRIND_MAGIC_SEQUENCE(res, (-1) /* default */, @@ -506,12 +507,11 @@ void thread_wrapper ( NewThreadInfo* info ) my_assert(res == 0); /* Minimally observe the attributes supplied. */ - if (attr) { - my_assert(attr->__detachstate == PTHREAD_CREATE_DETACHED - || attr->__detachstate == PTHREAD_CREATE_JOINABLE); - if (attr->__detachstate == PTHREAD_CREATE_DETACHED) - pthread_detach(pthread_self()); - } + if (attr__detachstate != PTHREAD_CREATE_DETACHED + && attr__detachstate != PTHREAD_CREATE_JOINABLE) + pthread_error("thread_wrapper: invalid attr->__detachstate"); + if (attr__detachstate == PTHREAD_CREATE_DETACHED) + pthread_detach(pthread_self()); /* The root function might not return. But if it does we simply move along to thread_exit_wrapper. All other ways out for the @@ -564,7 +564,11 @@ pthread_create (pthread_t *__restrict __thread, sizeof(NewThreadInfo), 0, 0, 0); my_assert(info != NULL); - info->attr = (pthread_attr_t*)__attr; + if (__attr) + info->attr__detachstate = __attr->__detachstate; + else + info->attr__detachstate = PTHREAD_CREATE_JOINABLE; + info->root_fn = __start_routine; info->arg = __arg; VALGRIND_MAGIC_SEQUENCE(tid_child, VG_INVALID_THREADID /* default */,