From d3a972434c8aec0a47500d7b368130701b4f0a52 Mon Sep 17 00:00:00 2001 From: Siddhesh Poyarekar Date: Thu, 25 Oct 2018 16:18:19 +0530 Subject: [PATCH 1/4] Add a wrapper for thread function in PyThread_start_new_thread PyThread_start_new_thread accepts a function of type void (*) (void *), which does not match with the pthread_create function callback prototype void *(*) (void *). This is undefined behaviour and hence results in an invalid function cast warning with gcc8. Fix this by wrapping the function in an internal pthread function callback that returns NULL. --- .../2018-08-24-09-48-25.bpo-33015.s21y74.rst | 3 ++ Python/thread_pthread.h | 42 +++++++++++++++++-- 2 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst diff --git a/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst new file mode 100644 index 00000000000000..d840f3883cd47e --- /dev/null +++ b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst @@ -0,0 +1,3 @@ +Add a wrapper for the thread function argument in +`PyThread_start_new_thread` to avoid an incompatible cast warning with gcc +8. diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index 6da8b3a44734c6..404f883b58e798 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -152,6 +152,31 @@ PyThread__init_thread(void) * Thread support. */ +/* The thread function argument passed to PyThread_start_new_thread has a + * different type compared to the thread function passed to the POSIX + * pthread_create function and the result of forcibly casting it to the latter + * type is undefined. The pythread_fn struct and the pythread_helper_fn below + * help work around this in a conforming manner by wrapping around the thread + * function. + */ +typedef struct { + void (*func) (void *); + void *arg; +} pythread_fn; + +static void * +pythread_helper_fn(void *fn) +{ + pythread_fn *pfn = fn; + + void (*func)(void *) = pfn->func; + void *arg = pfn->arg; + + PyMem_RawFree((void *)fn); + func(arg); + + return NULL; +} unsigned long PyThread_start_new_thread(void (*func)(void *), void *arg) @@ -188,21 +213,32 @@ PyThread_start_new_thread(void (*func)(void *), void *arg) pthread_attr_setscope(&attrs, PTHREAD_SCOPE_SYSTEM); #endif + pythread_fn *fn = (pythread_fn *)PyMem_RawMalloc(sizeof (pythread_fn)); + + if (fn == NULL) + return PYTHREAD_INVALID_THREAD_ID; + + fn->func = func; + fn->arg = arg; + status = pthread_create(&th, #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED) &attrs, #else (pthread_attr_t*)NULL, #endif - (void* (*)(void *))func, - (void *)arg + pythread_helper_fn, + fn ); #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED) pthread_attr_destroy(&attrs); #endif - if (status != 0) + + if (status != 0) { + PyMem_RawFree((void *)fn); return PYTHREAD_INVALID_THREAD_ID; + } pthread_detach(th); From 4d5b9b08225d7bb99536d312b2b42939bd54b2f9 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 30 Nov 2018 14:38:40 +0100 Subject: [PATCH 2/4] Cleanup code * Rename pythread_fn to pythread_callback * Rename pythread_helper_fn() to pythread_wrapper() * Rewrite the comment --- Python/thread_pthread.h | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index 404f883b58e798..01919dffcc497a 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -152,29 +152,26 @@ PyThread__init_thread(void) * Thread support. */ -/* The thread function argument passed to PyThread_start_new_thread has a - * different type compared to the thread function passed to the POSIX - * pthread_create function and the result of forcibly casting it to the latter - * type is undefined. The pythread_fn struct and the pythread_helper_fn below - * help work around this in a conforming manner by wrapping around the thread - * function. - */ +/* bpo-33015: pythread_callback struct and pythread_wrapper() cast + "void func(void *)" to "void* func(void *)": always return NULL. + + PyThread_start_new_thread() uses "void func(void *)" type, whereas + pthread_create() requires a void* return value. */ typedef struct { void (*func) (void *); void *arg; -} pythread_fn; +} pythread_callback; static void * -pythread_helper_fn(void *fn) +pythread_wrapper(void *arg) { - pythread_fn *pfn = fn; - - void (*func)(void *) = pfn->func; - void *arg = pfn->arg; - - PyMem_RawFree((void *)fn); - func(arg); + /* copy func and func_arg and free the temporary structure */ + pythread_callback *callback = arg; + void (*func)(void *) = callback->func; + void *func_arg = callback->arg; + PyMem_RawFree(arg); + func(func_arg); return NULL; } @@ -213,7 +210,7 @@ PyThread_start_new_thread(void (*func)(void *), void *arg) pthread_attr_setscope(&attrs, PTHREAD_SCOPE_SYSTEM); #endif - pythread_fn *fn = (pythread_fn *)PyMem_RawMalloc(sizeof (pythread_fn)); + pythread_callback *fn = PyMem_RawMalloc(sizeof(pythread_callback)); if (fn == NULL) return PYTHREAD_INVALID_THREAD_ID; @@ -227,9 +224,7 @@ PyThread_start_new_thread(void (*func)(void *), void *arg) #else (pthread_attr_t*)NULL, #endif - pythread_helper_fn, - fn - ); + pythread_wrapper, fn); #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED) pthread_attr_destroy(&attrs); From 6d0bade9431ccf1e15bfa0750f99342b51947b02 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 30 Nov 2018 14:43:29 +0100 Subject: [PATCH 3/4] More cleanup * Rename "fn" to "callback" * Add { ... } to if * Remove useless cast --- .../Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst | 6 +++--- Python/thread_pthread.h | 13 +++++++------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst index d840f3883cd47e..a26b0fed521918 100644 --- a/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst +++ b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst @@ -1,3 +1,3 @@ -Add a wrapper for the thread function argument in -`PyThread_start_new_thread` to avoid an incompatible cast warning with gcc -8. +Fix an undefined behaviour in the pthread implementation of +:c:func:`PyThread_start_new_thread`: add a function wrapper to always return +``NULL`. diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index 01919dffcc497a..09e53a14aa473b 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -210,13 +210,14 @@ PyThread_start_new_thread(void (*func)(void *), void *arg) pthread_attr_setscope(&attrs, PTHREAD_SCOPE_SYSTEM); #endif - pythread_callback *fn = PyMem_RawMalloc(sizeof(pythread_callback)); + pythread_callback *callback = PyMem_RawMalloc(sizeof(pythread_callback)); - if (fn == NULL) + if (callback == NULL) { return PYTHREAD_INVALID_THREAD_ID; + } - fn->func = func; - fn->arg = arg; + callback->func = func; + callback->arg = arg; status = pthread_create(&th, #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED) @@ -224,14 +225,14 @@ PyThread_start_new_thread(void (*func)(void *), void *arg) #else (pthread_attr_t*)NULL, #endif - pythread_wrapper, fn); + pythread_wrapper, callback); #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED) pthread_attr_destroy(&attrs); #endif if (status != 0) { - PyMem_RawFree((void *)fn); + PyMem_RawFree(callback); return PYTHREAD_INVALID_THREAD_ID; } From 1499f464a6b2646f5857ba1f60b549fa3ba2c5f7 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 30 Nov 2018 14:58:16 +0100 Subject: [PATCH 4/4] fix typo in the NEWS entry --- Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst index a26b0fed521918..8c5a0c91a46b46 100644 --- a/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst +++ b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst @@ -1,3 +1,3 @@ Fix an undefined behaviour in the pthread implementation of :c:func:`PyThread_start_new_thread`: add a function wrapper to always return -``NULL`. +``NULL``.