[PATCH 0/1] Allow 64 bit atomics in glibc 2.22 for 32bit SDK

[PATCH 0/1] Allow 64 bit atomics in glibc 2.22 for 32bit SDK

[Juro Bystricky]

Investigating the bug https://bugzilla.yoctoproject.org/show_bug.cgi?id=8140
points to a new bug in glibc that first appeared in 2.21, in particular in
libpthread-2.21.so and is still present in libpthread-2.22.so. 
Replacing these libraries with libpthread-2.20.so or libpthread-2.19.so fixes 
the original bug [YOCTO#8140]. 
The code between the 2.20 and 2.21 has changed quite a bit, so it is not easy 
to point out the exact culprit, but it is somewhere in the semaphore handling code.

This patch provides a workaround for the problem. The patch only affects
32 bit native SDKs. The patch consists of allowing 64bit atomic operations 
for x86. It is safe for Pentium and above, and as a matter of fact it is the best
fix available for these platforms in a sense that it not only fixes the problem
but also results in a more efficient code. It makes the need to find the actual bug
not neccessary (although, time-permitting I will try to determine the exact
root cause of the bug)

There is a question of why other 32 bit images do not exhibit this bug.
I don't know. It may be related to the way python multiprocessing module handles
semaphores/locking/queues. 

Any feedback welcome.



Juro Bystricky (1):
  glibc: Allow 64 bit atomics for x86

 .../glibc/glibc/use_64bit_atomics.patch            | 24 ++++++++++++++++++++++
 meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
 2 files changed, 25 insertions(+)
 create mode 100644 meta/recipes-core/glibc/glibc/use_64bit_atomics.patch

-- 
1.9.1

[PATCH 1/1] glibc: Allow 64 bit atomics for x86

[Juro Bystricky]

This patch fixes [YOCTO#8140].

The fix consist of allowing 64bit atomic ops for x86.
This should be safe for i586 and newer CPUs.
It also makes the synchronization more efficient.

Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
---
 .../glibc/glibc/use_64bit_atomics.patch            | 24 ++++++++++++++++++++++
 meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
 2 files changed, 25 insertions(+)
 create mode 100644 meta/recipes-core/glibc/glibc/use_64bit_atomics.patch

diff --git a/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch b/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
new file mode 100644
index 0000000..eb7f2b2
--- /dev/null
+++ b/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
@@ -0,0 +1,24 @@
+This patch alows using 64 bit atomic instructions on a
+32 bit platform. This is safe, providing x86 is Pentium or
+later (would not work on i386, i486). Using 64 bit atomic 
+instructions bypasses code containing a bug as documented in
+https://bugzilla.yoctoproject.org/show_bug.cgi?id=8140
+
+Upstream-Status: TBD
+
+Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
+
+
+Index: libc/sysdeps/i386/i486/bits/atomic.h
+===================================================================
+--- libc.orig/sysdeps/i386/i486/bits/atomic.h
++++ libc/sysdeps/i386/i486/bits/atomic.h
+@@ -54,7 +54,7 @@ typedef uintmax_t uatomic_max_t;
+ # endif
+ #endif
+ 
+-#define __HAVE_64B_ATOMICS 0
++#define __HAVE_64B_ATOMICS 1
+ #define USE_ATOMIC_COMPILER_BUILTINS 0
+ 
+ 
diff --git a/meta/recipes-core/glibc/glibc_2.22.bb b/meta/recipes-core/glibc/glibc_2.22.bb
index 020e417..2494ad7 100644
--- a/meta/recipes-core/glibc/glibc_2.22.bb
+++ b/meta/recipes-core/glibc/glibc_2.22.bb
@@ -50,6 +50,7 @@ SRC_URI_append_class-nativesdk = "\
            file://0001-nativesdk-glibc-Look-for-host-system-ld.so.cache-as-.patch \
            file://0002-nativesdk-glibc-Fix-buffer-overrun-with-a-relocated-.patch \
            file://0003-nativesdk-glibc-Raise-the-size-of-arrays-containing-.patch \
+           file://use_64bit_atomics.patch \
 "
 
 S = "${WORKDIR}/git"
-- 
1.9.1

Re: [PATCH 1/1] glibc: Allow 64 bit atomics for x86

[Mark Hatle]

On 10/29/15 3:43 PM, Juro Bystricky wrote:
> This patch fixes [YOCTO#8140].
> 
> The fix consist of allowing 64bit atomic ops for x86.
> This should be safe for i586 and newer CPUs.
> It also makes the synchronization more efficient.

I'm not sure this is correct.. see below.

> Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> ---
>  .../glibc/glibc/use_64bit_atomics.patch            | 24 ++++++++++++++++++++++
>  meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
>  2 files changed, 25 insertions(+)
>  create mode 100644 meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> 
> diff --git a/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch b/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> new file mode 100644
> index 0000000..eb7f2b2
> --- /dev/null
> +++ b/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> @@ -0,0 +1,24 @@
> +This patch alows using 64 bit atomic instructions on a
> +32 bit platform. This is safe, providing x86 is Pentium or
> +later (would not work on i386, i486). Using 64 bit atomic 
> +instructions bypasses code containing a bug as documented in
> +https://bugzilla.yoctoproject.org/show_bug.cgi?id=8140
> +
> +Upstream-Status: TBD
> +
> +Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> +
> +
> +Index: libc/sysdeps/i386/i486/bits/atomic.h
> +===================================================================
> +--- libc.orig/sysdeps/i386/i486/bits/atomic.h
> ++++ libc/sysdeps/i386/i486/bits/atomic.h
> +@@ -54,7 +54,7 @@ typedef uintmax_t uatomic_max_t;
> + # endif
> + #endif
> + 
> +-#define __HAVE_64B_ATOMICS 0
> ++#define __HAVE_64B_ATOMICS 1
> + #define USE_ATOMIC_COMPILER_BUILTINS 0

The patch will enable it for i486 and above, but you comment says i486 isn't
supported.

Normally you'd resolve this by creating a new

sysdeps/i386/i586/bits/atomic.h

(include or copy the original file)

and adjust the HAVE_64B_ATOMICS value at that point.

Then the sources will work for older systems and newer.

While 386/486 is likely not used much these days... I do also have a concern
about Quark CPUs.  Do you know if they support the 64B ATOMICS?  (I suspect they
do not.)  If that is the case, we might need to special case this patch and make
it 'i686' and above?

--Mark

> + 
> diff --git a/meta/recipes-core/glibc/glibc_2.22.bb b/meta/recipes-core/glibc/glibc_2.22.bb
> index 020e417..2494ad7 100644
> --- a/meta/recipes-core/glibc/glibc_2.22.bb
> +++ b/meta/recipes-core/glibc/glibc_2.22.bb
> @@ -50,6 +50,7 @@ SRC_URI_append_class-nativesdk = "\
>             file://0001-nativesdk-glibc-Look-for-host-system-ld.so.cache-as-.patch \
>             file://0002-nativesdk-glibc-Fix-buffer-overrun-with-a-relocated-.patch \
>             file://0003-nativesdk-glibc-Raise-the-size-of-arrays-containing-.patch \
> +           file://use_64bit_atomics.patch \
>  "
>  
>  S = "${WORKDIR}/git"
> 

Re: [PATCH 1/1] glibc: Allow 64 bit atomics for x86

[Bystricky, Juro]

Thanks for your quick reply.
Yes, the way you propose to apply the patch is cleaner, leaving i386 and i486 unaffected. 
However, they do not work already, so I cannot  do them any more harm.
Quark does support cmpxchg8b, so in theory it should work. But as you pointed out,
there are many errata for Quark atomic ops, so it is not guaranteed.




> -----Original Message-----
> From: Mark Hatle [mailto:mark.hatle@windriver.com]
> Sent: Thursday, October 29, 2015 2:01 PM
> To: Bystricky, Juro; openembedded-core@lists.openembedded.org;
> jurobystricky@hotmail.com
> Subject: Re: [OE-core] [PATCH 1/1] glibc: Allow 64 bit atomics for x86
> 
> On 10/29/15 3:43 PM, Juro Bystricky wrote:
> > This patch fixes [YOCTO#8140].
> >
> > The fix consist of allowing 64bit atomic ops for x86.
> > This should be safe for i586 and newer CPUs.
> > It also makes the synchronization more efficient.
> 
> I'm not sure this is correct.. see below.
> 
> > Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> > ---
> >  .../glibc/glibc/use_64bit_atomics.patch            | 24
> ++++++++++++++++++++++
> >  meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
> >  2 files changed, 25 insertions(+)
> >  create mode 100644
> > meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> >
> > diff --git a/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> > b/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> > new file mode 100644
> > index 0000000..eb7f2b2
> > --- /dev/null
> > +++ b/meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> > @@ -0,0 +1,24 @@
> > +This patch alows using 64 bit atomic instructions on a
> > +32 bit platform. This is safe, providing x86 is Pentium or later
> > +(would not work on i386, i486). Using 64 bit atomic instructions
> > +bypasses code containing a bug as documented in
> > +https://bugzilla.yoctoproject.org/show_bug.cgi?id=8140
> > +
> > +Upstream-Status: TBD
> > +
> > +Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> > +
> > +
> > +Index: libc/sysdeps/i386/i486/bits/atomic.h
> >
> +=========================================================
> ==========
> > +--- libc.orig/sysdeps/i386/i486/bits/atomic.h
> > ++++ libc/sysdeps/i386/i486/bits/atomic.h
> > +@@ -54,7 +54,7 @@ typedef uintmax_t uatomic_max_t;  # endif  #endif
> > +
> > +-#define __HAVE_64B_ATOMICS 0
> > ++#define __HAVE_64B_ATOMICS 1
> > + #define USE_ATOMIC_COMPILER_BUILTINS 0
> 
> The patch will enable it for i486 and above, but you comment says i486 isn't
> supported.
> 
> Normally you'd resolve this by creating a new
> 
> sysdeps/i386/i586/bits/atomic.h
> 
> (include or copy the original file)
> 
> and adjust the HAVE_64B_ATOMICS value at that point.
> 
> Then the sources will work for older systems and newer.
> 
> While 386/486 is likely not used much these days... I do also have a concern
> about Quark CPUs.  Do you know if they support the 64B ATOMICS?  (I
> suspect they do not.)  If that is the case, we might need to special case this
> patch and make it 'i686' and above?
> 
> --Mark
> 
> > +
> > diff --git a/meta/recipes-core/glibc/glibc_2.22.bb
> > b/meta/recipes-core/glibc/glibc_2.22.bb
> > index 020e417..2494ad7 100644
> > --- a/meta/recipes-core/glibc/glibc_2.22.bb
> > +++ b/meta/recipes-core/glibc/glibc_2.22.bb
> > @@ -50,6 +50,7 @@ SRC_URI_append_class-nativesdk = "\
> >             file://0001-nativesdk-glibc-Look-for-host-system-ld.so.cache-as-
> .patch \
> >             file://0002-nativesdk-glibc-Fix-buffer-overrun-with-a-relocated-
> .patch \
> >
> > file://0003-nativesdk-glibc-Raise-the-size-of-arrays-containing-.patch
> > \
> > +           file://use_64bit_atomics.patch \
> >  "
> >
> >  S = "${WORKDIR}/git"
> >

Re: [PATCH 1/1] glibc: Allow 64 bit atomics for x86

[Richard Purdie]

On Thu, 2015-10-29 at 13:43 -0700, Juro Bystricky wrote:
> This patch fixes [YOCTO#8140].
> 
> The fix consist of allowing 64bit atomic ops for x86.
> This should be safe for i586 and newer CPUs.
> It also makes the synchronization more efficient.
> 
> Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> ---
>  .../glibc/glibc/use_64bit_atomics.patch            | 24 ++++++++++++++++++++++
>  meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
>  2 files changed, 25 insertions(+)
>  create mode 100644 meta/recipes-core/glibc/glibc/use_64bit_atomics.patch

Since the patch is only changing nativesdk 32 bit x86 and we know that
the 32 bit SDK is pretty broken at the moment I've merged this on the
basis that it can't really make it any worse. There is pressure to move
to the next rc candidate for 2.0.

I would like to get to the bottom of the real issue here and I still
suspect 32 bit x86 images are likely broken too :/.

Cheers,

Richard

Re: [PATCH 1/1] glibc: Allow 64 bit atomics for x86

[Randy Witt]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

On Thu, Oct 29, 2015 at 3:20 PM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Thu, 2015-10-29 at 13:43 -0700, Juro Bystricky wrote:
> > This patch fixes [YOCTO#8140].
> >
> > The fix consist of allowing 64bit atomic ops for x86.
> > This should be safe for i586 and newer CPUs.
> > It also makes the synchronization more efficient.
> >
> > Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> > ---
> >  .../glibc/glibc/use_64bit_atomics.patch            | 24
> ++++++++++++++++++++++
> >  meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
> >  2 files changed, 25 insertions(+)
> >  create mode 100644 meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
>
> Since the patch is only changing nativesdk 32 bit x86 and we know that
> the 32 bit SDK is pretty broken at the moment I've merged this on the
> basis that it can't really make it any worse. There is pressure to move
> to the next rc candidate for 2.0.
>
> I would like to get to the bottom of the real issue here and I still
> suspect 32 bit x86 images are likely broken too :/.
>

With Juro's change I built core-image-minimal using a 32-bit buildtools on
a 32-bit host(vm) and that appears to boot fine using runqemu.


> Cheers,
>
> Richard
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

[-- Attachment #2: Type: text/html, Size: 2425 bytes --]

Re: [PATCH 1/1] glibc: Allow 64 bit atomics for x86

[Richard Purdie]

On Thu, 2015-10-29 at 15:29 -0700, Randy Witt wrote:
> 
> 
> On Thu, Oct 29, 2015 at 3:20 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>         On Thu, 2015-10-29 at 13:43 -0700, Juro Bystricky wrote:
>         > This patch fixes [YOCTO#8140].
>         >
>         > The fix consist of allowing 64bit atomic ops for x86.
>         > This should be safe for i586 and newer CPUs.
>         > It also makes the synchronization more efficient.
>         >
>         > Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
>         > ---
>         >  .../glibc/glibc/use_64bit_atomics.patch            | 24
>         ++++++++++++++++++++++
>         >  meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
>         >  2 files changed, 25 insertions(+)
>         >  create mode 100644
>         meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
>         
>         Since the patch is only changing nativesdk 32 bit x86 and we
>         know that
>         the 32 bit SDK is pretty broken at the moment I've merged this
>         on the
>         basis that it can't really make it any worse. There is
>         pressure to move
>         to the next rc candidate for 2.0.
>         
>         I would like to get to the bottom of the real issue here and I
>         still
>         suspect 32 bit x86 images are likely broken too :/.
>
>
> With Juro's change I built core-image-minimal using a 32-bit
> buildtools on a 32-bit host(vm) and that appears to boot fine using
> runqemu.

Right, but can we run bitbake in a qemux86 image (on target) or does it
show the same issue as bug 8140? If it shows the same issue, futexes are
likely bust on 32 bit x86 targets too.

Cheers,

Richard

Re: [PATCH 1/1] glibc: Allow 64 bit atomics for x86

[Bystricky, Juro]

So I put my insomnia to a good use and I think I can explain everything.
The offending code is in nptl/sem_wait.c:

#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_1)
int
attribute_compat_text_section
__old_sem_wait (sem_t *sem)
{
  int *futex = (int *) sem;
  int err;

  do
    {
      if (atomic_decrement_if_positive (futex) > 0)
	return 0;

      /* Enable asynchronous cancellation.  Required by the standard.  */
      int oldtype = __pthread_enable_asynccancel ();

      /* Always assume the semaphore is shared.  */
      err = lll_futex_wait (futex, 0, LLL_SHARED);

      /* Disable asynchronous cancellation.  */
      __pthread_disable_asynccancel (oldtype);
    }
  while (err == 0 || err == -EWOULDBLOCK);

  __set_errno (-err);
  return -1;
}

This is where we were looping forever, since the value in futex was -2.
So "atomic_decrement_if_positive" was not executed, never reaching "return 0".
The reason why the value was -2 is because struct_new_sem has changed from glibc 2.20 to 2.221:

/* Semaphore variable structure.  */
struct new_sem
{
#if __HAVE_64B_ATOMICS
  /* The data field holds both value (in the least-significant 32 bytes) and
     nwaiters.  */
# if __BYTE_ORDER == __LITTLE_ENDIAN
#  define SEM_VALUE_OFFSET 0
# elif __BYTE_ORDER == __BIG_ENDIAN
#  define SEM_VALUE_OFFSET 1
# else
# error Unsupported byte order.
# endif
# define SEM_NWAITERS_SHIFT 32
# define SEM_VALUE_MASK (~(unsigned int)0)
  uint64_t data;
  int private;
  int pad;
#else
# define SEM_VALUE_SHIFT 1
# define SEM_NWAITERS_MASK ((unsigned int)1)
  unsigned int value;
  unsigned int nwaiters;
  int private;
  int pad;
#endif
};

In particular for __HAVE_64B_ATOMICS == 0 the field "value" has a different meaning now:
top 31 bits correspond to the value (shifted left by 1) and the bit 0 indicates if there are any nwaiters.
(I guess the reasoning was semaphore value cannot be negative and so it only needs 31 bits
and we can use the extra bit we saved for something else) 
So when we initialize the semaphore value with SEM_VALUE_MAX   (2147483647) we actually store 
0x7FFFFFFF << 1 which is 0xFFFFFFFE which is  (-2). 
(bitbake/pyhon/multiprocessing initialize some semaphores to SEM_VALUE_MAX)    
So the reason forcing  __HAVE_64B_ATOMICS to 1 works for x32 has not much to do with the 
64bit atomics as such, but because the "value" filed is defined differently, as a 32bit value.
Long story short,  the reason this is broken is that the "old code" uses new structures. 
(to fix this   "atomic_decrement_if_positive"  needs to be replaced by a call that understands
the new layout, similar to the "new code")

I will report  this to glibc developers and wait for their reply.

Juro




> -----Original Message-----
> From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
> Sent: Friday, October 30, 2015 12:13 AM
> To: Randy Witt
> Cc: Bystricky, Juro; jurobystricky@hotmail.com; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH 1/1] glibc: Allow 64 bit atomics for x86
> 
> On Thu, 2015-10-29 at 15:29 -0700, Randy Witt wrote:
> >
> >
> > On Thu, Oct 29, 2015 at 3:20 PM, Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> >         On Thu, 2015-10-29 at 13:43 -0700, Juro Bystricky wrote:
> >         > This patch fixes [YOCTO#8140].
> >         >
> >         > The fix consist of allowing 64bit atomic ops for x86.
> >         > This should be safe for i586 and newer CPUs.
> >         > It also makes the synchronization more efficient.
> >         >
> >         > Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> >         > ---
> >         >  .../glibc/glibc/use_64bit_atomics.patch            | 24
> >         ++++++++++++++++++++++
> >         >  meta/recipes-core/glibc/glibc_2.22.bb              |  1 +
> >         >  2 files changed, 25 insertions(+)
> >         >  create mode 100644
> >         meta/recipes-core/glibc/glibc/use_64bit_atomics.patch
> >
> >         Since the patch is only changing nativesdk 32 bit x86 and we
> >         know that
> >         the 32 bit SDK is pretty broken at the moment I've merged this
> >         on the
> >         basis that it can't really make it any worse. There is
> >         pressure to move
> >         to the next rc candidate for 2.0.
> >
> >         I would like to get to the bottom of the real issue here and I
> >         still
> >         suspect 32 bit x86 images are likely broken too :/.
> >
> >
> > With Juro's change I built core-image-minimal using a 32-bit
> > buildtools on a 32-bit host(vm) and that appears to boot fine using
> > runqemu.
> 
> Right, but can we run bitbake in a qemux86 image (on target) or does it show
> the same issue as bug 8140? If it shows the same issue, futexes are likely
> bust on 32 bit x86 targets too.
> 
> Cheers,
> 
> Richard
> 
> 
> 
>