[Buildroot] [PATCH/next v2 1/2] valgrind: let the valgrind configure script detect TLS availability

[Buildroot] [PATCH/next v2 1/2] valgrind: let the valgrind configure script detect TLS availability

[Thomas Petazzoni]

Back in 2005, in commit
a2c326396a43ecbc8d02c3d815d4010a7ba2e004 ("update valgrind to the latest
and greatest"), an explicit --disable-tls option was added. More
recently, in commit 31a3f4bd54e12b8d6de286ab8fb6d9651990e2f5 ("valgrind:
enable tls support") changed this to be conditional on
BR2_GCC_ENABLE_TLS.

However, the configure script of valgrind is perfectly capable of
detecting TLS support, even in a cross-compilation case: it tries to
compile a program that uses __thread and sees if it works.

Since we're about to modify how BR2_GCC_ENABLE_TLS is handled, we'd
better remove its usage from packages, and valgrind is the only package
using this config option.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Changes since v1:
 - Added Arnout Reviewed-by.
---
 package/valgrind/valgrind.mk | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/package/valgrind/valgrind.mk b/package/valgrind/valgrind.mk
index 46ba13e..b97d446 100644
--- a/package/valgrind/valgrind.mk
+++ b/package/valgrind/valgrind.mk
@@ -15,12 +15,6 @@ VALGRIND_INSTALL_STAGING = YES
 # patch 0004-Fixes-for-musl-libc.patch touching configure.ac
 VALGRIND_AUTORECONF = YES
 
-ifeq ($(BR2_GCC_ENABLE_TLS),y)
-VALGRIND_CONF_OPTS += --enable-tls
-else
-VALGRIND_CONF_OPTS += --disable-tls
-endif
-
 # When Valgrind detects a 32-bit MIPS architecture, it forcibly adds
 # -march=mips32 to CFLAGS; when it detects a 64-bit MIPS architecture,
 # it forcibly adds -march=mips64. This causes Valgrind to be built
-- 
2.7.4

[Buildroot] [PATCH/next v2 2/2] gcc: remove BR2_GCC_ENABLE_TLS option

[Thomas Petazzoni]

The current BR2_GCC_ENABLE_TLS can cause users to make incorrect
choices, and is not very useful. This options allows to decide whether
we pass --enable-tls or --disable-tls to gcc, to enable or disable
support for Thread Local Storage.

Its behavior is:

 - The option is default to "y" but only exists if we're using
   uClibc/NPTL or glibc.

 - When we're using uClibc, the option can be disabled.

So, in practice, this means that currently:

 - TLS support is always on for glibc

 - TLS support is on by default for uClibc/NPTL, but can be disabled in
   the configuration. This is in fact bad and causes the build failure
   reported in bug #7424 (this bug is still reproducible on master)

 - TLS support is always disabled for uClibc/no-thread and
   uClibc/linuxthreads.

 - TLS support is always disabled for musl. This does not cause any
   build failure, but musl can use TLS support, and therefore be more
   efficient. According to
   http://www.openwall.com/lists/musl/2012/10/04/1, "Note that if you've
   been building gcc with --disable-tls, __thread was already working
   but gets emulated (very poorly; it's slow and will abort() if it runs
   out of memory) through libgcc.".

So, this commit completely removes the BR2_GCC_ENABLE_TLS and instead
makes the right choice inside gcc.mk directly:

 - TLS support enabled for glibc, musl and uClibc/NPTL

 - TLS support in other cases, i.e uClibc/no-thread and
   uClibc/linuxthreads.

We have intentionally *not* added the option to
Config.in.legacy. Indeed, the new behavior is *exactly* the same as the
older behavior, with the exception of:

 - People can no longer disable TLS support in uClibc/NPTL, which was
   anyway causing a build failure and therefore was not used.

 - TLS support is now enabled on musl, but people using musl already had
   BR2_GCC_ENABLE_TLS not set, so they wouldn't get the legacy warning.

Fixes bug #7424.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes since v1:
 - Invert the condition logic, to make it simpler. Suggested by Arnout.
---
 package/gcc/Config.in.host | 8 --------
 package/gcc/gcc.mk         | 9 ++++++---
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/package/gcc/Config.in.host b/package/gcc/Config.in.host
index 37728e0..8ed946b 100644
--- a/package/gcc/Config.in.host
+++ b/package/gcc/Config.in.host
@@ -148,14 +148,6 @@ config BR2_TOOLCHAIN_BUILDROOT_FORTRAN
 	  Fortran language and you want Fortran libraries to be
 	  installed on your target system.
 
-config BR2_GCC_ENABLE_TLS
-	bool "Enable compiler tls support" if BR2_TOOLCHAIN_BUILDROOT_UCLIBC
-	default y
-	depends on BR2_PTHREADS_NATIVE || BR2_TOOLCHAIN_BUILDROOT_GLIBC
-	help
-	  Enable the compiler to generate code for accessing
-	  thread local storage variables
-
 config BR2_GCC_ENABLE_LTO
 	bool "Enable compiler link-time-optimization support"
 	select BR2_BINUTILS_ENABLE_LTO
diff --git a/package/gcc/gcc.mk b/package/gcc/gcc.mk
index 39c3eeb..82050b4 100644
--- a/package/gcc/gcc.mk
+++ b/package/gcc/gcc.mk
@@ -133,10 +133,13 @@ ifeq ($(BR2_sparc)$(BR2_sparc64),y)
 HOST_GCC_COMMON_CONF_OPTS += --disable-libsanitizer
 endif
 
-ifeq ($(BR2_GCC_ENABLE_TLS),y)
-HOST_GCC_COMMON_CONF_OPTS += --enable-tls
-else
+# TLS support is not needed on uClibc/no-thread and
+# uClibc/linux-threads, otherwise, for all other situations (glibc,
+# musl and uClibc/NPTL), we need it.
+ifeq ($(BR2_TOOLCHAIN_BUILDROOT_UCLIBC)$(BR2_PTHREADS)$(BR2_PTHREADS_NONE),yy)
 HOST_GCC_COMMON_CONF_OPTS += --disable-tls
+else
+HOST_GCC_COMMON_CONF_OPTS += --enable-tls
 endif
 
 ifeq ($(BR2_GCC_ENABLE_LTO),y)
-- 
2.7.4

[Buildroot] [PATCH/next v2 2/2] gcc: remove BR2_GCC_ENABLE_TLS option

[Arnout Vandecappelle]

On 30-08-16 23:33, Thomas Petazzoni wrote:
[snip]
> So, this commit completely removes the BR2_GCC_ENABLE_TLS and instead
> makes the right choice inside gcc.mk directly:
> 
>  - TLS support enabled for glibc, musl and uClibc/NPTL
> 
>  - TLS support in other cases, i.e uClibc/no-thread and
>    uClibc/linuxthreads.
[snip]
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> Changes since v1:
>  - Invert the condition logic, to make it simpler. Suggested by Arnout.
> ---
[snip]
> diff --git a/package/gcc/gcc.mk b/package/gcc/gcc.mk
> index 39c3eeb..82050b4 100644
> --- a/package/gcc/gcc.mk
> +++ b/package/gcc/gcc.mk
> @@ -133,10 +133,13 @@ ifeq ($(BR2_sparc)$(BR2_sparc64),y)
>  HOST_GCC_COMMON_CONF_OPTS += --disable-libsanitizer
>  endif
>  
> -ifeq ($(BR2_GCC_ENABLE_TLS),y)
> -HOST_GCC_COMMON_CONF_OPTS += --enable-tls
> -else
> +# TLS support is not needed on uClibc/no-thread and

 I thought it was not even available for nothread or linuxthreads?

 But anyway:
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


> +# uClibc/linux-threads, otherwise, for all other situations (glibc,
> +# musl and uClibc/NPTL), we need it.
> +ifeq ($(BR2_TOOLCHAIN_BUILDROOT_UCLIBC)$(BR2_PTHREADS)$(BR2_PTHREADS_NONE),yy)
>  HOST_GCC_COMMON_CONF_OPTS += --disable-tls
> +else
> +HOST_GCC_COMMON_CONF_OPTS += --enable-tls
>  endif
>  
>  ifeq ($(BR2_GCC_ENABLE_LTO),y)
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH/next v2 2/2] gcc: remove BR2_GCC_ENABLE_TLS option

[Thomas Petazzoni]

Hello,

On Wed, 31 Aug 2016 00:58:36 +0200, Arnout Vandecappelle wrote:

> > +# TLS support is not needed on uClibc/no-thread and  
> 
>  I thought it was not even available for nothread or linuxthreads?

Well: TLS support is provided by the compiler and used by the C
library. At least that's how I see it. Since here we are in gcc.mk, we
are talking from the point of the view of the compiler, hence the "TLS
support is not needed for <this C library> variant".

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH/next v2 1/2] valgrind: let the valgrind configure script detect TLS availability

[Thomas Petazzoni]

Hello,

On Tue, 30 Aug 2016 23:33:27 +0200, Thomas Petazzoni wrote:
> Back in 2005, in commit
> a2c326396a43ecbc8d02c3d815d4010a7ba2e004 ("update valgrind to the latest
> and greatest"), an explicit --disable-tls option was added. More
> recently, in commit 31a3f4bd54e12b8d6de286ab8fb6d9651990e2f5 ("valgrind:
> enable tls support") changed this to be conditional on
> BR2_GCC_ENABLE_TLS.
> 
> However, the configure script of valgrind is perfectly capable of
> detecting TLS support, even in a cross-compilation case: it tries to
> compile a program that uses __thread and sees if it works.
> 
> Since we're about to modify how BR2_GCC_ENABLE_TLS is handled, we'd
> better remove its usage from packages, and valgrind is the only package
> using this config option.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> Changes since v1:
>  - Added Arnout Reviewed-by.
> ---
>  package/valgrind/valgrind.mk | 6 ------
>  1 file changed, 6 deletions(-)

I've applied both patches to next.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com