[Bug nptl/16796] New: [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[Bug nptl/16796] New: [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[michael.hudson at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

            Bug ID: 16796
           Summary: [aarch64] pthread_self not aligned to 16 bytes when
                    heap grows up
           Product: glibc
           Version: 2.19
            Status: NEW
          Severity: normal
          Priority: P2
         Component: nptl
          Assignee: unassigned at sourceware dot org
          Reporter: michael.hudson at linaro dot org
                CC: drepper.fsp at gmail dot com

Hi,

There is a test in glibc (tst-tls5) that tests that
((uintptr_t)pthread_self())%16 is zero.  But watch this:

(t-mwhudson)mwhudson@am1:~$ cat btp.c 
#include <stdint.h>
#include <stdio.h>
#include <pthread.h>

int
main(int argc, char** argv)
{
        uintptr_t p = (uintptr_t)__builtin_thread_pointer();
        uintptr_t q = (uintptr_t)pthread_self();
        printf("p: %lx %ld\n", p, p%16);
        printf("q: %lx %ld\n", q, q%16);
}
(t-mwhudson)mwhudson@am1:~$ gcc -o btp btp.c -lpthread
(t-mwhudson)mwhudson@am1:~$ ulimit -s unlimited
(t-mwhudson)mwhudson@am1:~$ ./btp
p: 2000028d88 8
q: 2000028698 8
(t-mwhudson)mwhudson@am1:~$ ulimit -S -s 8192
(t-mwhudson)mwhudson@am1:~$ ./btp
p: 7f7fd086f0 0
q: 7f7fd08000 0

So something is clearly wrong; maybe it's just that the test is too strict, but
somehow that seems a bit unlikely.  FWIW, this doesn't happen if you don't link
with libpthread so maaaaybe it's a bug in something that ends up in
libpthread's .init section?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[michael.hudson at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

Michael Hudson-Doyle <michael.hudson at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |adconrad at 0c3 dot net,
                   |                            |michael.hudson at linaro dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[pinskia at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu.org

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Is this on aarch64?
If so I am have a fix:
diff --git a/sysdeps/aarch64/nptl/tls.h b/sysdeps/aarch64/nptl/tls.h
index 36af3a8..05135db 100644
--- a/sysdeps/aarch64/nptl/tls.h
+++ b/sysdeps/aarch64/nptl/tls.h
@@ -62,7 +62,7 @@ typedef struct
 # define TLS_INIT_TCB_SIZE    sizeof (tcbhead_t)

 /* Alignment requirements for the initial TCB.  */
-# define TLS_INIT_TCB_ALIGN    __alignof__ (tcbhead_t)
+# define TLS_INIT_TCB_ALIGN    __alignof__ (struct pthread)

 /* This is the size of the TCB.  */
 # define TLS_TCB_SIZE        sizeof (tcbhead_t)
@@ -71,7 +71,7 @@ typedef struct
 # define TLS_PRE_TCB_SIZE    sizeof (struct pthread)

 /* Alignment requirements for the TCB.  */
-# define TLS_TCB_ALIGN        __alignof__ (tcbhead_t)
+# define TLS_TCB_ALIGN        __alignof__ (struct pthread)

 /* Install the dtv pointer.  The pointer passed is to the element with
    index -1 which contain the length.  */

--- CUT ----
The problem is we are using the incorrect alignment here when we create the TLS
section in memory.  As I said on the linaro list, I found this while working on
ILP32 and sotruss was failing.  I will submit this patch once I do a build and
test using the upstream glibc.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[pinskia at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |aarch64*-linux-gnu
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at sourceware dot org   |pinskia at gcc dot gnu.org

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Actually the summary says it is on aarch64 so taking this over.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[pinskia at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note the testcase in glibc does:
__alignof__ (struct pthread);

And not really 16 itself so the testcase is correct and my patch fixes the bug
correctly and follows what other targets (x86) does.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[michael.hudson at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

--- Comment #4 from Michael Hudson-Doyle <michael.hudson at linaro dot org> ---
Hi, that patch fixes things for me, thanks.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  15eaf6ffe3e117684a0e7b070c0a8754480d3fa3 (commit)
       via  75f11331f98ebf3873e887a683add944a1aec0fd (commit)
      from  c3ec475c5dd16499aa040908e11d382c3ded9692 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=15eaf6ffe3e117684a0e7b070c0a8754480d3fa3

commit 15eaf6ffe3e117684a0e7b070c0a8754480d3fa3
Author: Siddhesh Poyarekar <siddhesh@redhat.com>
Date:   Sat Feb 22 10:09:27 2014 +0530

    benchtests: Add new directive for benchmark initialization hook

    Add a new 'init' directive that specifies the name of the function to
    call to do function-specific initialization.  This is useful for
    benchmarks that need to do a one-time initialization before the
    functions are executed.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=75f11331f98ebf3873e887a683add944a1aec0fd

commit 75f11331f98ebf3873e887a683add944a1aec0fd
Author: Kyle McMartin <kmcmarti@redhat.com>
Date:   Mon May 26 12:33:22 2014 +0530

    [AARCH64] correct alignment of TLS_TCB_ALIGN (BZ #16796)

    This fixes a variety of testsuite failures for me:
    tststatic.out Error 1
    tststatic2.out Error 1
    tst-tls9-static.out Error 1
    tst-audit8.out Error 127
    tst-audit9.out Error 127
    tst-audit1.out Error 127
    and also has the added benefit of making LD_AUDIT/sotruss work on
    AArch64.

    Otherwise, we bail out early in _dl_try_allocate_static_tls as the
    alignment requirement of the PT_TLS section in libc is 16.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                   |   14 ++++++++++++++
 NEWS                        |    8 ++++----
 benchtests/README           |    1 +
 benchtests/bench-skeleton.c |    3 +++
 benchtests/scripts/bench.py |    7 ++++++-
 sysdeps/aarch64/nptl/tls.h  |    4 ++--
 6 files changed, 30 insertions(+), 7 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[siddhesh at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

Siddhesh Poyarekar <siddhesh at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
                 CC|                            |siddhesh at redhat dot com
         Resolution|---                         |FIXED

--- Comment #6 from Siddhesh Poyarekar <siddhesh at redhat dot com> ---
Fixed in master.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/16796] [aarch64] pthread_self not aligned to 16 bytes when heap grows up

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16796

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.