Re: [PATCH v8 1/8] nptl: Add C11 threads thrd_* functions

[Florian Weimer <fweimer@redhat.com>]

On 02/02/2018 06:04 PM, Adhemerval Zanella wrote:

> diff --git a/include/stdc-predef.h b/include/stdc-predef.h
> index c569759..c2ab78a 100644
> --- a/include/stdc-predef.h
> +++ b/include/stdc-predef.h
> @@ -57,7 +57,4 @@
>      - 3 additional Zanabazar Square characters */
>   #define __STDC_ISO_10646__		201706L
>   
> -/* We do not support C11 <threads.h>.  */
> -#define __STDC_NO_THREADS__		1

Should we do this only if we know that the compiler has _Thread_local 
support (based on a GCC and __cplusplus version check)?

> diff --git a/nptl/descr.h b/nptl/descr.h
> index 64ba29e..f00e2c0 100644
> --- a/nptl/descr.h
> +++ b/nptl/descr.h
> @@ -371,6 +371,8 @@ struct pthread
>        to the function.  */
>     void *(*start_routine) (void *);
>     void *arg;
> +  /* Indicates whether is a C11 thread created by thrd_creat.  */
> +  bool c11;
>   
>     /* Debug state.  */
>     td_eventbuf_t eventbuf;

Can you move the new member towards the end of the struct?  I'm worried 
about the ABI implications.

> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index caaf07c..74e773a 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -460,7 +460,19 @@ START_THREAD_DEFN
>         LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg);
>   
>         /* Run the code the user provided.  */
> -      THREAD_SETMEM (pd, result, pd->start_routine (pd->arg));
> +      void *ret;
> +      if (pd->c11)
> +	{
> +	  /* The function pointer of the c11 thread start is cast to an incorrect
> +	     type on __pthread_create_2_1 call, however it is casted back to correct
> +	     one so the call behavior is well-defined (it is assumed that pointers
> +	     to void are able to represent all values of int.  */
> +	  int (*start)(void*) = (int (*) (void*)) pd->start_routine;
> +	  ret = (void*) (intptr_t) start (pd->arg);

(I think this required on m68k, where void * and int are returned in 
different registers.)

> +int
> +thrd_join (thrd_t thr, int *res)
> +{
> +  void *pthread_res;
> +  int err_code = __pthread_join (thr, &pthread_res);
> +  if (res)
> +   *res = (int)((uintptr_t) pthread_res);
> +
> +  return thrd_err_map (err_code);
> +}

Slight inconsistency with intptr_t above.

> diff --git a/sysdeps/nptl/threads.h b/sysdeps/nptl/threads.h
> new file mode 100644
> index 0000000..6adcac4
> --- /dev/null
> +++ b/sysdeps/nptl/threads.h

Should this be nptl/threads.h, not sysdeps/nptl/threads.h?

Thanks,
Florian