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

[Chris Metcalf <cmetcalf at ezchip dot com>]

On 1/27/2015 6:40 AM, H.J. Lu wrote:
> On Mon, Jan 26, 2015 at 3:35 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> > --- 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.

It is not needed.  Torvald's claim was that it was more stylistically
consistent to use to_new_sem() in all cases, even here.  I am personally
on the fence about this; I think the new code and old code are both
plausible ways to represent the issue with the required alignment.

If Torvald prefers the new code and you prefer the old code, perhaps
we will need to ask for a tie-breaker vote. :-)

--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com