Re: [PATCH v2] handle sem_t with ILP32 and __HAVE_64B_ATOMICS

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Jan 26, 2015 at 3:35 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> This version reflects Torvald's recent comments.
>
> 2015-01-25  Chris Metcalf  <cmetcalf@ezchip.com>
>
>         * sysdeps/nptl/internaltypes.h (to_new_sem): Define.  Provides new
>         behavior for [__HAVE_64B_ATOMICS && !defined (_LP64)].
>         * nptl/sem_getvalue.c (__new_sem_getvalue): Use to_new_sem.
>         * nptl/sem_init.c (__new_sem_init): Likewise.
>         * nptl/sem_open.c (sem_open): Likewise.
>         * nptl/sem_post.c (__new_sem_post): Likewise.
>         * nptl/sem_timedwait.c (sem_timedwait): Likewise.
>         * nptl/sem_wait.c (__new_sem_wait): Likewise.
>         (__new_sem_trywait): Likewise.
>

>    /* Use the values the caller provided.  */
>  #if __HAVE_64B_ATOMICS
> diff --git a/nptl/sem_open.c b/nptl/sem_open.c
> index bfd2dea..202769f 100644
> --- a/nptl/sem_open.c
> +++ b/nptl/sem_open.c
> @@ -186,25 +186,28 @@ sem_open (const char *name, int oflag, ...)
>           return SEM_FAILED;
>         }
>
> -      /* Create the initial file content.  */
> -      union
> -      {
> -       sem_t initsem;
> -       struct new_sem newsem;
> -      } sem;
> +      /* Create the initial file content.  We force the alignment of
> +         the sem_t to match the alignment of struct new_sem since we
> +         will copy this stack structure to a file and then mmap it,
> +         so we must ensure it is aligned to zero here so that the
> +         behavior of to_new_sem () is the same as when we later mmap
> +         it into memory and have it be page-aligned.  */
> +      sem_t sem __attribute__ ((aligned (__alignof__ (struct new_sem))));;
> +      struct new_sem *newsem = to_new_sem (&sem);
> +
> +      /* Initialize the unused parts of sem_t to zero.
> +         The compiler will notice most of this memset is dead based on
> +         the assignments through the struct new_sem pointer.  */
> +      memset (&sem, '\0', sizeof (sem_t));

Why is this change needed?  Since union sem has the largest alignment of
sem_t and struct new_sem, sem is properly aligned here.

>  #if __HAVE_64B_ATOMICS
> -      sem.newsem.data = value;
> +      newsem->data = value;
>  #else
> -      sem.newsem.value = value << SEM_VALUE_SHIFT;
> -      sem.newsem.nwaiters = 0;
> +      newsem->value = value << SEM_VALUE_SHIFT;
> +      newsem->nwaiters = 0;
>  #endif
>        /* This always is a shared semaphore.  */
> -      sem.newsem.private = LLL_SHARED;
> -
> -      /* Initialize the remaining bytes as well.  */
> -      memset ((char *) &sem.initsem + sizeof (struct new_sem), '\0',
> -             sizeof (sem_t) - sizeof (struct new_sem));
> +      newsem->private = LLL_SHARED;
>
>        tmpfname = __alloca (shm_dirlen + sizeof SEM_SHM_PREFIX + 6);
>        char *xxxxxx = __mempcpy (tmpfname, shm_dir, shm_dirlen);
> @@ -242,7 +245,7 @@ sem_open (const char *name, int oflag, ...)
>           break;
>         }
>
> -      if (TEMP_FAILURE_RETRY (__libc_write (fd, &sem.initsem, sizeof
> (sem_t)))
> +      if (TEMP_FAILURE_RETRY (__libc_write (fd, &sem, sizeof (sem_t)))
>           == sizeof (sem_t)
>           /* Map the sem_t structure from the file.  */
>           && (result = (sem_t *) mmap (NULL, sizeof (sem_t),



-- 
H.J.