public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] elf: Change TLS static surplus default back to 1664
@ 2020-07-17 11:41 Florian Weimer
  2020-07-17 16:22 ` Carlos O'Donell
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Florian Weimer @ 2020-07-17 11:41 UTC (permalink / raw)
  To: libc-alpha

Make the computation in elf/dl-tls.c more transparent, and add
an explicit test for the historic value.

---
 elf/Makefile          |  4 +++-
 elf/dl-tls.c          | 37 ++++++++++++++++++++++++++++++-------
 elf/tst-tls-surplus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/elf/Makefile b/elf/Makefile
index a2c3b12007..0b78721848 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -216,7 +216,7 @@ tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
 	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
 	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
-	 tst-create_format1
+	 tst-create_format1 tst-tls-surplus
 tests-container += tst-pldd tst-dlopen-tlsmodid-container \
   tst-dlopen-self-container
 test-srcs = tst-pathopt
@@ -1794,3 +1794,5 @@ $(objpfx)tst-tls-ie-dlmopen.out: \
   $(objpfx)tst-tls-ie-mod4.so \
   $(objpfx)tst-tls-ie-mod5.so \
   $(objpfx)tst-tls-ie-mod6.so
+
+$(objpfx)tst-tls-surplus: $(libdl)
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 9a17427047..9fa62f5d22 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -54,13 +54,37 @@
    Audit modules use their own namespaces, they are not included in rtld.nns,
    but come on top when computing the number of namespaces.  */
 
-/* Size of initial-exec TLS in libc.so.  */
-#define LIBC_IE_TLS 160
+/* Size of initial-exec TLS in libc.so.  This should be the maximum of
+   observed PT_GNU_TLS sizes across all architectures.  Some
+   architectures have lower values due to differences in type sizes
+   and link editor capabilities.  */
+#define LIBC_IE_TLS 144
+
 /* Size of initial-exec TLS in libraries other than libc.so.
    This should be large enough to cover runtime libraries of the
    compiler such as libgomp and libraries in libc other than libc.so.  */
 #define OTHER_IE_TLS 144
 
+/* Default number of namespaces.  */
+#define DEFAULT_NNS 4
+
+/* Default for dl_tls_static_optional.  */
+#define OPTIONAL_TLS 512
+
+/* Compute the static TLS surplus based on the namespace count and the
+   TLS space that can be used for optimizations.  */
+static inline int
+tls_static_surplus (int nns, int opt_tls)
+{
+  return (nns - 1) * LIBC_IE_TLS + nns * OTHER_IE_TLS + opt_tls;
+}
+
+/* This value is chosen so that with default values for the tunables,
+   the computation of dl_tls_static_surplus in
+   _dl_tls_static_surplus_init yields the historic value 1664, for
+   backwards compatibility.  */
+#define LEGACY_TLS (1664 - tls_static_surplus (DEFAULT_NNS, OPTIONAL_TLS))
+
 /* Calculate the size of the static TLS surplus, when the given
    number of audit modules are loaded.  Must be called after the
    number of audit modules is known and before static TLS allocation.  */
@@ -74,8 +98,8 @@ _dl_tls_static_surplus_init (size_t naudit)
   opt_tls = TUNABLE_GET (optional_static_tls, size_t, NULL);
 #else
   /* Default values of the tunables.  */
-  nns = 4;
-  opt_tls = 512;
+  nns = DEFAULT_NNS;
+  opt_tls = OPTIONAL_TLS;
 #endif
   if (nns > DL_NNS)
     nns = DL_NNS;
@@ -85,9 +109,8 @@ _dl_tls_static_surplus_init (size_t naudit)
   nns += naudit;
 
   GL(dl_tls_static_optional) = opt_tls;
-  GLRO(dl_tls_static_surplus) = ((nns - 1) * LIBC_IE_TLS
-				 + nns * OTHER_IE_TLS
-				 + opt_tls);
+  assert (LEGACY_TLS >= 0);
+  GLRO(dl_tls_static_surplus) = tls_static_surplus (nns, opt_tls) + LEGACY_TLS;
 }
 
 /* Out-of-memory handler.  */
diff --git a/elf/tst-tls-surplus.c b/elf/tst-tls-surplus.c
new file mode 100644
index 0000000000..b0dea0b5ee
--- /dev/null
+++ b/elf/tst-tls-surplus.c
@@ -0,0 +1,42 @@
+/* Test size of the static TLS surplus reservation for backwards compatibility.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <support/check.h>
+#include <support/xdlfcn.h>
+
+static int do_test (void);
+#include <support/test-driver.c>
+
+/* This hack results in a definition of struct rtld_global_ro.  Do
+   this after all the other header inclusions, to minimize the
+   impact.  */
+#define SHARED
+#include <ldsodefs.h>
+
+static
+int do_test (void)
+{
+  /* Avoid introducing a copy relocation due to the hidden alias in
+     ld.so.  */
+  struct rtld_global_ro *glro = xdlsym (NULL, "_rtld_global_ro");
+  printf ("info: _dl_tls_static_surplus: %zu\n", glro->_dl_tls_static_surplus);
+  /* Hisoric value: 16 * 100 + 64.  */
+  TEST_VERIFY (glro->_dl_tls_static_surplus >= 1664);
+  return 0;
+}


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] elf: Change TLS static surplus default back to 1664
  2020-07-17 11:41 [PATCH v2] elf: Change TLS static surplus default back to 1664 Florian Weimer
@ 2020-07-17 16:22 ` Carlos O'Donell
  2020-07-17 20:07 ` Matheus Castanho
  2020-07-19  4:39 ` Vineet Gupta
  2 siblings, 0 replies; 5+ messages in thread
From: Carlos O'Donell @ 2020-07-17 16:22 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha

On 7/17/20 7:41 AM, Florian Weimer via Libc-alpha wrote:
> Make the computation in elf/dl-tls.c more transparent, and add
> an explicit test for the historic value.

OK for 2.32.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>


> ---
>  elf/Makefile          |  4 +++-
>  elf/dl-tls.c          | 37 ++++++++++++++++++++++++++++++-------
>  elf/tst-tls-surplus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+), 8 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index a2c3b12007..0b78721848 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -216,7 +216,7 @@ tests-internal += loadtest unload unload2 circleload1 \
>  	 neededtest neededtest2 neededtest3 neededtest4 \
>  	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
>  	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
> -	 tst-create_format1
> +	 tst-create_format1 tst-tls-surplus

OK. Verified this is using tests-internal.

>  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
>    tst-dlopen-self-container
>  test-srcs = tst-pathopt
> @@ -1794,3 +1794,5 @@ $(objpfx)tst-tls-ie-dlmopen.out: \
>    $(objpfx)tst-tls-ie-mod4.so \
>    $(objpfx)tst-tls-ie-mod5.so \
>    $(objpfx)tst-tls-ie-mod6.so
> +
> +$(objpfx)tst-tls-surplus: $(libdl)

OK.

> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 9a17427047..9fa62f5d22 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -54,13 +54,37 @@
>     Audit modules use their own namespaces, they are not included in rtld.nns,
>     but come on top when computing the number of namespaces.  */
>  
> -/* Size of initial-exec TLS in libc.so.  */
> -#define LIBC_IE_TLS 160
> +/* Size of initial-exec TLS in libc.so.  This should be the maximum of
> +   observed PT_GNU_TLS sizes across all architectures.  Some
> +   architectures have lower values due to differences in type sizes
> +   and link editor capabilities.  */
> +#define LIBC_IE_TLS 144

OK. Good comment.

> +
>  /* Size of initial-exec TLS in libraries other than libc.so.
>     This should be large enough to cover runtime libraries of the
>     compiler such as libgomp and libraries in libc other than libc.so.  */
>  #define OTHER_IE_TLS 144
>  
> +/* Default number of namespaces.  */
> +#define DEFAULT_NNS 4
> +
> +/* Default for dl_tls_static_optional.  */
> +#define OPTIONAL_TLS 512
> +
> +/* Compute the static TLS surplus based on the namespace count and the
> +   TLS space that can be used for optimizations.  */
> +static inline int
> +tls_static_surplus (int nns, int opt_tls)
> +{
> +  return (nns - 1) * LIBC_IE_TLS + nns * OTHER_IE_TLS + opt_tls;

OK.

> +}
> +
> +/* This value is chosen so that with default values for the tunables,
> +   the computation of dl_tls_static_surplus in
> +   _dl_tls_static_surplus_init yields the historic value 1664, for
> +   backwards compatibility.  */
> +#define LEGACY_TLS (1664 - tls_static_surplus (DEFAULT_NNS, OPTIONAL_TLS))

OK.

> +
>  /* Calculate the size of the static TLS surplus, when the given
>     number of audit modules are loaded.  Must be called after the
>     number of audit modules is known and before static TLS allocation.  */
> @@ -74,8 +98,8 @@ _dl_tls_static_surplus_init (size_t naudit)
>    opt_tls = TUNABLE_GET (optional_static_tls, size_t, NULL);
>  #else
>    /* Default values of the tunables.  */
> -  nns = 4;
> -  opt_tls = 512;
> +  nns = DEFAULT_NNS;
> +  opt_tls = OPTIONAL_TLS;

OK.

>  #endif
>    if (nns > DL_NNS)
>      nns = DL_NNS;
> @@ -85,9 +109,8 @@ _dl_tls_static_surplus_init (size_t naudit)
>    nns += naudit;
>  
>    GL(dl_tls_static_optional) = opt_tls;
> -  GLRO(dl_tls_static_surplus) = ((nns - 1) * LIBC_IE_TLS
> -				 + nns * OTHER_IE_TLS
> -				 + opt_tls);
> +  assert (LEGACY_TLS >= 0);
> +  GLRO(dl_tls_static_surplus) = tls_static_surplus (nns, opt_tls) + LEGACY_TLS;

OK.

>  }
>  
>  /* Out-of-memory handler.  */
> diff --git a/elf/tst-tls-surplus.c b/elf/tst-tls-surplus.c
> new file mode 100644
> index 0000000000..b0dea0b5ee
> --- /dev/null
> +++ b/elf/tst-tls-surplus.c
> @@ -0,0 +1,42 @@
> +/* Test size of the static TLS surplus reservation for backwards compatibility.

OK.

> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/xdlfcn.h>
> +
> +static int do_test (void);
> +#include <support/test-driver.c>
> +
> +/* This hack results in a definition of struct rtld_global_ro.  Do
> +   this after all the other header inclusions, to minimize the
> +   impact.  */

OK. :-)

> +#define SHARED
> +#include <ldsodefs.h>
> +
> +static
> +int do_test (void)
> +{
> +  /* Avoid introducing a copy relocation due to the hidden alias in
> +     ld.so.  */
> +  struct rtld_global_ro *glro = xdlsym (NULL, "_rtld_global_ro");
> +  printf ("info: _dl_tls_static_surplus: %zu\n", glro->_dl_tls_static_surplus);
> +  /* Hisoric value: 16 * 100 + 64.  */
> +  TEST_VERIFY (glro->_dl_tls_static_surplus >= 1664);

OK.

> +  return 0;
> +}
> 


-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] elf: Change TLS static surplus default back to 1664
  2020-07-17 11:41 [PATCH v2] elf: Change TLS static surplus default back to 1664 Florian Weimer
  2020-07-17 16:22 ` Carlos O'Donell
@ 2020-07-17 20:07 ` Matheus Castanho
  2020-07-19  4:39 ` Vineet Gupta
  2 siblings, 0 replies; 5+ messages in thread
From: Matheus Castanho @ 2020-07-17 20:07 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha

Hi Florian,

Thank you for taking a look at this. I confirm this patch fixes the
issues on powerpc, powerpc64 and powerpc64le.

--
Matheus Castanho

On 7/17/20 8:41 AM, Florian Weimer via Libc-alpha wrote:
> Make the computation in elf/dl-tls.c more transparent, and add
> an explicit test for the historic value.
> 
> ---
>  elf/Makefile          |  4 +++-
>  elf/dl-tls.c          | 37 ++++++++++++++++++++++++++++++-------
>  elf/tst-tls-surplus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+), 8 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index a2c3b12007..0b78721848 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -216,7 +216,7 @@ tests-internal += loadtest unload unload2 circleload1 \
>  	 neededtest neededtest2 neededtest3 neededtest4 \
>  	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
>  	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
> -	 tst-create_format1
> +	 tst-create_format1 tst-tls-surplus
>  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
>    tst-dlopen-self-container
>  test-srcs = tst-pathopt
> @@ -1794,3 +1794,5 @@ $(objpfx)tst-tls-ie-dlmopen.out: \
>    $(objpfx)tst-tls-ie-mod4.so \
>    $(objpfx)tst-tls-ie-mod5.so \
>    $(objpfx)tst-tls-ie-mod6.so
> +
> +$(objpfx)tst-tls-surplus: $(libdl)
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 9a17427047..9fa62f5d22 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -54,13 +54,37 @@
>     Audit modules use their own namespaces, they are not included in rtld.nns,
>     but come on top when computing the number of namespaces.  */
>  
> -/* Size of initial-exec TLS in libc.so.  */
> -#define LIBC_IE_TLS 160
> +/* Size of initial-exec TLS in libc.so.  This should be the maximum of
> +   observed PT_GNU_TLS sizes across all architectures.  Some
> +   architectures have lower values due to differences in type sizes
> +   and link editor capabilities.  */
> +#define LIBC_IE_TLS 144
> +
>  /* Size of initial-exec TLS in libraries other than libc.so.
>     This should be large enough to cover runtime libraries of the
>     compiler such as libgomp and libraries in libc other than libc.so.  */
>  #define OTHER_IE_TLS 144
>  
> +/* Default number of namespaces.  */
> +#define DEFAULT_NNS 4
> +
> +/* Default for dl_tls_static_optional.  */
> +#define OPTIONAL_TLS 512
> +
> +/* Compute the static TLS surplus based on the namespace count and the
> +   TLS space that can be used for optimizations.  */
> +static inline int
> +tls_static_surplus (int nns, int opt_tls)
> +{
> +  return (nns - 1) * LIBC_IE_TLS + nns * OTHER_IE_TLS + opt_tls;
> +}
> +
> +/* This value is chosen so that with default values for the tunables,
> +   the computation of dl_tls_static_surplus in
> +   _dl_tls_static_surplus_init yields the historic value 1664, for
> +   backwards compatibility.  */
> +#define LEGACY_TLS (1664 - tls_static_surplus (DEFAULT_NNS, OPTIONAL_TLS))
> +
>  /* Calculate the size of the static TLS surplus, when the given
>     number of audit modules are loaded.  Must be called after the
>     number of audit modules is known and before static TLS allocation.  */
> @@ -74,8 +98,8 @@ _dl_tls_static_surplus_init (size_t naudit)
>    opt_tls = TUNABLE_GET (optional_static_tls, size_t, NULL);
>  #else
>    /* Default values of the tunables.  */
> -  nns = 4;
> -  opt_tls = 512;
> +  nns = DEFAULT_NNS;
> +  opt_tls = OPTIONAL_TLS;
>  #endif
>    if (nns > DL_NNS)
>      nns = DL_NNS;
> @@ -85,9 +109,8 @@ _dl_tls_static_surplus_init (size_t naudit)
>    nns += naudit;
>  
>    GL(dl_tls_static_optional) = opt_tls;
> -  GLRO(dl_tls_static_surplus) = ((nns - 1) * LIBC_IE_TLS
> -				 + nns * OTHER_IE_TLS
> -				 + opt_tls);
> +  assert (LEGACY_TLS >= 0);
> +  GLRO(dl_tls_static_surplus) = tls_static_surplus (nns, opt_tls) + LEGACY_TLS;
>  }
>  
>  /* Out-of-memory handler.  */
> diff --git a/elf/tst-tls-surplus.c b/elf/tst-tls-surplus.c
> new file mode 100644
> index 0000000000..b0dea0b5ee
> --- /dev/null
> +++ b/elf/tst-tls-surplus.c
> @@ -0,0 +1,42 @@
> +/* Test size of the static TLS surplus reservation for backwards compatibility.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/xdlfcn.h>
> +
> +static int do_test (void);
> +#include <support/test-driver.c>
> +
> +/* This hack results in a definition of struct rtld_global_ro.  Do
> +   this after all the other header inclusions, to minimize the
> +   impact.  */
> +#define SHARED
> +#include <ldsodefs.h>
> +
> +static
> +int do_test (void)
> +{
> +  /* Avoid introducing a copy relocation due to the hidden alias in
> +     ld.so.  */
> +  struct rtld_global_ro *glro = xdlsym (NULL, "_rtld_global_ro");
> +  printf ("info: _dl_tls_static_surplus: %zu\n", glro->_dl_tls_static_surplus);
> +  /* Hisoric value: 16 * 100 + 64.  */
> +  TEST_VERIFY (glro->_dl_tls_static_surplus >= 1664);
> +  return 0;
> +}
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] elf: Change TLS static surplus default back to 1664
  2020-07-17 11:41 [PATCH v2] elf: Change TLS static surplus default back to 1664 Florian Weimer
  2020-07-17 16:22 ` Carlos O'Donell
  2020-07-17 20:07 ` Matheus Castanho
@ 2020-07-19  4:39 ` Vineet Gupta
  2020-07-20 14:33   ` Florian Weimer
  2 siblings, 1 reply; 5+ messages in thread
From: Vineet Gupta @ 2020-07-19  4:39 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha; +Cc: arcml

Hi Florian,

On 7/17/20 4:41 AM, Florian Weimer via Libc-alpha wrote:
> Make the computation in elf/dl-tls.c more transparent, and add
> an explicit test for the historic value.
> 
> ---
>  elf/Makefile          |  4 +++-
>  elf/dl-tls.c          | 37 ++++++++++++++++++++++++++++++-------
>  elf/tst-tls-surplus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+), 8 deletions(-)

On ARC, tst-tls-surplus passes, but it seems there's still some static TLS
shenanigans.

FAIL: elf/tst-tls-ie
FAIL: elf/tst-tls-ie-dlmopen

$ cat elf/tst-tls-ie.out

maintls[1000]:	 0x20026f80 .. 0x20027368
var0[480]:	 0x2093c648 .. 0x2093c828 global-dynamic
var1[120]:	 0x2093cc40 .. 0x2093ccb8 global-dynamic
var2[24]:	 0x2093d0d0 .. 0x2093d0e8 global-dynamic
var3[16]:	 0x2093d500 .. 0x2093d510 global-dynamic
error: xdlfcn.c:29: error: dlopen:
glibc-2.31.9000-730-ge9422236a2dd4/build/elf/tst-tls-ie-mod4.so: cannot allocate
memory in static TLS block

cat elf/tst-tls-ie-dlmopen.out
maintls[1000]:	 0x20026f80 .. 0x20027368
var0[480]:	 0x2093c648 .. 0x2093c828 global-dynamic
var1[120]:	 0x2093d3d8 .. 0x2093d450 global-dynamic
var2[24]:	 0x2093e000 .. 0x2093e018 global-dynamic
var3[16]:	 0x2093ebc8 .. 0x2093ebd8 global-dynamic
error: xdlmopen.c:28: error: dlmopen:
glibc-2.31.9000-730-ge9422236a2dd4/build/elf/tst-tls-ie-mod6.so: cannot allocate
memory in static TLS block


> 
> diff --git a/elf/Makefile b/elf/Makefile
> index a2c3b12007..0b78721848 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -216,7 +216,7 @@ tests-internal += loadtest unload unload2 circleload1 \
>  	 neededtest neededtest2 neededtest3 neededtest4 \
>  	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
>  	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
> -	 tst-create_format1
> +	 tst-create_format1 tst-tls-surplus
>  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
>    tst-dlopen-self-container
>  test-srcs = tst-pathopt
> @@ -1794,3 +1794,5 @@ $(objpfx)tst-tls-ie-dlmopen.out: \
>    $(objpfx)tst-tls-ie-mod4.so \
>    $(objpfx)tst-tls-ie-mod5.so \
>    $(objpfx)tst-tls-ie-mod6.so
> +
> +$(objpfx)tst-tls-surplus: $(libdl)
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 9a17427047..9fa62f5d22 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -54,13 +54,37 @@
>     Audit modules use their own namespaces, they are not included in rtld.nns,
>     but come on top when computing the number of namespaces.  */
>  
> -/* Size of initial-exec TLS in libc.so.  */
> -#define LIBC_IE_TLS 160
> +/* Size of initial-exec TLS in libc.so.  This should be the maximum of
> +   observed PT_GNU_TLS sizes across all architectures.  Some
> +   architectures have lower values due to differences in type sizes
> +   and link editor capabilities.  */
> +#define LIBC_IE_TLS 144
> +
>  /* Size of initial-exec TLS in libraries other than libc.so.
>     This should be large enough to cover runtime libraries of the
>     compiler such as libgomp and libraries in libc other than libc.so.  */
>  #define OTHER_IE_TLS 144
>  
> +/* Default number of namespaces.  */
> +#define DEFAULT_NNS 4
> +
> +/* Default for dl_tls_static_optional.  */
> +#define OPTIONAL_TLS 512
> +
> +/* Compute the static TLS surplus based on the namespace count and the
> +   TLS space that can be used for optimizations.  */
> +static inline int
> +tls_static_surplus (int nns, int opt_tls)
> +{
> +  return (nns - 1) * LIBC_IE_TLS + nns * OTHER_IE_TLS + opt_tls;
> +}
> +
> +/* This value is chosen so that with default values for the tunables,
> +   the computation of dl_tls_static_surplus in
> +   _dl_tls_static_surplus_init yields the historic value 1664, for
> +   backwards compatibility.  */
> +#define LEGACY_TLS (1664 - tls_static_surplus (DEFAULT_NNS, OPTIONAL_TLS))
> +
>  /* Calculate the size of the static TLS surplus, when the given
>     number of audit modules are loaded.  Must be called after the
>     number of audit modules is known and before static TLS allocation.  */
> @@ -74,8 +98,8 @@ _dl_tls_static_surplus_init (size_t naudit)
>    opt_tls = TUNABLE_GET (optional_static_tls, size_t, NULL);
>  #else
>    /* Default values of the tunables.  */
> -  nns = 4;
> -  opt_tls = 512;
> +  nns = DEFAULT_NNS;
> +  opt_tls = OPTIONAL_TLS;
>  #endif
>    if (nns > DL_NNS)
>      nns = DL_NNS;
> @@ -85,9 +109,8 @@ _dl_tls_static_surplus_init (size_t naudit)
>    nns += naudit;
>  
>    GL(dl_tls_static_optional) = opt_tls;
> -  GLRO(dl_tls_static_surplus) = ((nns - 1) * LIBC_IE_TLS
> -				 + nns * OTHER_IE_TLS
> -				 + opt_tls);
> +  assert (LEGACY_TLS >= 0);
> +  GLRO(dl_tls_static_surplus) = tls_static_surplus (nns, opt_tls) + LEGACY_TLS;
>  }
>  
>  /* Out-of-memory handler.  */
> diff --git a/elf/tst-tls-surplus.c b/elf/tst-tls-surplus.c
> new file mode 100644
> index 0000000000..b0dea0b5ee
> --- /dev/null
> +++ b/elf/tst-tls-surplus.c
> @@ -0,0 +1,42 @@
> +/* Test size of the static TLS surplus reservation for backwards compatibility.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/xdlfcn.h>
> +
> +static int do_test (void);
> +#include <support/test-driver.c>
> +
> +/* This hack results in a definition of struct rtld_global_ro.  Do
> +   this after all the other header inclusions, to minimize the
> +   impact.  */
> +#define SHARED
> +#include <ldsodefs.h>
> +
> +static
> +int do_test (void)
> +{
> +  /* Avoid introducing a copy relocation due to the hidden alias in
> +     ld.so.  */
> +  struct rtld_global_ro *glro = xdlsym (NULL, "_rtld_global_ro");
> +  printf ("info: _dl_tls_static_surplus: %zu\n", glro->_dl_tls_static_surplus);
> +  /* Hisoric value: 16 * 100 + 64.  */
> +  TEST_VERIFY (glro->_dl_tls_static_surplus >= 1664);
> +  return 0;
> +}
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] elf: Change TLS static surplus default back to 1664
  2020-07-19  4:39 ` Vineet Gupta
@ 2020-07-20 14:33   ` Florian Weimer
  0 siblings, 0 replies; 5+ messages in thread
From: Florian Weimer @ 2020-07-20 14:33 UTC (permalink / raw)
  To: Vineet Gupta via Libc-alpha; +Cc: Vineet Gupta, arcml

* Vineet Gupta via Libc-alpha:

> Hi Florian,
>
> On 7/17/20 4:41 AM, Florian Weimer via Libc-alpha wrote:
>> Make the computation in elf/dl-tls.c more transparent, and add
>> an explicit test for the historic value.
>> 
>> ---
>>  elf/Makefile          |  4 +++-
>>  elf/dl-tls.c          | 37 ++++++++++++++++++++++++++++++-------
>>  elf/tst-tls-surplus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 75 insertions(+), 8 deletions(-)
>
> On ARC, tst-tls-surplus passes, but it seems there's still some static TLS
> shenanigans.
>
> FAIL: elf/tst-tls-ie
> FAIL: elf/tst-tls-ie-dlmopen
>
> $ cat elf/tst-tls-ie.out
>
> maintls[1000]:	 0x20026f80 .. 0x20027368
> var0[480]:	 0x2093c648 .. 0x2093c828 global-dynamic
> var1[120]:	 0x2093cc40 .. 0x2093ccb8 global-dynamic
> var2[24]:	 0x2093d0d0 .. 0x2093d0e8 global-dynamic
> var3[16]:	 0x2093d500 .. 0x2093d510 global-dynamic
> error: xdlfcn.c:29: error: dlopen:
> glibc-2.31.9000-730-ge9422236a2dd4/build/elf/tst-tls-ie-mod4.so: cannot allocate
> memory in static TLS block
>
> cat elf/tst-tls-ie-dlmopen.out
> maintls[1000]:	 0x20026f80 .. 0x20027368
> var0[480]:	 0x2093c648 .. 0x2093c828 global-dynamic
> var1[120]:	 0x2093d3d8 .. 0x2093d450 global-dynamic
> var2[24]:	 0x2093e000 .. 0x2093e018 global-dynamic
> var3[16]:	 0x2093ebc8 .. 0x2093ebd8 global-dynamic
> error: xdlmopen.c:28: error: dlmopen:
> glibc-2.31.9000-730-ge9422236a2dd4/build/elf/tst-tls-ie-mod6.so: cannot allocate
> memory in static TLS block

I looked at the binaries, but I do not see anything obviously wrong.  I
think you need to single-step through the allocations to see where they
fail, sorry.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-20 14:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 11:41 [PATCH v2] elf: Change TLS static surplus default back to 1664 Florian Weimer
2020-07-17 16:22 ` Carlos O'Donell
2020-07-17 20:07 ` Matheus Castanho
2020-07-19  4:39 ` Vineet Gupta
2020-07-20 14:33   ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).