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

[Torvald Riegel <triegel at redhat dot com>]

On Mon, 2015-01-26 at 23:31 -0500, Carlos O'Donell wrote:
> On 01/26/2015 06:35 PM, Chris Metcalf 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.
> 
> I'm not all that happy with this patch.
> 
> For example a statically compiled application sharing a semaphore
> with a dynamically compiled application would appear to break
> under your changes and nothing you can do would fix it. They each
> have different layouts for the semaphore. This is a common problem
> when switching internal layouts (like we did from Linuxthreads to
> NPTL). I accept that you may want to make this change and that this
> particular case might be unsupportable, but I want more discussion
> on the topic.

I'm not quite sure what you are concerned about.  The sem_t structure
exposed to programs does not change.  I agree the internal layout
changes, depending on the address of the sem_t instance.  This is
similar to having a pointer in sem_t pointing to within this instance.
One isn't allowed to do bitwise copy though, so once initialized at an
address, an instance can't move to another address (with the exception
of mapping at a different page, but that won't change the within-a-page
offset, which really matters here).

If we have a statically compiled application sharing a semaphore with a
dynamically compiled application, then if they use the same glibc code,
I don't see what could break.
If they don't, things will break, but even without the patch by Chris.

So, are you concerned about process-shared pthreads objects where the
glibc code used on both sides differs?  We can decide to never change
any of such code in incompatible ways (and this might already happen
just if we remove barriers or read from it differently!) -- but then we
won't be able to fix certain bugs (eg, this semaphore bug).  I also
don't see how versioning could help -- the (process-)sharing between the
applications happens at the source code level, and we have no versioning
support built into the data representation, so we can't detect the
difference when sharing.

> Similarly H.J's comment to change the alignment of the type
> is equally flawed as embedded sem_t's in other types would break
> other ABIs down the line. I see he's commented on that in a later
> down-thread post.

I don't think it's the same, but maybe I don't understand your concern.

> As far as I can tell the only immediately kosher solution is for
> these machines, that can't use 64-bit atomics because the
> alignment of their sem_t is not correct, is to use 32-bit atomics
> (for now). The choice of 32-bit atomics in no way prevents you
> from future 64-bit atomic uses. You can switch AFAICT to another
> internal implementation at a later date.

I agree that this is possible.  But not that this will change the
internal representation of struct new_sem too.